From 2bf21e174747052c7628eb9c50654fc1738a17a2 Mon Sep 17 00:00:00 2001 From: Rene Weber Date: Fri, 8 Apr 2016 01:02:07 +0100 Subject: [PATCH] fix(Router): replace state when normalized path is equal to current normalized path Make sure the same path is not added multiple times to the history. It is replacing the state, instead of skipping it completely, because the current path in the browser might not be normalized, while the given one is normalized. Closes #7829 Closes #7897 --- .../@angular/common/src/location/location.ts | 7 +++++++ .../@angular/common/testing/location_mock.ts | 17 +++++++++++++++-- .../@angular/router-deprecated/src/router.ts | 6 +++++- .../test/integration/navigation_spec.ts | 16 ++++++++++++++++ modules/angular1_router/lib/facades.es5 | 9 +++++++++ 5 files changed, 52 insertions(+), 3 deletions(-) diff --git a/modules/@angular/common/src/location/location.ts b/modules/@angular/common/src/location/location.ts index 9e9acdd750..284d4c650f 100644 --- a/modules/@angular/common/src/location/location.ts +++ b/modules/@angular/common/src/location/location.ts @@ -63,6 +63,13 @@ export class Location { */ path(): string { return this.normalize(this.platformStrategy.path()); } + /** + * Normalizes the given path and compares to the current normalized path. + */ + isCurrentPathEqualTo(path: string, query: string = ''): boolean { + return this.path() == this.normalize(path + Location.normalizeQueryParams(query)); + } + /** * Given a string representing a URL, returns the normalized URL path without leading or * trailing slashes diff --git a/modules/@angular/common/testing/location_mock.ts b/modules/@angular/common/testing/location_mock.ts index 85d05ab4eb..f5f9f7505c 100644 --- a/modules/@angular/common/testing/location_mock.ts +++ b/modules/@angular/common/testing/location_mock.ts @@ -23,6 +23,14 @@ export class SpyLocation implements Location { path(): string { return this._history[this._historyIndex].path; } + isCurrentPathEqualTo(path: string, query: string = ''): boolean { + var givenPath = path.endsWith('/') ? path.substring(0, path.length - 1) : path; + var currPath = + this.path().endsWith('/') ? this.path().substring(0, this.path().length - 1) : this.path(); + + return currPath == givenPath + (query.length > 0 ? ('?' + query) : ''); + } + simulateUrlPop(pathname: string) { ObservableWrapper.callEmit(this._subject, {'url': pathname, 'pop': true}); } @@ -62,8 +70,13 @@ export class SpyLocation implements Location { replaceState(path: string, query: string = '') { path = this.prepareExternalUrl(path); - this._history[this._historyIndex].path = path; - this._history[this._historyIndex].query = query; + var history = this._history[this._historyIndex]; + if (history.path == path && history.query == query) { + return; + } + + history.path = path; + history.query = query; var url = path + (query.length > 0 ? ('?' + query) : ''); this.urlChanges.push('replace: ' + url); diff --git a/modules/@angular/router-deprecated/src/router.ts b/modules/@angular/router-deprecated/src/router.ts index 62492b1da2..3448571926 100644 --- a/modules/@angular/router-deprecated/src/router.ts +++ b/modules/@angular/router-deprecated/src/router.ts @@ -526,7 +526,11 @@ export class RootRouter extends Router { } var promise = super.commit(instruction); if (!_skipLocationChange) { - promise = promise.then((_) => { this._location.go(emitPath, emitQuery); }); + if (this._location.isCurrentPathEqualTo(emitPath, emitQuery)) { + promise = promise.then((_) => { this._location.replaceState(emitPath, emitQuery); }); + } else { + promise = promise.then((_) => { this._location.go(emitPath, emitQuery); }); + } } return promise; } diff --git a/modules/@angular/router-deprecated/test/integration/navigation_spec.ts b/modules/@angular/router-deprecated/test/integration/navigation_spec.ts index 8d4a7471e2..33f7a1a03a 100644 --- a/modules/@angular/router-deprecated/test/integration/navigation_spec.ts +++ b/modules/@angular/router-deprecated/test/integration/navigation_spec.ts @@ -130,6 +130,22 @@ export function main() { }); })); + + it('should replace state when normalized paths are equal', + inject([AsyncTestCompleter, Location], (async, location) => { + compile(tcb) + .then((rtc) => {fixture = rtc}) + .then((_) => location.setInitialPath("/test/")) + .then((_) => rtr.config([new Route({path: '/test', component: HelloCmp})])) + .then((_) => rtr.navigateByUrl('/test')) + .then((_) => { + fixture.detectChanges(); + expect(fixture.debugElement.nativeElement).toHaveText('hello'); + expect(location.urlChanges).toEqual(['replace: /test']); + async.done(); + }); + })); + it('should reuse common parent components', inject([AsyncTestCompleter], (async) => { compile(tcb) .then((rtc) => {fixture = rtc}) diff --git a/modules/angular1_router/lib/facades.es5 b/modules/angular1_router/lib/facades.es5 index 1744f10d53..b98a78aa13 100644 --- a/modules/angular1_router/lib/facades.es5 +++ b/modules/angular1_router/lib/facades.es5 @@ -313,9 +313,18 @@ Location.prototype.subscribe = function () { Location.prototype.path = function () { return $location.url(); }; +Location.prototype.isCurrentPathEqualTo = function (path, query) { + if (query === void 0) { query = ''; } + return this.path() === (path + query); +}; Location.prototype.go = function (path, query) { return $location.url(path + query); }; +Location.prototype.replaceState = function (path, query) { + if (query === void 0) { query = ''; } + $location.url(path + query); + $location.replace(); +}; Location.prototype.prepareExternalUrl = function(url) { if (url.length > 0 && !url.startsWith('/')) { url = '/' + url;