From 04c6b2fe858a68b7eb8d974696717416b4c6f31d Mon Sep 17 00:00:00 2001 From: Victor Savkin Date: Fri, 12 Aug 2016 14:30:51 -0700 Subject: [PATCH] fix(router): location changes and redirects break the back button (#10742) --- .../@angular/common/testing/location_mock.ts | 1 + modules/@angular/router/src/router.ts | 16 +++++--- .../@angular/router/test/integration.spec.ts | 38 +++++++++++++++++++ tools/public_api_guard/router/index.d.ts | 1 + 4 files changed, 50 insertions(+), 6 deletions(-) diff --git a/modules/@angular/common/testing/location_mock.ts b/modules/@angular/common/testing/location_mock.ts index 5c423ebe6a..24baad5943 100644 --- a/modules/@angular/common/testing/location_mock.ts +++ b/modules/@angular/common/testing/location_mock.ts @@ -77,6 +77,7 @@ export class SpyLocation implements Location { var url = path + (query.length > 0 ? ('?' + query) : ''); this.urlChanges.push(url); + this._subject.emit({'url': url, 'pop': false}); } replaceState(path: string, query: string = '') { diff --git a/modules/@angular/router/src/router.ts b/modules/@angular/router/src/router.ts index 73913066fc..219ba22262 100644 --- a/modules/@angular/router/src/router.ts +++ b/modules/@angular/router/src/router.ts @@ -47,6 +47,7 @@ export interface NavigationExtras { preserveQueryParams?: boolean; preserveFragment?: boolean; skipLocationChange?: boolean; + replaceUrl?: boolean; } /** @@ -160,7 +161,7 @@ export class Router { */ initialNavigation(): void { this.setUpLocationChangeListener(); - this.navigateByUrl(this.location.path(true)); + this.navigateByUrl(this.location.path(true), {replaceUrl: true}); } /** @@ -336,7 +337,8 @@ export class Router { private scheduleNavigation(url: UrlTree, extras: NavigationExtras): Promise { const id = ++this.navigationId; this.routerEvents.next(new NavigationStart(id, this.serializeUrl(url))); - return Promise.resolve().then((_) => this.runNavigate(url, extras.skipLocationChange, id)); + return Promise.resolve().then( + (_) => this.runNavigate(url, extras.skipLocationChange, extras.replaceUrl, id)); } private setUpLocationChangeListener(): void { @@ -347,12 +349,14 @@ export class Router { // we fire multiple events for a single URL change // we should navigate only once return this.currentUrlTree.toString() !== tree.toString() ? - this.scheduleNavigation(tree, change['pop']) : + this.scheduleNavigation(tree, {skipLocationChange: change['pop'], replaceUrl: true}) : null; })); } - private runNavigate(url: UrlTree, preventPushState: boolean, id: number): Promise { + private runNavigate( + url: UrlTree, shouldPreventPushState: boolean, shouldReplaceUrl: boolean, + id: number): Promise { if (id !== this.navigationId) { this.location.go(this.urlSerializer.serialize(this.currentUrlTree)); this.routerEvents.next(new NavigationCancel(id, this.serializeUrl(url))); @@ -415,9 +419,9 @@ export class Router { new ActivateRoutes(state, storedState).activate(this.outletMap); - if (!preventPushState) { + if (!shouldPreventPushState) { let path = this.urlSerializer.serialize(appliedUrl); - if (this.location.isCurrentPathEqualTo(path)) { + if (this.location.isCurrentPathEqualTo(path) || shouldReplaceUrl) { this.location.replaceState(path); } else { this.location.go(path); diff --git a/modules/@angular/router/test/integration.spec.ts b/modules/@angular/router/test/integration.spec.ts index 1b7fe696a0..ea24355594 100644 --- a/modules/@angular/router/test/integration.spec.ts +++ b/modules/@angular/router/test/integration.spec.ts @@ -872,6 +872,44 @@ describe('Integration', () => { expect(location.path()).toEqual('/team/22'); }))); + + it('should not break the back button when trigger by location change', + fakeAsync(inject( + [Router, TestComponentBuilder, Location], + (router: Router, tcb: TestComponentBuilder, location: Location) => { + const fixture = tcb.createFakeAsync(RootCmp); + advance(fixture); + router.resetConfig([ + {path: 'initial', component: BlankCmp}, + {path: 'old/team/:id', redirectTo: 'team/:id'}, + {path: 'team/:id', component: TeamCmp} + ]); + + location.go('initial'); + location.go('old/team/22'); + + // initial navigation + router.initialNavigation(); + advance(fixture); + expect(location.path()).toEqual('/team/22'); + + location.back(); + advance(fixture); + expect(location.path()).toEqual('/initial'); + + // location change + (location).go('/old/team/33'); + + + advance(fixture); + expect(location.path()).toEqual('/team/33'); + + location.back(); + advance(fixture); + expect(location.path()).toEqual('/initial'); + }))); + + // should not break the back button when trigger by initial navigation }); describe('guards', () => { diff --git a/tools/public_api_guard/router/index.d.ts b/tools/public_api_guard/router/index.d.ts index 3fe43037a9..f6afd7b843 100644 --- a/tools/public_api_guard/router/index.d.ts +++ b/tools/public_api_guard/router/index.d.ts @@ -108,6 +108,7 @@ export interface NavigationExtras { preserveQueryParams?: boolean; queryParams?: Params; relativeTo?: ActivatedRoute; + replaceUrl?: boolean; skipLocationChange?: boolean; }