Trailingslash has a wrong ranking and creates empty params in array if after a wildcard parameter

[Tofandel]

Reproduction

https://play.vuejs.org/#eNqtVV1P2zAU/SteNqnt1tqlQzxkoWNDaGzaB2LTXpY9pImbBhLbsh0oivrfd22niQsFXibUNrn33ONzfY9NE1RJwfCVCsKgqASXGjUolTTR9IMQaIOWkldocFPTQcw6gOS1prLLYuICPcTUbnOJENjVx6xjHsJnFDOEcK3o0JW794rXTA8HL6FsMArGQVsO+iJNK1FC/dwAo9XB/JyWJTeLvYgIvNqwsD/woLTkLJ+f1lJSpp1mJBK9CiPS5lDToFc2gZd1WV5AEm02loY4noglN1vCS6vya8GukebHcUDiYP6JwzM65xWNSJ9/tCJZpCSjy75SU6WfqQxtacOSioZoYAomIsnpYAzNyKRSIWowxq4LFzGZHMAz6MVfCN0W0KDJPVgyIttGI2OIXRm/C3qLSItr0xHxpgGvKpWF0EhRXZud66wC47UkvpUmrV3egSE4A11uNscdeDiCFEzJcs7BBK4A6z02/UYrLu/OC6XhZ9wGne59axqxLYOZmm1t69RtoLXr1ssLqNzBdZHO13RtoV43EtrxtQwbs30rJzPcp304GhuIrYah/nEjaJxl0YDAwFMOkhi4OezFb2yVAXoOseZwZaF5H+LX70ev71H0je3l6F32kMikSGi+h3GcvRk9w/wXvjdwvGGS/h6bM+37BpUJy8HrWoFtPQNYg907/LP5lgmO/mzXkLDMzoieWed/enW/1E5NqxV2uvEPrLlzdvU7TZMqEXA3cwYdWP/EbQKEw7F3Q4uDXqYJx8FKa6FCQmomrnMMcyE94uQQv8VTkoHnvCimqposJL9V8HwF7K0h4uAEQHBl3WjOSzVJRPHYEg+AJ0f4CB+QslgQYCcFy+jachtqaHgDbWoFm7os8ntNGisVJZU/hC5g03eaTeDKv/1iY1rWtBOarmh6vSd+pdZO8oWk0N0N9ZrTicypdumzn9/pGp67ZMWzugT0E8lLqnhZG40O9rFmGcj2cFbtZzuzguW/1NlaU6a2TRmhdjcs3g7y9InWe7lv8aG3i0rflVThVJnrEe7xMTKXtKtbcJlRCf8LxBqB2CJDL6fTKVgWAUjmBZssuNa8CtHBVKxtXCRZBmK7CKwSM6BFc5SgN/CxxG11SZdw4H3kauav3NP3AtI09QSEaAp/s5Yh2PwD2IzlHA==

Steps to reproduce the bug

1. Click on "Go to test"
2. Notice the route.params is an array with an empty element at the end
3. Click on "Go to test with page", notice the page has now 2 slashes in it

Expected behavior

Without a trailing slash the ranking is correct, I think the trailingslash should have a lower rank than a regex
https://paths.esm.dev/?p=AAMeJUyB3A7ACAROgDYMTAHuJXIFqHQAUAFACTg_OQA5INikft_qADWgAQVDQMk8w6RBgNwSAIA.&t=/en/brands/abcd/efh/page/2#

Actual behavior

With a trailing slash now the ranking is wrong and the second route can never be matched https://paths.esm.dev/?p=AAMeJUyB3A7ACAROgDYMTAHuJXIFqHQAUAFACTg_OQA5INikft_qADWg4AUBBfMMSSMghySCcrsAAA..&t=/en/brands/abcd/efh/page/2#

Additional information

I think the trailingslash should be discarded from the params list because it's in any case optional with vue router

The ranking should also be changed so that a trailingslash doesn't alter the ranking

Related #1300
#1123

[posva]

The ranking of a trailing slash is an intended behavior.
Note you can use the strict option too. Also, the page makes more sense as a query param in your case, which fixes the issue. This important when using the wildcard .* which matches anything since they can create redundant paths like this one

[Tofandel]

Also, the page makes more sense as a query param in your case, which fixes the issue.

Sorry, that's not an option

I'm using the strict and end option but the page is still not being matched if both have a trailingslash

Here are the regex and scores

^/en/product-category(?:/((?:.*)(?:/(?:.*))*))?/$ [ [ 80 ], [ 80 ], [ -8 ], [ 90.7 ] ]
^/en/product-category(?:/((?:.*)(?:/(?:.*))*))?/page(?:/(\d+))?/$ [ [ 80 ], [ 80 ], [ -8 ], [ 80 ], [ 62 ], [ 90.7 ] ]

As you can see the strict option is not working properly in this case
https://paths.esm.dev/?p=ABMMIPQgYAqQdgEjEDgB1jMwBaS3zAWgdABQAUAJJD84ADmgubTh2OsANaCQFQEFeAwz-MpjCYPVCgB5&t=/en/brands/abcd/efh/page/2/#

The ranking of a trailing slash is an intended behavior.

It might be but it's not a good one in certain cases, a trailing slash should rank lower than a slash that is not trailing if the end option is true or if a wildcard regex has been seen in the path, otherwise it can never be matched because any path with a wildcard before that will be checked and a wildcard by definition matches everything, so the wildcard will always match? How is a route that can never be matched intended behavior? (Changing it for those cases would not be a breaking change because of this reason and I can make a PR that fixes it with a test case)

The only way this works is with no trailingslash or by relying on a bug (#1301 (comment)) by using another regex than .* on one of the path that is equivalent like .*? which has a +50 higher score than .*

https://paths.esm.dev/?p=ABMMIPQgYAqQdgEjEDgB1jMwBaS3zAWgdABQAcAPOSUAkIDi0uZT7wOEgK7W1BRAGATw8cdmBpWlEvJ5AAA.&t=/en/brands/abcd/efh/page/2/#

[posva]

Having redundant paths will always yield issues in routing and should be avoided. Using .* can easily create these problems too. Using query params should also be your go to solution your pagination. In your case you might need to combine it with server redirects to fully work. In any case, I'm glad you found a workaround (BTW, the ranking difference with .* is not a bug).

[Tofandel]

Having redundant paths will always yield issues in routing and should be avoided

It shouldn't, that's never been a problem when routing uses the user provided order instead of computing a score, but if it computes a score then it should do so correctly or provide us with the option to change the score of a route manually to avoid this being an issue.
I would use one route instead of 2 having multiple regex in a route which wouldn't have a ranking problem, but that is also buggy with lazy modifiers being treated as greedy which is the bug I was referencing

Using query params should also be your go to solution your pagination. In your case you might need to combine it with server redirects to fully work

That's basically suggesting another feature because the core feature of the router is not working properly, I don't see how a server redirect when vue router does client side routing would work properly, I could have this issue with any other things than pagination that wouldn't be suited to query args for example /character/:character(.*?)+/ and /character/:character(.*?)+/pet/:pet/ where character is a page for the character and character/pet is another page for the character's pet with a different component and different content
https://paths.esm.dev/?p=ABMMIPQgYAwIFidgQHsFLmQ0xwWjJABEDqAqgPgBpAQogGAINVnPMQ1nmjJ3ABxVBnwA0g..&t=/character/buddy/the/one/pet/dog/# the fact that it's on pagination there is a distraction to the real problem

Btw if you start toggling on and off strict mode on this https://paths.esm.dev/?p=AAMeJYyBwQIJDJDWgQvuTXABS8EAfA6gKoD4AaAEgBoIKEV0sHR1bzqPAPBZdXstpCEAAA..&t=/character/buddy/the/one/pet/dog#, you will also see a similar but different issue on path ranking that causes the same problem without trailing slash, the strict bonus should not apply at all or be negative

I'm glad you found a workaround (BTW, the ranking difference with .* is not a bug)

Thanks, at least I have a workaround so it's a less pressing issue. - I'm talking about the fact that .*? is not treated as a lazy regex when it's wrapped by being marked as optional or repeated, but as a greedy one, not about the ranking (which means the 2 regexes are perfectly equivalent but if this bug is fixed then they wouldn't be perfectly equivalent, but bad example because in this particular case they are not being wrapped)

From what I gathered it just seems like an oversight there with trailingslashes being considered as another root segment, shouldn't this just happen for the actual root path / (so for the first segment) and not for trailing slashes? Like this a trailing slash would not add any segment score and would have the same behavior as no trailing slash

router/packages/router/src/matcher/pathParserRanker.ts

Line 141 in a24d55b

const segmentScores: number[] = segment.length ? [] : [PathScore.Root]

[Tofandel]

All in all I can submit a PR for bug 1 which is optional or repeated params lazyness is being overwritten by being wrapped (by checking the presence of (+|*|?)? in the regex and appending another ? to the wrapped regex in this case

And another PR for the trailingslash issue

I have no problem spending a bit of time for that. But they should be regarded as what they are, which is bugs. I only have a problem spending time on a PR for nothing (that will never be accepted because it is "intended behavior")

[posva]

I have no problem spending a bit of time for that. But they should be regarded as what they are, which is bugs. I only have a problem spending time on a PR for nothing (that will never be accepted because it is "intended behavior")

That's totally understandable.
If you manage to tweak the scoring while keeping scoring very similar (to avoid unexpected breaking changes outside), not break or change any existing assertions on ranking. I can take a look. To be fully transparent to you, if you open a PR, I will need some time to get it released (probably 2 to 3 months), but I will eventually take a look and address the problem if possible.

From what I gathered it just seems like an oversight there with trailingslashes being considered as another root segment, shouldn't this just happen for the actual root path / (so for the first segment) and not for trailing slashes? Like this a trailing slash would not add any segment score and would have the same behavior as no trailing slash

Maybe there is a differentiation to be made between end and non-end versions. In the future, I doubt the end option will be preserved by default as it creates incoherences in ranking (it seems this is one of them)

I'm talking about the fact that .*? is not treated as a lazy regex when it's wrapped by being marked as optional or repeated, but as a greedy one, not about the ranking (which means the 2 regexes are perfectly equivalent but if this bug is fixed then they wouldn't be perfectly equivalent, but bad example because in this particular case they are not being wrapped)

I see. Unless the current behavior exposes a ranking issue, I doubt any effort will be made to break the current workaround, especially knowing now that it is a workaround. It's great you bring it up thought

[Tofandel]

That is perfectly reasonable to me, my PR will likely not arrive before 2 weeks and there will be no rush on this.

And if you deem the ranking to be too different on the trailingslash issue I would have no problem if it got sent to the next major (I will included tests from the linked issues otherwise so I'm sure it won't be a breaking change,
scoring in the end only has any effect on overlaping routes with wildcards and changing trailingslash has no effect on this if the routes are declared consistently all with trailingslash or all without trailingslash), I can fix it multiple ways, 1 is to not have an additional 90 score at all in case of trailingslash that is not the root and one is to lower the score to 70 for trailingslash so it is less specific than a static match

I think option 1 would be the most correct way because trailingslash in the route definition should not change the score at least in non strict mode

I can also mix and match option 1 in non strict mode and option 2 in strict

I could also give the option to users to hardcode route priority in a minor and the ranking change in a major to still allow this kind of issues to be worked around until a few years when the next major releases