Skip to content

Commit

Permalink
fix(router): handle new navigations from a NavigationEnd event (#41262)…
Browse files Browse the repository at this point in the history
… (#41511)

This commit removes the line to set `currentNavigation` to `null` in the
navigation transitions subscription of the router. This logic is
already handled in the `finalize` stage of the transition pipe and has
been found to cause issues if a new navigation is triggered from a
subscription to the `NavigationEnd` event.

fixes #37460

PR Close #41262

PR Close #41511
  • Loading branch information
atscott authored and zarend committed Apr 8, 2021
1 parent 74d1769 commit e34299a
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 4 deletions.
1 change: 0 additions & 1 deletion packages/router/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1245,7 +1245,6 @@ export class Router {
.next(new NavigationEnd(
t.id, this.serializeUrl(t.extractedUrl), this.serializeUrl(this.currentUrlTree)));
this.lastSuccessfulNavigation = this.currentNavigation;
this.currentNavigation = null;
t.resolve(true);
},
e => {
Expand Down
38 changes: 35 additions & 3 deletions packages/router/test/bootstrap.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ describe('bootstrap', () => {
let log: any[] = [];
let testProviders: any[] = null!;

@Component({template: 'simple'})
class SimpleCmp {
}

@Component({selector: 'test-app', template: 'root <router-outlet></router-outlet>'})
class RootCmp {
constructor() {
Expand Down Expand Up @@ -369,7 +373,35 @@ describe('bootstrap', () => {
done();
});

function waitForNavigationToComplete(router: Router): Promise<any> {
return router.events.pipe(filter((e: any) => e instanceof NavigationEnd), first()).toPromise();
}
it('can schedule a navigation from the NavigationEnd event #37460', async (done) => {
@NgModule({
imports: [
BrowserModule,
RouterModule.forRoot(
[
{path: 'a', component: SimpleCmp},
{path: 'b', component: SimpleCmp},
],
)
],
declarations: [RootCmp, SimpleCmp],
bootstrap: [RootCmp],
providers: [...testProviders],
})
class TestModule {
}

const res = await platformBrowserDynamic([]).bootstrapModule(TestModule);
const router = res.injector.get(Router);
router.events.subscribe(() => {
expect(router.getCurrentNavigation()?.id).toBeDefined();
});
router.events.subscribe(async (e) => {
if (e instanceof NavigationEnd && e.url === '/b') {
await router.navigate(['a']);
done();
}
});
await router.navigateByUrl('/b');
});
});

0 comments on commit e34299a

Please sign in to comment.