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 race condition with RouterLink created dynamically from template ref #18983

Closed
andrewseguin opened this Issue Aug 31, 2017 · 1 comment

Comments

Projects
None yet
3 participants
@andrewseguin
Contributor

andrewseguin commented Aug 31, 2017

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 sets isActive in an immediately resolved Promise, but between creating the Promise and resolving it, the links may have changed their href.

This is manifested in the following plunker: http://plnkr.co/edit/iio3c6erK8H3qtn93l7c?p=preview

Apologies if the reproduction is not minimal enough. When container.addLink() is called, the container creates a new embedded view with the implicit data 'my-link'. That newly created view has a router link that uses the implicit data as its link.

It appears that router's update function is called twice: when the router link is created from the createEmbeddedView call in container, and then again when change detection runs to provide the implicit data. When the update() runs the first time, it sees that the link is undefined, which matches the current route. It sets a Promise to resolve setting isActive to true. Before that promise resolves, update() is called again with the 'my-link' href but does nothing because isActive is still false at the moment. After the thread completes, the promise resolves and incorrectly sets isActive to true.

Calling rla.update() manually corrects the RouterLinkActive's isActive.

Expected behavior

The router link active should reflect isActive properly and not have any race condition with the Promise.

Minimal reproduction of the problem with instructions

See http://plnkr.co/edit/iio3c6erK8H3qtn93l7c?p=preview

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

Caused a bug in the Angular Material CDK Table. See issue here: angular/material2#6701

Environment


Angular version: v4.3.6

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

This comment has been minimized.

Show comment
Hide comment
@Airblader

Airblader Nov 17, 2017

Any chance the fix for this introduced an infinite loop of some sort (seems like in recent history RouterLinkActive was prone to that already). With 4.4.6 I'm experiencing a hang and 100% CPU when using a component that pretty much only uses RouterLinkActive. On 4.4.5 it works fine and the 4.4.6 changelog is pretty small.

Airblader commented Nov 17, 2017

Any chance the fix for this introduced an infinite loop of some sort (seems like in recent history RouterLinkActive was prone to that already). With 4.4.6 I'm experiencing a hang and 100% CPU when using a component that pretty much only uses RouterLinkActive. On 4.4.5 it works fine and the 4.4.6 changelog is pretty small.

jmleoni pushed a commit to jmleoni/angular that referenced this issue Oct 6, 2018

jmleoni pushed a commit to jmleoni/angular that referenced this issue Oct 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment