diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts index e5c190ea112cf..d9478096fcb41 100644 --- a/packages/router/src/router.ts +++ b/packages/router/src/router.ts @@ -651,13 +651,12 @@ export class Router { }), switchMap(t => { const urlTransition = !this.navigated || - t.extractedUrl.toString() !== this.browserUrlTree.toString(); - /* || this.browserUrlTree.toString() !== this.currentUrlTree.toString() */ - // TODO(atscott): Run TGP to see if the above change can be made. There are - // situations where a navigation is canceled _after_ browserUrlTree is - // updated. For example, urlUpdateStrategy === 'eager': if a new - // navigation happens (i.e. in a guard), this would cause the router to - // be in an invalid state of tracking. + t.extractedUrl.toString() !== this.browserUrlTree.toString() || + // Navigations which succeed or ones which fail and are cleaned up + // correctly should result in `browserUrlTree` and `currentUrlTree` + // matching. If this is not the case, assume something went wrong and try + // processing the URL again. + this.browserUrlTree.toString() !== this.currentUrlTree.toString(); const processCurrentUrl = (this.onSameUrlNavigation === 'reload' ? true : urlTransition) && this.urlHandlingStrategy.shouldProcessUrl(t.rawUrl); diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts index 19c82f0a86d5c..1e46bb41c5c43 100644 --- a/packages/router/test/integration.spec.ts +++ b/packages/router/test/integration.spec.ts @@ -871,17 +871,36 @@ describe('Integration', () => { }))); describe('"eager" urlUpdateStrategy', () => { + @Injectable() + class AuthGuard implements CanActivate { + canActivateResult = true; + + canActivate() { + return this.canActivateResult; + } + } + @Injectable() + class DelayedGuard implements CanActivate { + canActivate() { + return of('').pipe(delay(1000), mapTo(true)); + } + } + beforeEach(() => { const serializer = new DefaultUrlSerializer(); TestBed.configureTestingModule({ - providers: [{ - provide: 'authGuardFail', - useValue: (a: any, b: any) => { - return new Promise(res => { - setTimeout(() => res(serializer.parse('/login')), 1); - }); - } - }] + providers: [ + { + provide: 'authGuardFail', + useValue: (a: any, b: any) => { + return new Promise(res => { + setTimeout(() => res(serializer.parse('/login')), 1); + }); + } + }, + AuthGuard, + DelayedGuard, + ] }); }); @@ -1028,6 +1047,52 @@ describe('Integration', () => { expect(navigation.extras.state).toBeDefined(); expect(navigation.extras.state).toEqual({foo: 'bar'}); }))); + + it('can renavigate to rejected URL', fakeAsync(() => { + const router = TestBed.inject(Router); + const canActivate = TestBed.inject(AuthGuard); + const location = TestBed.inject(Location) as SpyLocation + router.urlUpdateStrategy = 'eager'; + router.resetConfig([ + {path: '', component: BlankCmp}, + {path: 'simple', component: SimpleCmp, canActivate: [AuthGuard]}, + ]); + const fixture = createRoot(router, RootCmp); + + canActivate.canActivateResult = false; + router.navigateByUrl('/simple'); + advance(fixture); + expect(location.path()).toEqual('/'); + expect(fixture.nativeElement.innerHTML).not.toContain('simple'); + + canActivate.canActivateResult = true; + router.navigateByUrl('/simple'); + advance(fixture); + expect(location.path()).toEqual('/simple'); + expect(fixture.nativeElement.innerHTML).toContain('simple'); + })); + + it('can renavigate to same URL during in-flight navigation', fakeAsync(() => { + const router = TestBed.inject(Router); + const location = TestBed.inject(Location) as SpyLocation + router.urlUpdateStrategy = 'eager'; + router.resetConfig([ + {path: '', component: BlankCmp}, + {path: 'simple', component: SimpleCmp, canActivate: [DelayedGuard]}, + ]); + const fixture = createRoot(router, RootCmp); + + router.navigateByUrl('/simple'); + tick(); + // eager update strategy so URL is already updated. + expect(location.path()).toEqual('/simple'); + expect(fixture.nativeElement.innerHTML).not.toContain('simple'); + + router.navigateByUrl('/simple'); + tick(1000); + expect(location.path()).toEqual('/simple'); + expect(fixture.nativeElement.innerHTML).toContain('simple'); + })); }); it('should navigate back and forward', @@ -1568,6 +1633,31 @@ describe('Integration', () => { expect(locationUrlBeforeEmittingError).toEqual('/simple'); })); + it('can renavigate to throwing component', fakeAsync(() => { + const router = TestBed.inject(Router); + const location = TestBed.inject(Location) as SpyLocation + router.urlUpdateStrategy = 'eager'; + router.resetConfig([ + {path: '', component: BlankCmp}, + {path: 'throwing', component: ConditionalThrowingCmp}, + ]); + const fixture = createRoot(router, RootCmp); + + ConditionalThrowingCmp.throwError = true; + expect(() => { + router.navigateByUrl('/throwing'); + advance(fixture); + }).toThrow(); + expect(location.path()).toEqual('/'); + expect(fixture.nativeElement.innerHTML).not.toContain('throwing'); + + ConditionalThrowingCmp.throwError = false; + router.navigateByUrl('/throwing'); + advance(fixture); + expect(location.path()).toEqual('/throwing'); + expect(fixture.nativeElement.innerHTML).toContain('throwing'); + })); + it('should reset the url with the right state when navigation errors', fakeAsync(() => { const router: Router = TestBed.inject(Router); const location = TestBed.inject(Location) as SpyLocation; @@ -6198,6 +6288,15 @@ class ThrowingCmp { throw new Error('Throwing Cmp'); } } +@Component({selector: 'conditional-throwing-cmp', template: 'conditional throwing'}) +class ConditionalThrowingCmp { + static throwError = true; + constructor() { + if (ConditionalThrowingCmp.throwError) { + throw new Error('Throwing Cmp'); + } + } +} @@ -6248,7 +6347,8 @@ class LazyComponent { RootCmpWithTwoOutlets, RootCmpWithNamedOutlet, EmptyQueryParamsCmp, - ThrowingCmp + ThrowingCmp, + ConditionalThrowingCmp, ], @@ -6280,7 +6380,8 @@ class LazyComponent { RootCmpWithTwoOutlets, RootCmpWithNamedOutlet, EmptyQueryParamsCmp, - ThrowingCmp + ThrowingCmp, + ConditionalThrowingCmp, ], @@ -6313,7 +6414,8 @@ class LazyComponent { RootCmpWithTwoOutlets, RootCmpWithNamedOutlet, EmptyQueryParamsCmp, - ThrowingCmp + ThrowingCmp, + ConditionalThrowingCmp, ] }) class TestModule {