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

Allow [...rest] params to be optional #768

Merged
merged 3 commits into from
Mar 31, 2021
Merged

Allow [...rest] params to be optional #768

merged 3 commits into from
Mar 31, 2021

Conversation

Rich-Harris
Copy link
Member

With this PR, /foo/bar would match src/routes/foo/[...rest]/bar, and page.params.rest would equal ''. Closes #751

@benmccann
Copy link
Member

/foo/bar would match src/routes/foo/[...rest]/bar

I'm not sure whether that's what I would expect. It makes sense that it's optional when it's last. Less sure when it's in the middle

@Conduitry
Copy link
Member

As I mentioned on the issue, I think it's pretty safe to match liberally because you can always limit it with load fallthrough.

@benmccann
Copy link
Member

I'm not sure which page I would expect to be evaluated first with fall-through. I can imagine it causing unexpected bugs. E.g. I add a new route foo/[...rest]/bar and then foo/bar unexpectedly loads the new route. I'm unlikely to have visited foo/bar while developing foo/[...rest]/bar so could easily miss this this and not know I need to develop a fallthrough case for it. I sort of think that the better way to support such a case would be to allow programmatic access to the router for the user to add arbitrary regex routes. That would support a wider range of use cases and be more explicit. It seems like there'd be a lot of potential for confusion to do it by default

@Rich-Harris
Copy link
Member Author

I think that's really just a question of documentation. If someone already had src/routes/foo/bar.svelte then it would match first (rest parameters are low specificity). And presumably you're going to be checking that params.rest is valid anyway, otherwise things are guaranteed to break

@rmunn
Copy link
Contributor

rmunn commented Apr 15, 2021

I created a route src/routes/foo/[...rest]/bar, and it worked as designed: loading localhost:3000/foo/bar matched the route, with page.params.rest equal to '' (empty string). But it also matched the route localhost:3000/food/bar with page.params.rest equal to 'd'. Details at #918 (comment).

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

Successfully merging this pull request may close these issues.

Should / get picked up by [...index].svelte?
4 participants