Skip to content

Conversation

babu-ch
Copy link

@babu-ch babu-ch commented Jan 7, 2025

fix #2419

Removed if optional parameter is empty
Now the param will match if :to is string and if it is object

In the case of string path, it should not be an empty string parameter(Right?)
, so I thought there would be no problem with excluding it.

However, this may be a breaking change if you have users who were using this

In addition, I added a router-link to Playgound to check the operation.

Copy link

netlify bot commented Jan 7, 2025

Deploy Preview for vue-router canceled.

Name Link
🔨 Latest commit 66f824f
🔍 Latest deploy log https://app.netlify.com/sites/vue-router/deploys/677e37044abd7e000807f3dd

@@ -310,6 +310,12 @@ export function createRouterMatcher(
// we know the matcher works because we tested the regexp
params = matcher.parse(path)!
name = matcher.record.name

matcher?.keys.forEach(key => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the ?. be changed to . instead?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skirtles-code
Thank you for your comment.
Oh, it was a remnant from writing outside of this block.
Fixed!
66f824f

@skirtles-code
Copy link
Contributor

I was wondering about some other edge cases. I'm not sure whether these necessarily impact this PR, but I thought they were worth considered before it's merged.

I put together this Playground to demonstrate:

These are are to relevant values:

  1. '/features'
  2. { name: 'features' }
  3. { name: 'features', params: { id: undefined } }
  4. { name: 'features', params: { id: null } }
  5. { name: 'features', params: { id: '' } }

All of them lead to the same path.

Clicking around between them, it seems that 2, 3 and 4 are considered equivalent, which makes sense. It's 5 that I found most interesting. It seems to behave like it's a child of 2, 3 and 4.

I also tried with { name: 'features', params: { id: '123' } }, which seems to behave similarly. When that route is active it shows 2, 3 and 4 as active, though not exact active.

Not sure if that's intentional or not, but it doesn't seem to match what's described at https://router.vuejs.org/guide/essentials/active-links.html. I wrote that page, so I may be to blame for describing these classes incorrectly. It seems that there's some special handling for optional params that I didn't take into account in my description.

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

Successfully merging this pull request may close these issues.

Active class not applied on RouterLink when using to with string or object with path key
2 participants