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

Inconsistent trailing slash behaviour with multiple optional parameters #2190

Closed
panstromek opened this issue Mar 28, 2024 · 5 comments
Closed

Comments

@panstromek
Copy link
Contributor

panstromek commented Mar 28, 2024

Reproduction

https://jsfiddle.net/7sg5pry1/

Steps to reproduce the bug

Try route like this:

'/page/:first(a)?:second(b)?'

(clicking the button in the reporduction link)

Expected behavior

It matches:

/page

Text "should be visible" is visible in repro

Actual behavior

Doesn't match /page

Text "should be visible" is not visible in repro

Additional information

This is inconsistent with the behaviour with only a single optional parameter:

'/page/:first(a)?' matches /page

Documentation doesn't explicitly mention this case, but implies should that trailing slash is removed in both cases

Copy link
Member

posva commented Mar 28, 2024

By having only 2 optional parameters, the parser expects the path to have at least one character. It only makes it fully optional if it's only an optional param. So you can group both params into a single one or create an alias without the params. I'm curious to see what the real route is to see if this is worth supporting or if the alias solution is good enough. Can you please share that?

@panstromek
Copy link
Contributor Author

The real route is something like this: '/articles/:slug([\\w-]+-)?:article_id(\\d+)'. The idea is to have optional slug for SEO and append id to it, so it should match

/articles/99
but also
/articles/new-vue-version-released-99

@panstromek
Copy link
Contributor Author

panstromek commented Mar 28, 2024

I realized the previous example doesn't have the optional id, sorry. That one is something like:

'users/:userId/videos/:slug([\\w-]+-)?:video_id(\\d+)?'

which should match
/users/1/videos/my-second-video-2
but also
/users/1/videos/2

It shows a list of videos in swipe screen, and optionally starts at the specific one based on video_id

@posva
Copy link
Member

posva commented Mar 28, 2024

I see. Thanks for sharing! Using an alias is indeed the correct solution

@posva posva closed this as not planned Won't fix, can't repro, duplicate, stale Mar 28, 2024
@panstromek
Copy link
Contributor Author

Ok, it would be useful to mention this in the docs, though. I broke a production for a few minutes, it's pretty unexpected behaviour. I'll submit a PR when I have some time to look into it

panstromek added a commit to panstromek/router that referenced this issue Mar 29, 2024
posva added a commit that referenced this issue Jul 9, 2024
…2192)

* docs: mention an edge case with mutliple optional params in the docs

See #2190 for more info.

* Update packages/docs/guide/essentials/route-matching-syntax.md

---------

Co-authored-by: Eduardo San Martin Morote <[email protected]>
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

No branches or pull requests

2 participants