Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic Route Parameters Options #1268

Closed
lsabi opened this issue Apr 28, 2021 · 7 comments · Fixed by #1373
Closed

Dynamic Route Parameters Options #1268

lsabi opened this issue Apr 28, 2021 · 7 comments · Fixed by #1373
Labels
documentation Improvements or additions to documentation router
Milestone

Comments

@lsabi
Copy link

lsabi commented Apr 28, 2021

Is your feature request related to a problem? Please describe.
So, I'm using a dynamic route in a file /path/[slug]-[id].svelte. If I navigate to /path/foo-1, the variables are assigned as follows

slug === "foo"
id === "1"

Instead, when I navigate to /path/foo1-foo2-1, then the variables are assigned as follows

slug === "foo1"
id === "foo2-1"

Describe the solution you'd like
What I expect is for the first dynamic parameter to match the longest possible pattern. Otherwise, at least, have a way to specify it. For instance the regex [a-zA-Z-]*-[a-zA-Z] matches the pattern with the last characters after - to be in the second part, but this is not a valid name for a file.

Also, the docs IMHO could be more extensive regarding this subject.

Describe alternatives you've considered
One can match the entire part of the route and then split it.

@Rich-Harris Rich-Harris added this to the 1.0 milestone Apr 28, 2021
@benmccann
Copy link
Member

benmccann commented Apr 28, 2021

Hmm. I'm not sure what's best here because the URL is ambiguous with respect to the pattern. In your case the request makes sense, but if someone else had [id]-[slug] they'd probably want the opposite (though I think your URL looks nicer in that case). Or another example would be if someone wanted [slug]-[date] where date is something like 2021-04-28.

@Conduitry
Copy link
Member

One of the reasons for implementing route fallthrough was so that we could get rid of Sapper's regex route params.

I don't have strong opinions about how to resolve this. Maybe it would be nice for SvelteKit to provide all different partitionings of the path and try them in some arbitrary order, but that sounds annoying.

A simpler solution would be to pick one way to handle this, document it, and also document that if people want different behavior, they can use a single param for the folder name or filename and then do their own splitting and fall through if appropriate.

@benmccann
Copy link
Member

I hadn't thought of the single param solution. I agree that's the best way to handle this and documenting it would be nice

@benmccann benmccann added the documentation Improvements or additions to documentation label Apr 28, 2021
@lsabi
Copy link
Author

lsabi commented Apr 29, 2021

As @benmccann suggested, a single dynamic parameter can be matched against a regex in the load function. Though the developer has then to redirect to a 404 Not found url. It could become tricky to maintain, especially for big projects.

Another possibility could be to specify a regex that sveltekit can use to match against the dynamic parameter and handle the routing in case of 404. In case the route exists/is valid, one will then access the parameter and split it wherever more appropriate.

@GrygrFlzr
Copy link
Member

the developer has then to redirect to a 404 Not found url

We can already take advantage of fallthrough by not returning in the load function, so non-matches would already turn into a 404.

Rich-Harris pushed a commit that referenced this issue May 7, 2021
* test for split params ambiguous behaviour (#1268)

* docs
@irg1008
Copy link

irg1008 commented Aug 1, 2021

Sorry to type in here again but this is not working for me:

"We can already take advantage of fallthrough by not returning in the load function, so non-matches would already turn into a 404."

Instead of falling back to 404 page it says:
"Bad request in load function: failed to fetch undefined"

@benmccann
Copy link
Member

There's an open bug for that: #2041

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation router
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants