diff --git a/modules/@angular/router/src/router.ts b/modules/@angular/router/src/router.ts index 5b5cd3551b320..7425cdf783167 100644 --- a/modules/@angular/router/src/router.ts +++ b/modules/@angular/router/src/router.ts @@ -281,11 +281,11 @@ function defaultErrorHandler(error: any): any { type NavigationParams = { id: number, rawUrl: UrlTree, - prevRawUrl: UrlTree, extras: NavigationExtras, resolve: any, reject: any, - promise: Promise + promise: Promise, + imperative: boolean }; @@ -387,8 +387,19 @@ export class Router { // which does not work properly with zone.js in IE and Safari this.locationSubscription = this.location.subscribe(Zone.current.wrap((change: any) => { const rawUrlTree = this.urlSerializer.parse(change['url']); + const lastNavigation = this.navigations.value; + + // If the user triggers a navigation imperatively (e.g., by using navigateByUrl), + // and that navigation results in 'replaceState' that leads to the same URL, + // we should skip those. + if (lastNavigation && lastNavigation.imperative && + lastNavigation.rawUrl.toString() === rawUrlTree.toString()) { + return; + } + setTimeout(() => { - this.scheduleNavigation(rawUrlTree, {skipLocationChange: change['pop'], replaceUrl: true}); + this.scheduleNavigation( + rawUrlTree, false, {skipLocationChange: change['pop'], replaceUrl: true}); }, 0); })); } @@ -510,11 +521,12 @@ export class Router { navigateByUrl(url: string|UrlTree, extras: NavigationExtras = {skipLocationChange: false}): Promise { if (url instanceof UrlTree) { - return this.scheduleNavigation(this.urlHandlingStrategy.merge(url, this.rawUrlTree), extras); + return this.scheduleNavigation( + this.urlHandlingStrategy.merge(url, this.rawUrlTree), true, extras); } else { const urlTree = this.urlSerializer.parse(url); return this.scheduleNavigation( - this.urlHandlingStrategy.merge(urlTree, this.rawUrlTree), extras); + this.urlHandlingStrategy.merge(urlTree, this.rawUrlTree), true, extras); } } @@ -596,12 +608,8 @@ export class Router { .subscribe(() => {}); } - private scheduleNavigation(rawUrl: UrlTree, extras: NavigationExtras): Promise { - const prevRawUrl = this.navigations.value ? this.navigations.value.rawUrl : null; - if (prevRawUrl && prevRawUrl.toString() === rawUrl.toString()) { - return this.navigations.value.promise; - } - + private scheduleNavigation(rawUrl: UrlTree, imperative: boolean, extras: NavigationExtras): + Promise { let resolve: any = null; let reject: any = null; @@ -611,18 +619,17 @@ export class Router { }); const id = ++this.navigationId; - this.navigations.next({id, rawUrl, prevRawUrl, extras, resolve, reject, promise}); + this.navigations.next({id, imperative, rawUrl, extras, resolve, reject, promise}); // Make sure that the error is propagated even though `processNavigations` catch // handler does not rethrow return promise.catch((e: any) => Promise.reject(e)); } - private executeScheduledNavigation({id, rawUrl, prevRawUrl, extras, resolve, - reject}: NavigationParams): void { + private executeScheduledNavigation({id, rawUrl, extras, resolve, reject}: NavigationParams): + void { const url = this.urlHandlingStrategy.extract(rawUrl); - const prevUrl = prevRawUrl ? this.urlHandlingStrategy.extract(prevRawUrl) : null; - const urlTransition = !prevUrl || url.toString() !== prevUrl.toString(); + const urlTransition = !this.navigated || url.toString() !== this.currentUrlTree.toString(); if (urlTransition && this.urlHandlingStrategy.shouldProcessUrl(rawUrl)) { this.routerEvents.next(new NavigationStart(id, this.serializeUrl(url))); @@ -635,7 +642,8 @@ export class Router { // we cannot process the current URL, but we could process the previous one => // we need to do some cleanup } else if ( - urlTransition && prevRawUrl && this.urlHandlingStrategy.shouldProcessUrl(prevRawUrl)) { + urlTransition && this.rawUrlTree && + this.urlHandlingStrategy.shouldProcessUrl(this.rawUrlTree)) { this.routerEvents.next(new NavigationStart(id, this.serializeUrl(url))); Promise.resolve() .then( diff --git a/modules/@angular/router/test/integration.spec.ts b/modules/@angular/router/test/integration.spec.ts index 897d5451f5e60..0ffd0709a9f27 100644 --- a/modules/@angular/router/test/integration.spec.ts +++ b/modules/@angular/router/test/integration.spec.ts @@ -1383,6 +1383,13 @@ describe('Integration', () => { useValue: (c: any, a: ActivatedRouteSnapshot, b: RouterStateSnapshot) => { return false; } }, + { + provide: 'alwaysFalseAndLogging', + useValue: (c: any, a: ActivatedRouteSnapshot, b: RouterStateSnapshot) => { + log.push('called'); + return false; + } + }, ] }); }); @@ -1532,62 +1539,86 @@ describe('Integration', () => { expect(location.path()).toEqual('/main/component1'); }))); - }); - - describe('should work when given a class', () => { - class AlwaysTrue implements CanDeactivate { - canDeactivate( - component: TeamCmp, route: ActivatedRouteSnapshot, - state: RouterStateSnapshot): boolean { - return true; - } - } + it('should call guards every time when navigating to the same url over and over again', + fakeAsync(inject([Router, Location], (router: Router, location: Location) => { + const fixture = createRoot(router, RootCmp); - beforeEach(() => { TestBed.configureTestingModule({providers: [AlwaysTrue]}); }); + router.resetConfig([ + {path: 'simple', component: SimpleCmp, canDeactivate: ['alwaysFalseAndLogging']}, + {path: 'blank', component: BlankCmp} - it('works', fakeAsync(inject([Router, Location], (router: Router, location: Location) => { - const fixture = createRoot(router, RootCmp); + ]); - router.resetConfig( - [{path: 'team/:id', component: TeamCmp, canDeactivate: [AlwaysTrue]}]); + router.navigateByUrl('/simple'); + advance(fixture); - router.navigateByUrl('/team/22'); + router.navigateByUrl('/blank'); advance(fixture); - expect(location.path()).toEqual('/team/22'); + expect(log).toEqual(['called']); + expect(location.path()).toEqual('/simple'); - router.navigateByUrl('/team/33'); + router.navigateByUrl('/blank'); advance(fixture); - expect(location.path()).toEqual('/team/33'); + expect(log).toEqual(['called', 'called']); + expect(location.path()).toEqual('/simple'); }))); - }); - describe('should work when returns an observable', () => { - beforeEach(() => { - TestBed.configureTestingModule({ - providers: [{ - provide: 'CanDeactivate', - useValue: (c: TeamCmp, a: ActivatedRouteSnapshot, b: RouterStateSnapshot) => { - return Observable.create((observer: any) => { observer.next(false); }); - } - }] - }); + describe('should work when given a class', () => { + class AlwaysTrue implements CanDeactivate { + canDeactivate( + component: TeamCmp, route: ActivatedRouteSnapshot, + state: RouterStateSnapshot): boolean { + return true; + } + } + + beforeEach(() => { TestBed.configureTestingModule({providers: [AlwaysTrue]}); }); + + it('works', fakeAsync(inject([Router, Location], (router: Router, location: Location) => { + const fixture = createRoot(router, RootCmp); + + router.resetConfig( + [{path: 'team/:id', component: TeamCmp, canDeactivate: [AlwaysTrue]}]); + + router.navigateByUrl('/team/22'); + advance(fixture); + expect(location.path()).toEqual('/team/22'); + + router.navigateByUrl('/team/33'); + advance(fixture); + expect(location.path()).toEqual('/team/33'); + }))); }); - it('works', fakeAsync(inject([Router, Location], (router: Router, location: Location) => { - const fixture = createRoot(router, RootCmp); - router.resetConfig( - [{path: 'team/:id', component: TeamCmp, canDeactivate: ['CanDeactivate']}]); + describe('should work when returns an observable', () => { + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [{ + provide: 'CanDeactivate', + useValue: (c: TeamCmp, a: ActivatedRouteSnapshot, b: RouterStateSnapshot) => { + return Observable.create((observer: any) => { observer.next(false); }); + } + }] + }); + }); - router.navigateByUrl('/team/22'); - advance(fixture); - expect(location.path()).toEqual('/team/22'); + it('works', fakeAsync(inject([Router, Location], (router: Router, location: Location) => { + const fixture = createRoot(router, RootCmp); - router.navigateByUrl('/team/33'); - advance(fixture); - expect(location.path()).toEqual('/team/22'); - }))); + router.resetConfig( + [{path: 'team/:id', component: TeamCmp, canDeactivate: ['CanDeactivate']}]); + + router.navigateByUrl('/team/22'); + advance(fixture); + expect(location.path()).toEqual('/team/22'); + + router.navigateByUrl('/team/33'); + advance(fixture); + expect(location.path()).toEqual('/team/22'); + }))); + }); }); }); @@ -1826,6 +1857,7 @@ describe('Integration', () => { TestBed.configureTestingModule({declarations: [RootCmpWithLink]}); const router: Router = TestBed.get(Router); + const loc: any = TestBed.get(Location); const f = TestBed.createComponent(RootCmpWithLink); advance(f);