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

isActive is not set when routes use wildcards #2115

Closed
hugoattal opened this issue Jan 24, 2024 · 2 comments
Closed

isActive is not set when routes use wildcards #2115

hugoattal opened this issue Jan 24, 2024 · 2 comments

Comments

@hugoattal
Copy link

hugoattal commented Jan 24, 2024

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 #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
Copy link
Author

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 ❤️!

Copy link
Member

posva commented Jan 24, 2024

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)

@posva posva closed this as not planned Won't fix, can't repro, duplicate, stale Jan 24, 2024
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