fix(router): make setUpLocationChangeListener idempotent

This commit is contained in:
vsavkin 2016-11-22 14:50:52 -08:00 committed by Alex Rickabaugh
parent 307c4693dc
commit 25e5b2fdf0
2 changed files with 46 additions and 16 deletions

View File

@ -382,23 +382,25 @@ export class Router {
setUpLocationChangeListener(): void { setUpLocationChangeListener(): void {
// Zone.current.wrap is needed because of the issue with RxJS scheduler, // Zone.current.wrap is needed because of the issue with RxJS scheduler,
// which does not work properly with zone.js in IE and Safari // which does not work properly with zone.js in IE and Safari
this.locationSubscription = <any>this.location.subscribe(Zone.current.wrap((change: any) => { if (!this.locationSubscription) {
const rawUrlTree = this.urlSerializer.parse(change['url']); this.locationSubscription = <any>this.location.subscribe(Zone.current.wrap((change: any) => {
const lastNavigation = this.navigations.value; const rawUrlTree = this.urlSerializer.parse(change['url']);
const lastNavigation = this.navigations.value;
// If the user triggers a navigation imperatively (e.g., by using navigateByUrl), // If the user triggers a navigation imperatively (e.g., by using navigateByUrl),
// and that navigation results in 'replaceState' that leads to the same URL, // and that navigation results in 'replaceState' that leads to the same URL,
// we should skip those. // we should skip those.
if (lastNavigation && lastNavigation.imperative && if (lastNavigation && lastNavigation.imperative &&
lastNavigation.rawUrl.toString() === rawUrlTree.toString()) { lastNavigation.rawUrl.toString() === rawUrlTree.toString()) {
return; return;
} }
setTimeout(() => { setTimeout(() => {
this.scheduleNavigation( this.scheduleNavigation(
rawUrlTree, false, {skipLocationChange: change['pop'], replaceUrl: true}); rawUrlTree, false, {skipLocationChange: change['pop'], replaceUrl: true});
}, 0); }, 0);
})); }));
}
} }
/** /**
@ -443,7 +445,12 @@ export class Router {
/** /**
* Disposes of the router. * Disposes of the router.
*/ */
dispose(): void { this.locationSubscription.unsubscribe(); } dispose(): void {
if (this.locationSubscription) {
this.locationSubscription.unsubscribe();
this.locationSubscription = null;
}
}
/** /**
* Applies an array of commands to the current url tree and creates a new url tree. * Applies an array of commands to the current url tree and creates a new url tree.

View File

@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import {Location} from '@angular/common';
import {TestBed} from '@angular/core/testing'; import {TestBed} from '@angular/core/testing';
import {ResolveData} from '../src/config'; import {ResolveData} from '../src/config';
@ -32,6 +33,28 @@ describe('Router', () => {
}); });
}); });
describe('setUpLocationChangeListener', () => {
beforeEach(() => { TestBed.configureTestingModule({imports: [RouterTestingModule]}); });
it('should be indempotent', () => {
const r: Router = TestBed.get(Router);
const location: Location = TestBed.get(Location);
r.setUpLocationChangeListener();
const a = (<any>r).locationSubscription;
r.setUpLocationChangeListener();
const b = (<any>r).locationSubscription;
expect(a).toBe(b);
r.dispose();
r.setUpLocationChangeListener();
const c = (<any>r).locationSubscription;
expect(c).not.toBe(b);
});
});
describe('PreActivation', () => { describe('PreActivation', () => {
const serializer = new DefaultUrlSerializer(); const serializer = new DefaultUrlSerializer();
const inj = {get: (token: any) => () => `${token}_value`}; const inj = {get: (token: any) => () => `${token}_value`};