vuejs / router

🚦 The official router for Vue.js
https://router.vuejs.org/
MIT License
3.88k stars 1.18k forks source link

Inconsistent trailing slash behaviour with multiple optional parameters #2190

Closed panstromek closed 6 months ago

panstromek commented 6 months ago

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

posva commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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

panstromek commented 6 months ago

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