fix(router): Only trigger router navigation on popstate events from Location subscription (#43328)

With the merging of #41730, our tests now more closely emulate real
browser behavior. This means that we can make changes to the `Router` which depend
on actual browser behavior rather than incorrectly mocked test behavior.

This change updates the logic in the `Router` to only trigger
navigations on `popstate` events. Since our `SpyLocation` now triggers
the `popstate` event correctly on `simulateHashChange`, `back`,
`forward`, and `simulatePopState`, we are able to rely on this being
true for all of these navigations in tests when using `SpyLocation` (the
default for `RouterTestingModule`).

In addition, this behavior relies on documented browser behavior: The
`popstate` event happens due to browser transitions as a result of user
triggered "back" button or otherwise.
https://developer.mozilla.org/en-US/docs/Web/API/Window/popstate_event#when_popstate_is_sent

In addition, this change chooses `popstate` over `hashchange` because it
is the event that is already being used for navigations since the
documented behavior of browsers is that `popstate` happens before
`hashchange`.

fixes #41782

PR Close #43328
This commit is contained in:
Andrew Scott
2021-09-02 13:31:44 -07:00
committed by Andrew Scott
parent 93c9932d17
commit 9e039ca68b
+8 -57
View File
@@ -351,16 +351,6 @@ function defaultRouterHook(snapshot: RouterStateSnapshot, runExtras: {
return of(null) as any;
}
/**
* Information related to a location change, necessary for scheduling follow-up Router navigations.
*/
type LocationChangeInfo = {
source: 'popstate'|'hashchange',
urlTree: UrlTree,
state: RestoredState|null,
transitionId: number
};
/**
* The equivalent `IsActiveMatchOptions` options for `Router.isActive` is called with `true`
* (exact = true).
@@ -407,11 +397,6 @@ export class Router {
private disposed = false;
private locationSubscription?: SubscriptionLike;
/**
* Tracks the previously seen location change from the location subscription so we can compare
* the two latest to see if they are duplicates. See setUpLocationChangeListener.
*/
private lastLocationChangeInfo: LocationChangeInfo|null = null;
private navigationId: number = 0;
/**
@@ -1039,13 +1024,15 @@ export class Router {
// run into ngZone
if (!this.locationSubscription) {
this.locationSubscription = this.location.subscribe(event => {
const currentChange = this.extractLocationChangeInfoFromEvent(event);
// The `setTimeout` was added in #12160 and is likely to support Angular/AngularJS
// hybrid apps.
if (this.shouldScheduleNavigation(this.lastLocationChangeInfo, currentChange)) {
const source = event['type'] === 'popstate' ? 'popstate' : 'hashchange';
if (source === 'popstate') {
// The `setTimeout` was added in #12160 and is likely to support Angular/AngularJS
// hybrid apps.
setTimeout(() => {
const {source, state, urlTree} = currentChange;
const extras: NavigationExtras = {replaceUrl: true};
// Navigations coming from Angular router have a navigationId state
// property. When this exists, restore the state.
const state = event.state?.navigationId ? event.state : null;
if (state) {
const stateCopy = {...state} as Partial<RestoredState>;
delete stateCopy.navigationId;
@@ -1054,50 +1041,14 @@ export class Router {
extras.state = stateCopy;
}
}
const urlTree = this.parseUrl(event['url']!);
this.scheduleNavigation(urlTree, source, state, extras);
}, 0);
}
this.lastLocationChangeInfo = currentChange;
});
}
}
/** Extracts router-related information from a `PopStateEvent`. */
private extractLocationChangeInfoFromEvent(change: PopStateEvent): LocationChangeInfo {
return {
source: change['type'] === 'popstate' ? 'popstate' : 'hashchange',
urlTree: this.parseUrl(change['url']!),
// Navigations coming from Angular router have a navigationId state
// property. When this exists, restore the state.
state: change.state?.navigationId ? change.state : null,
transitionId: this.getTransition().id
} as const;
}
/**
* Determines whether two events triggered by the Location subscription are due to the same
* navigation. The location subscription can fire two events (popstate and hashchange) for a
* single navigation. The second one should be ignored, that is, we should not schedule another
* navigation in the Router.
*/
private shouldScheduleNavigation(previous: LocationChangeInfo|null, current: LocationChangeInfo):
boolean {
if (!previous) return true;
const sameDestination = current.urlTree.toString() === previous.urlTree.toString();
const eventsOccurredAtSameTime = current.transitionId === previous.transitionId;
if (!eventsOccurredAtSameTime || !sameDestination) {
return true;
}
if ((current.source === 'hashchange' && previous.source === 'popstate') ||
(current.source === 'popstate' && previous.source === 'hashchange')) {
return false;
}
return true;
}
/** The current URL. */
get url(): string {
return this.serializeUrl(this.currentUrlTree);