fix(router): adjust setting navigationTransition when a new navigation cancels an existing one (#29636)

Prior to this change, if a navigation was ongoing and a new one came in, the router could get into a state where `router.currentNavigation` was `null` even though a navigation was executing. This change moves where we set the `currentNavigation` value so it's inside a `switchMap`. This solves the problem because the `finally` on the `switchMap` had been setting `currentNavigation` to `null` but the new `currentNavigation` value would have already been set. Essentially this was a timing problem and is resolved with this change.

Fixes #29389 #29590

PR Close #29636
This commit is contained in:
Jason Aden 2019-04-01 10:39:24 -07:00
parent d7e5535d00
commit f9497bf28b
2 changed files with 34 additions and 14 deletions

View File

@ -452,25 +452,24 @@ export class Router {
...t, extractedUrl: this.urlHandlingStrategy.extract(t.rawUrl)
} as NavigationTransition)),
// Store the Navigation object
tap(t => {
this.currentNavigation = {
id: t.id,
initialUrl: t.currentRawUrl,
extractedUrl: t.extractedUrl,
trigger: t.source,
extras: t.extras,
previousNavigation: this.lastSuccessfulNavigation ?
{...this.lastSuccessfulNavigation, previousNavigation: null} :
null
};
}),
// Using switchMap so we cancel executing navigations when a new one comes in
switchMap(t => {
let completed = false;
let errored = false;
return of (t).pipe(
// Store the Navigation object
tap(t => {
this.currentNavigation = {
id: t.id,
initialUrl: t.currentRawUrl,
extractedUrl: t.extractedUrl,
trigger: t.source,
extras: t.extras,
previousNavigation: this.lastSuccessfulNavigation ?
{...this.lastSuccessfulNavigation, previousNavigation: null} :
null
};
}),
switchMap(t => {
const urlTransition =
!this.navigated || t.extractedUrl.toString() !== this.browserUrlTree.toString();

View File

@ -1067,6 +1067,27 @@ describe('Integration', () => {
]);
})));
it('should properly set currentNavigation when cancelling in-flight navigations',
fakeAsync(inject([Router], (router: Router) => {
const fixture = createRoot(router, RootCmp);
router.resetConfig([{path: 'user/:name', component: UserCmp}]);
router.navigateByUrl('/user/init');
advance(fixture);
router.navigateByUrl('/user/victor');
expect((router as any).currentNavigation).not.toBe(null);
router.navigateByUrl('/user/fedor');
// Due to https://github.com/angular/angular/issues/29389, this would be `false`
// when running a second navigation.
expect((router as any).currentNavigation).not.toBe(null);
advance(fixture);
expect((router as any).currentNavigation).toBe(null);
expect(fixture.nativeElement).toHaveText('user fedor');
})));
it('should handle failed navigations gracefully', fakeAsync(inject([Router], (router: Router) => {
const fixture = createRoot(router, RootCmp);