Skip to content

Commit efb440e

Browse files
aahmedayedalxhub
authored andcommitted
refactor(router): compute correct history restoration when navigation is cancelled (angular#38884)
We can’t determine whether the user actually meant the `back` or the `forward` using the popstate event (triggered by a browser back/forward) so we instead need to store information on the state and compute the distance the user is traveling withing the browser history. So by using the `History#go` method, we can bring the user back to the page where he is supposed to be after performing the action. implementation for angular#13586 PR Close angular#38884
1 parent 52e0987 commit efb440e

File tree

3 files changed

+556
-212
lines changed

3 files changed

+556
-212
lines changed

goldens/size-tracking/integration-payloads.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@
3838
"cli-hello-world-lazy": {
3939
"master": {
4040
"uncompressed": {
41-
"runtime-es2015": 2850,
42-
"main-es2015": 295741,
43-
"polyfills-es2015": 36975,
44-
"src_app_lazy_lazy_module_ts-es2015": 825
41+
"runtime-es2015": 2854,
42+
"main-es2015": 296436,
43+
"polyfills-es2015": 37064,
44+
"src_app_lazy_lazy_module_ts-es2015": 822
4545
}
4646
}
4747
},

packages/router/src/router.ts

Lines changed: 109 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,12 @@ function defaultMalformedUriErrorHandler(
233233
}
234234

235235
export type RestoredState = {
236-
[k: string]: any; navigationId: number;
236+
[k: string]: any,
237+
// TODO(#27607): Remove `navigationId` and `ɵrouterPageId` and move to `ng` or `ɵ` namespace.
238+
navigationId: number,
239+
// The `ɵ` prefix is there to reduce the chance of colliding with any existing user properties on
240+
// the history state.
241+
ɵrouterPageId?: number,
237242
};
238243

239244
/**
@@ -303,6 +308,7 @@ export interface Navigation {
303308

304309
export type NavigationTransition = {
305310
id: number,
311+
targetPageId: number,
306312
currentUrlTree: UrlTree,
307313
currentRawUrl: UrlTree,
308314
extractedUrl: UrlTree,
@@ -409,6 +415,12 @@ export class Router {
409415
*/
410416
private lastLocationChangeInfo: LocationChangeInfo|null = null;
411417
private navigationId: number = 0;
418+
419+
/**
420+
* The id of the currently active page in the router.
421+
* Updated to the transition's target id on a successful navigation.
422+
*/
423+
private currentPageId: number = 0;
412424
private configLoader: RouterConfigLoader;
413425
private ngModule: NgModuleRef<any>;
414426
private console: Console;
@@ -509,6 +521,25 @@ export class Router {
509521
*/
510522
relativeLinkResolution: 'legacy'|'corrected' = 'corrected';
511523

524+
/**
525+
* Configures how the Router attempts to restore state when a navigation is cancelled.
526+
*
527+
* 'replace' - Always uses `location.replaceState` to set the browser state to the state of the
528+
* router before the navigation started.
529+
*
530+
* 'computed' - Will always return to the same state that corresponds to the actual Angular route
531+
* when the navigation gets cancelled right after triggering a `popstate` event.
532+
*
533+
* The default value is `replace`
534+
*
535+
* @internal
536+
*/
537+
// TODO(atscott): Determine how/when/if to make this public API
538+
// This shouldn’t be an option at all but may need to be in order to allow migration without a
539+
// breaking change. We need to determine if it should be made into public api (or if we forgo
540+
// the option and release as a breaking change bug fix in a major version).
541+
canceledNavigationResolution: 'replace'|'computed' = 'replace';
542+
512543
/**
513544
* Creates the router service.
514545
*/
@@ -535,6 +566,7 @@ export class Router {
535566

536567
this.transitions = new BehaviorSubject<NavigationTransition>({
537568
id: 0,
569+
targetPageId: 0,
538570
currentUrlTree: this.currentUrlTree,
539571
currentRawUrl: this.currentUrlTree,
540572
extractedUrl: this.urlHandlingStrategy.extract(this.currentUrlTree),
@@ -634,9 +666,7 @@ export class Router {
634666
tap(t => {
635667
if (this.urlUpdateStrategy === 'eager') {
636668
if (!t.extras.skipLocationChange) {
637-
this.setBrowserUrl(
638-
t.urlAfterRedirects, !!t.extras.replaceUrl, t.id,
639-
t.extras.state);
669+
this.setBrowserUrl(t.urlAfterRedirects, t);
640670
}
641671
this.browserUrlTree = t.urlAfterRedirects;
642672
}
@@ -731,11 +761,7 @@ export class Router {
731761

732762
filter(t => {
733763
if (!t.guardsResult) {
734-
this.resetUrlToCurrentUrlTree();
735-
const navCancel =
736-
new NavigationCancel(t.id, this.serializeUrl(t.extractedUrl), '');
737-
eventsSubject.next(navCancel);
738-
t.resolve(false);
764+
this.cancelNavigationTransition(t, '');
739765
return false;
740766
}
741767
return true;
@@ -760,11 +786,9 @@ export class Router {
760786
next: () => dataResolved = true,
761787
complete: () => {
762788
if (!dataResolved) {
763-
const navCancel = new NavigationCancel(
764-
t.id, this.serializeUrl(t.extractedUrl),
789+
this.cancelNavigationTransition(
790+
t,
765791
`At least one route resolver didn't emit any value.`);
766-
eventsSubject.next(navCancel);
767-
t.resolve(false);
768792
}
769793
}
770794
}),
@@ -818,8 +842,7 @@ export class Router {
818842

819843
if (this.urlUpdateStrategy === 'deferred') {
820844
if (!t.extras.skipLocationChange) {
821-
this.setBrowserUrl(
822-
this.rawUrlTree, !!t.extras.replaceUrl, t.id, t.extras.state);
845+
this.setBrowserUrl(this.rawUrlTree, t);
823846
}
824847
this.browserUrlTree = t.urlAfterRedirects;
825848
}
@@ -853,13 +876,10 @@ export class Router {
853876
// sync code which looks for a value here in order to determine whether or
854877
// not to handle a given popstate event or to leave it to the Angular
855878
// router.
856-
this.resetUrlToCurrentUrlTree();
857-
const navCancel = new NavigationCancel(
858-
t.id, this.serializeUrl(t.extractedUrl),
879+
this.cancelNavigationTransition(
880+
t,
859881
`Navigation ID ${t.id} is not equal to the current navigation id ${
860882
this.navigationId}`);
861-
eventsSubject.next(navCancel);
862-
t.resolve(false);
863883
}
864884
// currentNavigation should always be reset to null here. If navigation was
865885
// successful, lastSuccessfulTransition will have already been set. Therefore
@@ -982,6 +1002,7 @@ export class Router {
9821002
if (state) {
9831003
const stateCopy = {...state} as Partial<RestoredState>;
9841004
delete stateCopy.navigationId;
1005+
delete stateCopy.ɵrouterPageId;
9851006
if (Object.keys(stateCopy).length !== 0) {
9861007
extras.state = stateCopy;
9871008
}
@@ -1192,7 +1213,15 @@ export class Router {
11921213
const urlTree = isUrlTree(url) ? url : this.parseUrl(url);
11931214
const mergedTree = this.urlHandlingStrategy.merge(urlTree, this.rawUrlTree);
11941215

1195-
return this.scheduleNavigation(mergedTree, 'imperative', null, extras);
1216+
let restoredState: RestoredState|null = null;
1217+
if (this.canceledNavigationResolution === 'computed') {
1218+
const isInitialPage = this.currentPageId === 0;
1219+
if (isInitialPage || extras.skipLocationChange || extras.replaceUrl) {
1220+
restoredState = this.location.getState() as RestoredState | null;
1221+
}
1222+
}
1223+
1224+
return this.scheduleNavigation(mergedTree, 'imperative', restoredState, extras);
11961225
}
11971226

11981227
/**
@@ -1297,6 +1326,7 @@ export class Router {
12971326
t => {
12981327
this.navigated = true;
12991328
this.lastSuccessfulId = t.id;
1329+
this.currentPageId = t.targetPageId;
13001330
(this.events as Subject<Event>)
13011331
.next(new NavigationEnd(
13021332
t.id, this.serializeUrl(t.extractedUrl), this.serializeUrl(this.currentUrlTree)));
@@ -1356,8 +1386,23 @@ export class Router {
13561386
}
13571387

13581388
const id = ++this.navigationId;
1389+
let targetPageId: number;
1390+
if (this.canceledNavigationResolution === 'computed') {
1391+
// If the `ɵrouterPageId` exist in the state then `targetpageId` should have the value of
1392+
// `ɵrouterPageId`
1393+
if (restoredState && restoredState.ɵrouterPageId) {
1394+
targetPageId = restoredState.ɵrouterPageId;
1395+
} else {
1396+
targetPageId = this.currentPageId + 1;
1397+
}
1398+
} else {
1399+
// This is unused when `canceledNavigationResolution` is not computed.
1400+
targetPageId = 0;
1401+
}
1402+
13591403
this.setTransition({
13601404
id,
1405+
targetPageId,
13611406
source,
13621407
restoredState,
13631408
currentUrlTree: this.currentUrlTree,
@@ -1378,15 +1423,13 @@ export class Router {
13781423
});
13791424
}
13801425

1381-
private setBrowserUrl(
1382-
url: UrlTree, replaceUrl: boolean, id: number, state?: {[key: string]: any}) {
1426+
private setBrowserUrl(url: UrlTree, t: NavigationTransition) {
13831427
const path = this.urlSerializer.serialize(url);
1384-
state = state || {};
1385-
if (this.location.isCurrentPathEqualTo(path) || replaceUrl) {
1386-
// TODO(jasonaden): Remove first `navigationId` and rely on `ng` namespace.
1387-
this.location.replaceState(path, '', {...state, navigationId: id});
1428+
const state = {...t.extras.state, ...this.generateNgRouterState(t.id, t.targetPageId)};
1429+
if (this.location.isCurrentPathEqualTo(path) || !!t.extras.replaceUrl) {
1430+
this.location.replaceState(path, '', state);
13881431
} else {
1389-
this.location.go(path, '', {...state, navigationId: id});
1432+
this.location.go(path, '', state);
13901433
}
13911434
}
13921435

@@ -1399,7 +1442,44 @@ export class Router {
13991442

14001443
private resetUrlToCurrentUrlTree(): void {
14011444
this.location.replaceState(
1402-
this.urlSerializer.serialize(this.rawUrlTree), '', {navigationId: this.lastSuccessfulId});
1445+
this.urlSerializer.serialize(this.rawUrlTree), '',
1446+
this.generateNgRouterState(this.lastSuccessfulId, this.currentPageId));
1447+
}
1448+
1449+
/**
1450+
* Responsible for handling the cancellation of a navigation:
1451+
* - performs the necessary rollback action to restore the browser URL to the
1452+
* state before the transition
1453+
* - triggers the `NavigationCancel` event
1454+
* - resolves the transition promise with `false`
1455+
*/
1456+
private cancelNavigationTransition(t: NavigationTransition, reason: string) {
1457+
if (this.canceledNavigationResolution === 'computed') {
1458+
// The navigator change the location before triggered the browser event,
1459+
// so we need to go back to the current url if the navigation is canceled.
1460+
// Also, when navigation gets cancelled while using url update strategy eager, then we need to
1461+
// go back. Because, when `urlUpdateSrategy` is `eager`; `setBrowserUrl` method is called
1462+
// before any verification.
1463+
if (t.source === 'popstate' || this.urlUpdateStrategy === 'eager') {
1464+
const targetPagePosition = this.currentPageId - t.targetPageId;
1465+
this.location.historyGo(targetPagePosition);
1466+
} else {
1467+
// If update is not 'eager' and the transition navigation source isn't 'popstate', then the
1468+
// navigation was cancelled before any browser url change so nothing needs to be restored.
1469+
}
1470+
} else {
1471+
this.resetUrlToCurrentUrlTree();
1472+
}
1473+
const navCancel = new NavigationCancel(t.id, this.serializeUrl(t.extractedUrl), reason);
1474+
this.triggerEvent(navCancel);
1475+
t.resolve(false);
1476+
}
1477+
1478+
private generateNgRouterState(navigationId: number, routerPageId?: number) {
1479+
if (this.canceledNavigationResolution === 'computed') {
1480+
return {navigationId, ɵrouterPageId: routerPageId};
1481+
}
1482+
return {navigationId};
14031483
}
14041484
}
14051485

0 commit comments

Comments
 (0)