Internal router transition Subject has stale state #30340
Labels
Milestone
Comments
|
When fixed, need to un-revert this PR and verify this set of tests pass. |
jasonaden
added a commit
to jasonaden/angular
that referenced
this issue
May 8, 2019
…se redirect is skipped In some cases where multiple navigations happen to the same URL, the router will not process a given URL. In those cases, we fall into logic that resets state for the next navigation. One piece of this resetting is to set the `browserUrlTree` to the most recent `urlAfterRedirects`i. However, there was bug in this logic because in some cases the `urlAfterRedirects` is a stale value. This happens any time a URL won't be processed, and the previous URL will also not be processed. This creates unpredictable behavior, not the least of which ends up being a broken `back` button. This PR kicks off new navigations with the current value the router assumes is in the browser. All the logic around how to handle future navigations is based on this value compared to the current transition, so it's important to kick off all new navigations with the current value so in the edge case described above we don't end up with an old value being set into `browserUrlTree`. Fixes angular#30340 Related to angular#30160
jasonaden
added a commit
to jasonaden/angular
that referenced
this issue
May 8, 2019
…se redirect is skipped In some cases where multiple navigations happen to the same URL, the router will not process a given URL. In those cases, we fall into logic that resets state for the next navigation. One piece of this resetting is to set the `browserUrlTree` to the most recent `urlAfterRedirects`i. However, there was bug in this logic because in some cases the `urlAfterRedirects` is a stale value. This happens any time a URL won't be processed, and the previous URL will also not be processed. This creates unpredictable behavior, not the least of which ends up being a broken `back` button. This PR kicks off new navigations with the current value the router assumes is in the browser. All the logic around how to handle future navigations is based on this value compared to the current transition, so it's important to kick off all new navigations with the current value so in the edge case described above we don't end up with an old value being set into `browserUrlTree`. Fixes angular#30340 Related to angular#30160
alxhub
pushed a commit
that referenced
this issue
May 15, 2019
…se redirect is skipped (#30344) In some cases where multiple navigations happen to the same URL, the router will not process a given URL. In those cases, we fall into logic that resets state for the next navigation. One piece of this resetting is to set the `browserUrlTree` to the most recent `urlAfterRedirects`i. However, there was bug in this logic because in some cases the `urlAfterRedirects` is a stale value. This happens any time a URL won't be processed, and the previous URL will also not be processed. This creates unpredictable behavior, not the least of which ends up being a broken `back` button. This PR kicks off new navigations with the current value the router assumes is in the browser. All the logic around how to handle future navigations is based on this value compared to the current transition, so it's important to kick off all new navigations with the current value so in the edge case described above we don't end up with an old value being set into `browserUrlTree`. Fixes #30340 Related to #30160 PR Close #30344
BioPhoton
pushed a commit
to BioPhoton/angular
that referenced
this issue
May 21, 2019
…se redirect is skipped (angular#30344) In some cases where multiple navigations happen to the same URL, the router will not process a given URL. In those cases, we fall into logic that resets state for the next navigation. One piece of this resetting is to set the `browserUrlTree` to the most recent `urlAfterRedirects`i. However, there was bug in this logic because in some cases the `urlAfterRedirects` is a stale value. This happens any time a URL won't be processed, and the previous URL will also not be processed. This creates unpredictable behavior, not the least of which ends up being a broken `back` button. This PR kicks off new navigations with the current value the router assumes is in the browser. All the logic around how to handle future navigations is based on this value compared to the current transition, so it's important to kick off all new navigations with the current value so in the edge case described above we don't end up with an old value being set into `browserUrlTree`. Fixes angular#30340 Related to angular#30160 PR Close angular#30344
|
I see a difference in behavior betwen ng6 and ng8 and I am wondering if it is because of these changes. In ng6, navigating to the same route for the purposes of updating the query params did not trigger NavigationStart and NavigationEnd events. In ng8 it is so that navigating to the same route for the purposes of updating the query params does trigger NavigationStart and NavigationEnd events. Was this behavior change intended? |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Navigations are initialized by sending a new value into
Router.transitions. In order to do so, we take the new value and merge it with the previous value. However, that previous value will always be the previous value prior to navigations having been run.The real data we need is the
NavigationTransitionthat most recently passed to thesubscribeonRouter.navigations. This will contain reference to the recenturlAfterRedirectsand any other data added to the transition through the process of navigating.Starting with old values can produce incorrect behavior. For instance, a test in Google navigates to a route, then navigates to the same route again, which in the router codebase falls into a path that essentially marks it to do nothing. Upon marking it to do nothing, we attempt to get the state right for the next navigation. However, the value is stale due to the above description.
The next navigation they are performing in the test is hitting
back, which goes to the root. Because they happened to start on root, then navigate twice to the same place, then specifically gobackto the root again, thebackoperation does nothing because the router thinks it’s already there.The text was updated successfully, but these errors were encountered: