vuejs / router

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

`isActive` is not set when routes use wildcards #2115

Closed hugoattal closed 6 months ago

hugoattal commented 6 months ago

Reproduction

https://jsfiddle.net/5mg7jhz3/

Steps to reproduce the bug

Click on Bar (/foo/bar link) which is under the /foo/:other(.*)* route.

There are only two routes:

[
  { path: '/', component: Home },
  { path: '/foo/:other(.*)*', component: Foo }
]

Expected behavior

Foo (/foo link) should have

- isActive: true
- isExactActive: false

Actual behavior

Foo (/foo link) have

- isActive: false
- isExactActive: false

Additional information

I thought it was https://github.com/vuejs/router/issues/1552, but it's actually different, since it's the same route in the router.

According to https://router.vuejs.org/guide/migration/index.html#Removal-of-the-exact-prop-in-router-link- Routes are now active based on the route records they represent So I think it should be active 🤔

I can do a PR if that's okay.

hugoattal commented 6 months ago

After taking a look at the code, it seems like this is intended: https://github.com/vuejs/router/blob/main/packages/router/src/RouterLink.ts

const isActive = computed<boolean>(
  () =>
    activeRecordIndex.value > -1 &&
    includesParams(currentRoute.params, route.value.params)
)

I thought it worked something like this:

const isActive = computed<boolean>(
  () =>
    activeRecordIndex.value > -1 &&
    activeRecordIndex.value === currentRoute.matched.length - 1
)

Is that's so, I suggest adding something like isLooseActive.

If you don't think it's useful, that's okay, thanks a lot for your hard work ❤️!

posva commented 6 months ago

Hello! Yes, this is intended and explained in the migration guide. You should be able to find more details in the RFC I wrote for this. It was decided that it wasn't needed to add this method because it's a one liner of path.startsWith(otherPath)