skills/angular__angular
1
0
Fork 0
mirror of https://github.com/angular/angular.git synced 2026-09-14 13:54:52 +08:00
Code Issues Packages Projects Releases Wiki Activity

refactor(router): Remove use of browserUrlTree in scheduleNavigation (#43632)

Browse Source 
There are no added tests here because the change is generally equivalent
to what was there before. The goal of that piece of code is to prevent
duplicate navigations due to the location synchronization code between
AngularJS and Angular. That is, the Angular router listens to the
`popstate` event and triggers navigations, but so does the AngularJS
router and triggers navigations through `router.navigateByUrl`. The
`setTimeout` in the Angular Router's `setUpLocationChangeListener` is
there to make this bit of code work: the 'popstate'/'hashchange'
navigation will necessarily come after the imperative nav triggered by
AngularJS's location sync.

Anyhow, in the long run, I would like to get rid of this bit of code
altogether. We should not have special handling for these cases. The
AngularJS/Angular location sync should either:

1. Not trigger a duplicate navigation in the first place
or
2. Be tolerant to processing the duplicate navigation.

In the short term, this change benefits the router while we investigate
removing this logic completely because now browserUrlTree has a single
responsiblity: to support `onSameUrlNavigation==='reload'`. This means
that we can work towards removing the `browserUrlTree` tracking from the
Router code at some point as well and have the `onSameUrlNavigation`
logic be part of a pre-navigation hook that can be handled outside the
`Router`. This may or may not be possible, but the change here opens up
that change for investigation.

PR Close #43632
This commit is contained in:
Andrew Scott
2021-09-28 16:34:45 -07:00
committed by Andrew Kushnir
parent 25b9c8cc5a
commit a3ee474897
1 changed files with 9 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/router/src/router.ts
+9 -17
View File
@@ -1353,26 +1353,18 @@ export class Router {
return Promise.resolve(false);
}
// * Duplicate navigations may also be triggered by attempts to sync AngularJS and Angular
// router states.
// * Imperative navigations can be cancelled by router guards, meaning the URL won't change. If
// the user follows that with a navigation using the back/forward button or manual URL change,
// the destination may be the same as the previous imperative attempt. We should not skip
// these navigations because it's a separate case from the one above -- it's not a duplicate
// navigation.
// Duplicate navigations may be triggered by attempts to sync AngularJS and
// Angular router states. We have the setTimeout in the location listener to
// ensure the imperative nav is scheduled before the browser nav.
const lastNavigation = this.transitions.value;
// We don't want to skip duplicate successful navs if they're imperative because
// onSameUrlNavigation could be 'reload' (so the duplicate is intended).
const browserNavPrecededByRouterNav = isBrowserTriggeredNavigation(source) && lastNavigation &&
!isBrowserTriggeredNavigation(lastNavigation.source);
const lastNavigationSucceeded = this.lastSuccessfulId === lastNavigation.id;
// If the last navigation succeeded or is in flight, we can use the rawUrl as the comparison.
// However, if it failed, we should compare to the final result (urlAfterRedirects).
const lastNavigationUrl = (lastNavigationSucceeded || this.currentNavigation) ?
lastNavigation.rawUrl :
(lastNavigation.urlAfterRedirects ?? this.browserUrlTree);
const duplicateNav = lastNavigationUrl.toString() === rawUrl.toString();
if (browserNavPrecededByRouterNav && duplicateNav) {
const navToSameUrl = lastNavigation.rawUrl.toString() === rawUrl.toString();
const lastNavigationInProgress = lastNavigation.id === this.currentNavigation?.id;
// We consider duplicates as ones that goes to the same URL while the first
// is still processing.
const isDuplicateNav = navToSameUrl && lastNavigationInProgress;
if (browserNavPrecededByRouterNav && isDuplicateNav) {
return Promise.resolve(true); // return value is not used
}