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

`routerLinkActive` and `router.isActive` do not work with arrays in queryParams. #22223

Closed
gregoirechauvet opened this issue Feb 14, 2018 · 5 comments

Comments

@gregoirechauvet
Copy link

@gregoirechauvet gregoirechauvet commented Feb 14, 2018

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

routerLinkActive and router.isActive do not work with arrays in queryParams.

Expected behavior

routerLinkActive and router.isActive should match correctly an URL which has arrays in queryParams.

Minimal reproduction of the problem with instructions

Here is a plunker that show what the issue is: http://plnkr.co/edit/CgMuRGMvsu2KAOm0aYOQ
If you click on Link with an array in the plunker, the navigation is achieved, but the class active is not added, and the link is not red. Because there is a queryParams with an array.

However, if you click on Link with an integer, it works fine, and the link is highlighted in red.

Identification of the issue

I have looked for the issue in angular code, and I think the issue is caused by this method which compare queryParams:
In file packages/router/src/url_tree.ts:

function containsQueryParams(
    container: {[k: string]: string}, containee: {[k: string]: string}): boolean {
  return Object.keys(containee).length <= Object.keys(container).length &&
      Object.keys(containee).every(key => containee[key] === container[key]);
}

The issue is that containee[key] === container[key] cannot work with arrays.
[1] === [1] is false.

Maybe I can do a pull request to fix this.

What is the motivation / use case for changing the behavior?

I am using arrays in queryParams, to store some state in the URL, in order to be able to save the link and use it later.
I could probably use a workaround ; sort and stringify my array before using it, but I prefer not to.

Environment


Angular version: 5.2.5


Browser:
- [x] Chrome (desktop) version XX
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [x] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX
@jasonaden

This comment has been minimized.

Copy link
Contributor

@jasonaden jasonaden commented Feb 20, 2018

@Prince-Leto Thanks for the issue. Yes, a PR would definitely be helpful.

@gregoirechauvet

This comment has been minimized.

Copy link
Author

@gregoirechauvet gregoirechauvet commented Feb 23, 2018

Ok, I will try to make a pull request in 3 three weeks. I will have some time.

@ngbot ngbot bot modified the milestones: needsTriage, Backlog Feb 26, 2018
Zaid-Al-Omari pushed a commit to Zaid-Al-Omari/angular that referenced this issue Mar 8, 2018
…ntain arrays

The url_tree equalQueryParams and containsQueryParam methods did not handle query params that has arrays, which resulted in the routerLinkActive to not behave as expected, change was made to ensure query params with arrays are handled correctly

fixes angular#22223
Zaid-Al-Omari pushed a commit to Zaid-Al-Omari/angular that referenced this issue Mar 8, 2018
…ntain arrays

The url_tree equalQueryParams and containsQueryParam methods did not handle query params that has arrays, which resulted in the routerLinkActive to not behave as expected, change was made to ensure query params with arrays are handled correctly

fixes angular#22223
Zaid-Al-Omari added a commit to Zaid-Al-Omari/angular that referenced this issue Mar 8, 2018
…in arrays

The url_tree equalQueryParams and containsQueryParam methods did not handle query params that has arrays, which resulted in the routerLinkActive to not behave as expected, change was made to ensure query params with arrays are handled correctly

fixes angular#22223
Zaid-Al-Omari added a commit to Zaid-Al-Omari/angular that referenced this issue Mar 9, 2018
…in arrays

The url_tree equalQueryParams and containsQueryParam methods did not handle query params that has arrays, which resulted in the routerLinkActive to not behave as expected, change was made to ensure query params with arrays are handled correctly

fixes angular#22223
@Zaid-Al-Omari

This comment has been minimized.

Copy link
Contributor

@Zaid-Al-Omari Zaid-Al-Omari commented Mar 9, 2018

I have fixed the issue with the PR #22666.
kindly review @jasonaden

@IliaVolk

This comment has been minimized.

Copy link

@IliaVolk IliaVolk commented Jul 26, 2018

Any updates?

@stocksr

This comment has been minimized.

Copy link

@stocksr stocksr commented Nov 23, 2018

I would like this fix included please - can I help get the pull request merged?

mhevery added a commit to Zaid-Al-Omari/angular that referenced this issue Nov 11, 2019
…in arrays

The url_tree equalQueryParams and containsQueryParam methods did not handle query params that has arrays, which resulted in the routerLinkActive to not behave as expected, change was made to ensure query params with arrays are handled correctly

fixes angular#22223
@kara kara closed this in b30bb8d Nov 13, 2019
kara added a commit that referenced this issue Nov 13, 2019
…in arrays (#22666)

The url_tree equalQueryParams and containsQueryParam methods did not handle query params that has arrays, which resulted in the routerLinkActive to not behave as expected, change was made to ensure query params with arrays are handled correctly

fixes #22223

PR Close #22666
ajitsinghkaler added a commit to ajitsinghkaler/angular that referenced this issue Nov 18, 2019
…in arrays (angular#22666)

The url_tree equalQueryParams and containsQueryParam methods did not handle query params that has arrays, which resulted in the routerLinkActive to not behave as expected, change was made to ensure query params with arrays are handled correctly

fixes angular#22223

PR Close angular#22666
matsko added a commit that referenced this issue Nov 22, 2019
…in arrays (#22666)

The url_tree equalQueryParams and containsQueryParam methods did not handle query params that has arrays, which resulted in the routerLinkActive to not behave as expected, change was made to ensure query params with arrays are handled correctly

fixes #22223

PR Close #22666
matsko added a commit that referenced this issue Nov 22, 2019
…in arrays (#22666)

The url_tree equalQueryParams and containsQueryParam methods did not handle query params that has arrays, which resulted in the routerLinkActive to not behave as expected, change was made to ensure query params with arrays are handled correctly

fixes #22223

PR Close #22666
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.