mirror of
https://github.com/angular/angular.git
synced 2026-09-14 13:54:52 +08:00
refactor(router): Remove special logic for hybrid apps (#45240)
This commit removes special (undocumented) logic in the Router code that is
meant to prevent duplicate navigations that result from location syncs in
AngularJS/Angular hybrid applications.
The duplicate navigations can occur when both the Router and the AngularJS sync
code detect a location change via a popstate/hashchange event. When this
happens, the Angular Router schedules a navigation to sync itself with
the browser, but the hybrid listener may also schedule an additional
navigation. There are a few reasons this logic should not be included in
the Router:
* This special logic is not tree shakeable so it introduces a bundle
size cost for all applications, most of which don't need it.
* There have been many updates to the routing pipeline to tolerate
duplicate navigations. That is, duplicate navigations can happen and
routing should still complete successfully.
* https://github.com/angular/angular/commit/0e8548f667e5fdefa3ac7cdf1ba47e3e17011ffc
* https://github.com/angular/angular/commit/9e039ca68bfae5328f3fc1f16fabd7673c466a25
* The logic is really in the wrong place: The hybrid sync code should be
the location to handle this. If duplicate navigations are meant to be
avoided, the hybrid sync code should have handling to _not_ trigger
duplicate navs.
* This logic _also_ used to exist because the mock location
helper used for test incorrectly triggered popstate events during
router navigations. In order to avoid unexpected behavior in tests, this
logic needed to be added. This incorrect mocking may also have been
put in place because the upgrade module _would_ see a location change
event and trigger a duplicate navigation. The location mock has since been updated to
match real browser behavior so this is no longer necessary. The
upgrade module has also been updated to not trigger duplicate
navigations. The following commits are related to this:
* https://github.com/angular/angular/commit/202a1a56314af4ddb99c476f974536a10e390319
* https://github.com/angular/angular/commit/c6a93001eb74374b0fbc6aea1286fe1183d21382
Side note: The `setTimeout` in the location change listener is used to
ensure the ordering of duplicate navigations was consistent. You can see
that the logic being removed here expects the imperative navigation to precede the
popstate/hashchange. With the removal of this code, the `setTimeout` no
longer serves a purpose. However, it has been found that tests can rely
on this behavior (incorrectly) because they expect the navigation to be
complete but in reality, it hasn't even started because the test has not
flushed the timeout. Removing the timeout would be a breaking change as
a result.
PR Close #45240
This commit is contained in:
committed by
Andrew Kushnir
parent
913bbba42b
commit
e16dab82a1
@@ -1354,21 +1354,6 @@ export class Router {
|
||||
return Promise.resolve(false);
|
||||
}
|
||||
|
||||
// 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;
|
||||
const browserNavPrecededByRouterNav = isBrowserTriggeredNavigation(source) && lastNavigation &&
|
||||
!isBrowserTriggeredNavigation(lastNavigation.source);
|
||||
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
|
||||
}
|
||||
|
||||
let resolve: any;
|
||||
let reject: any;
|
||||
let promise: Promise<boolean>;
|
||||
|
||||
@@ -1204,26 +1204,6 @@ describe('Integration', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('should ignore the duplicate resulting from a location sync', fakeAsync(() => {
|
||||
const router = TestBed.inject(Router);
|
||||
const fixture = createRoot(router, RootCmp);
|
||||
const location = TestBed.inject(Location) as SpyLocation;
|
||||
router.resetConfig([{path: 'simple', component: SimpleCmp, canActivate: ['in1Second']}]);
|
||||
|
||||
const recordedEvents: any[] = [];
|
||||
router.events.forEach(e => onlyNavigationStartAndEnd(e) && recordedEvents.push(e));
|
||||
|
||||
// setTimeout used so this navigation resolves at the same time as the one that results
|
||||
// from the location PopStateEvent (see Router#setUpLocationChangeListener).
|
||||
setTimeout(() => {
|
||||
router.navigateByUrl('/simple');
|
||||
}, 0);
|
||||
location.simulateUrlPop('/simple');
|
||||
tick(1000);
|
||||
advance(fixture);
|
||||
expectEvents(recordedEvents, [[NavigationStart, '/simple'], [NavigationEnd, '/simple']]);
|
||||
}));
|
||||
|
||||
it('should reset location if a navigation by location is successful', fakeAsync(() => {
|
||||
const router = TestBed.inject(Router);
|
||||
const location = TestBed.inject(Location) as SpyLocation;
|
||||
|
||||
Reference in New Issue
Block a user