diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts index 7e9bb3f2d3..6ccfd0bf9f 100644 --- a/packages/router/src/router.ts +++ b/packages/router/src/router.ts @@ -1127,20 +1127,11 @@ export class Router { extras: NavigationExtras, priorPromise?: {resolve: any, reject: any, promise: Promise}): Promise { const lastNavigation = this.getTransition(); - // * Imperative navigations (router.navigate) might trigger additional navigations to the same - // URL via a popstate event and the locationChangeListener. We should skip these duplicate - // navs. - // * Imperative navigations can be cancelled by router guards, meaning the URL won't change. If - // the user follows that with a navigation using the back/forward button or manual URL change, - // the destination may be the same as the previous imperative attempt. We should not skip - // these navigations because it's a separate case from the one above -- it's not a duplicate - // navigation. - // Note that imperative navs might only trigger a popstate in tests because the - // SpyLocation triggers it on replaceState. Real browsers don't; see #27059. - const navigationToSameUrl = lastNavigation.urlAfterRedirects.toString() === rawUrl.toString(); - const browserNavPrecededByRouterNav = - lastNavigation && source !== 'imperative' && lastNavigation.source === 'imperative'; - if (browserNavPrecededByRouterNav && navigationToSameUrl) { + // 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 && source !== 'imperative' && lastNavigation.source === 'imperative' && + lastNavigation.rawUrl.toString() === rawUrl.toString()) { return Promise.resolve(true); // return value is not used } diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts index 09e9d43f7d..5e72ee6920 100644 --- a/packages/router/test/integration.spec.ts +++ b/packages/router/test/integration.spec.ts @@ -2539,161 +2539,6 @@ describe('Integration', () => { }))); }); - describe('should not break the history', () => { - @Injectable({providedIn: 'root'}) - class MyGuard implements CanDeactivate { - allow: boolean = true; - canDeactivate(): boolean { - return this.allow; - } - } - - @Component({selector: 'parent', template: ''}) - class Parent { - } - - @Component({selector: 'home', template: 'home'}) - class Home { - } - - @Component({selector: 'child1', template: 'child1'}) - class Child1 { - } - - @Component({selector: 'child2', template: 'child2'}) - class Child2 { - } - - @Component({selector: 'child3', template: 'child3'}) - class Child3 { - } - - @Component({selector: 'child4', template: 'child4'}) - class Child4 { - } - - @Component({selector: 'child5', template: 'child5'}) - class Child5 { - } - - @NgModule({ - declarations: [Parent, Home, Child1, Child2, Child3, Child4, Child5], - entryComponents: [Child1, Child2, Child3, Child4, Child5], - imports: [RouterModule] - }) - class TestModule { - } - - let fixture: ComponentFixture; - - beforeEach(fakeAsync(() => { - TestBed.configureTestingModule({imports: [TestModule]}); - const router = TestBed.get(Router); - const location = TestBed.get(Location); - fixture = createRoot(router, Parent); - - router.resetConfig([ - {path: '', component: Home}, - {path: 'first', component: Child1}, - {path: 'second', component: Child2}, - {path: 'third', component: Child3, canDeactivate: [MyGuard]}, - {path: 'fourth', component: Child4}, - {path: 'fifth', component: Child5}, - ]); - - // Create a navigation history of pages 1-5, and go back to 3 so that there is both - // back and forward history. - router.navigateByUrl('/first'); - advance(fixture); - router.navigateByUrl('/second'); - advance(fixture); - router.navigateByUrl('/third'); - advance(fixture); - router.navigateByUrl('/fourth'); - advance(fixture); - router.navigateByUrl('/fifth'); - advance(fixture); - location.back(); - advance(fixture); - location.back(); - advance(fixture); - })); - - // TODO(https://github.com/angular/angular/issues/13586) - // A fix to this requires much more design - xit('when navigate back using Back button', fakeAsync(() => { - const location = TestBed.get(Location); - expect(location.path()).toEqual('/third'); - - TestBed.get(MyGuard).allow = false; - location.back(); - advance(fixture); - expect(location.path()).toEqual('/third'); - expect(fixture.nativeElement).toHaveText('child3'); - - TestBed.get(MyGuard).allow = true; - location.back(); - advance(fixture); - expect(location.path()).toEqual('/second'); - expect(fixture.nativeElement).toHaveText('child2'); - })); - - it('when navigate back imperatively', fakeAsync(() => { - const router = TestBed.get(Router); - const location = TestBed.get(Location); - expect(location.path()).toEqual('/third'); - - TestBed.get(MyGuard).allow = false; - router.navigateByUrl('/second'); - advance(fixture); - expect(location.path()).toEqual('/third'); - expect(fixture.nativeElement).toHaveText('child3'); - - TestBed.get(MyGuard).allow = true; - location.back(); - advance(fixture); - expect(location.path()).toEqual('/second'); - expect(fixture.nativeElement).toHaveText('child2'); - })); - - // TODO(https://github.com/angular/angular/issues/13586) - // A fix to this requires much more design - xit('when navigate back using Foward button', fakeAsync(() => { - const location = TestBed.get(Location); - expect(location.path()).toEqual('/third'); - - TestBed.get(MyGuard).allow = false; - location.forward(); - advance(fixture); - expect(location.path()).toEqual('/third'); - expect(fixture.nativeElement).toHaveText('child3'); - - TestBed.get(MyGuard).allow = true; - location.forward(); - advance(fixture); - expect(location.path()).toEqual('/fourth'); - expect(fixture.nativeElement).toHaveText('child4'); - })); - - it('when navigate forward imperatively', fakeAsync(() => { - const router = TestBed.get(Router); - const location = TestBed.get(Location); - expect(location.path()).toEqual('/third'); - - TestBed.get(MyGuard).allow = false; - router.navigateByUrl('/fourth'); - advance(fixture); - expect(location.path()).toEqual('/third'); - expect(fixture.nativeElement).toHaveText('child3'); - - TestBed.get(MyGuard).allow = true; - location.forward(); - advance(fixture); - expect(location.path()).toEqual('/fourth'); - expect(fixture.nativeElement).toHaveText('child4'); - })); - }); - describe('should redirect when guard returns UrlTree', () => { beforeEach(() => TestBed.configureTestingModule({ providers: [