Skip to content

The state passed to router.navigate is lost when doing multiple navigations #29389

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.

Sign up for GitHub

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

Jump to bottom
Closed
pcurrivan opened this issue Mar 19, 2019 · 3 comments
Closed

The state passed to router.navigate is lost when doing multiple navigations #29389

pcurrivan opened this issue Mar 19, 2019 · 3 comments
Labels
area: router
Milestone
needsTriage

Comments

@pcurrivan
Copy link

pcurrivan commented Mar 19, 2019 •
edited
Loading

🐞 bug report

Affected Package

@angular/router

Description

The state passed to router.navigate is lost when doing multiple navigations.

🔬 Minimal Reproduction

https://stackblitz.com/edit/angular-nrjzxe

Pressing 'FOO' will call router.navigate with state { foo: true }.
If you press it once, you will see in the console that the resolver can see this state (via getCurrentNavigation).
If you press it multiple times quickly (the resolver takes two seconds), all but the last navigation are canceled and during the navigations past the initial one, the resolver can no longer see this state.

I would expect to always be able to access the state from the resolver.

🌍 Your Environment

Angular version 7.2.5

EDIT: Some more info:
Looks like currentNavigation is set to null here:

angular/packages/router/src/router.ts

Line 721 in fd5cd10

this.currentNavigation = null;

It is then null here, contradicting the '!' non-null assertion operator:

angular/packages/router/src/router.ts

Line 506 in fd5cd10

...this.currentNavigation !,

Thus you are left with a currentNavigation only containing finalUrl.
@ngbot ngbot bot added this to the needsTriage milestone Mar 19, 2019
@Airblader Airblader mentioned this issue Mar 29, 2019
Closed
@pcurrivan
Copy link
Author

Moving the setting of currentNavigation:

angular/packages/router/src/router.ts

Line 457 in fd5cd10

this.currentNavigation = {
to inside the switchMap at 474:

angular/packages/router/src/router.ts

Line 474 in fd5cd10

switchMap(t => {
seems to fix it. Is it that simple?

jasonaden added a commit to jasonaden/angular that referenced this issue Apr 1, 2019
@jasonaden
fix(router): adjust setting navigationTransition when a new navigatio…
bcaa84a 
…n cancels an existing one

Prior to this change, if a navigation was ongoing and a new one came in, the router could get into a state where `router.currentNavigation` was `null` even though a navigation was executing. This change moves where we set the `currentNavigation` value so it's inside a `switchMap`. This solves the problem because the `finally` on the `switchMap` had been setting `currentNavigation` to `null` but the new `currentNavigation` value would have already been set. Essentially this was a timing problem and is resolved with this change.

Fixes angular#29389 angular#29590
@jasonaden jasonaden mentioned this issue Apr 1, 2019
Closed
@jasonaden
Copy link
Contributor

Thanks for the detailed analysis! It appears this does fix the problem. I created a PR to get this addressed.

jasonaden added a commit to jasonaden/angular that referenced this issue Apr 1, 2019
@jasonaden
fix(router): adjust setting navigationTransition when a new navigatio…
947e341 
…n cancels an existing one

Prior to this change, if a navigation was ongoing and a new one came in, the router could get into a state where `router.currentNavigation` was `null` even though a navigation was executing. This change moves where we set the `currentNavigation` value so it's inside a `switchMap`. This solves the problem because the `finally` on the `switchMap` had been setting `currentNavigation` to `null` but the new `currentNavigation` value would have already been set. Essentially this was a timing problem and is resolved with this change.

Fixes angular#29389 angular#29590
jasonaden added a commit that referenced this issue Apr 1, 2019
@jasonaden
fix(router): adjust setting navigationTransition when a new navigatio…
e884c0c 
…n cancels an existing one (#29636)

Prior to this change, if a navigation was ongoing and a new one came in, the router could get into a state where `router.currentNavigation` was `null` even though a navigation was executing. This change moves where we set the `currentNavigation` value so it's inside a `switchMap`. This solves the problem because the `finally` on the `switchMap` had been setting `currentNavigation` to `null` but the new `currentNavigation` value would have already been set. Essentially this was a timing problem and is resolved with this change.

Fixes #29389 #29590

PR Close #29636
wKoza pushed a commit to wKoza/angular that referenced this issue Apr 17, 2019
@jasonaden @wKoza
fix(router): adjust setting navigationTransition when a new navigatio…
880a585 
…n cancels an existing one (angular#29636)

Prior to this change, if a navigation was ongoing and a new one came in, the router could get into a state where `router.currentNavigation` was `null` even though a navigation was executing. This change moves where we set the `currentNavigation` value so it's inside a `switchMap`. This solves the problem because the `finally` on the `switchMap` had been setting `currentNavigation` to `null` but the new `currentNavigation` value would have already been set. Essentially this was a timing problem and is resolved with this change.

Fixes angular#29389 angular#29590

PR Close angular#29636
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area: router
Projects
None yet
Milestone
needsTriage
Development

Successfully merging a pull request may close this issue.

fix(router): adjust setting navigationTransition when a new navigatio… jasonaden/angular
3 participants
@jasonaden @pkozlowski-opensource @pcurrivan