Skip to content

Commit

Permalink
fix(router): Ensure that new RouterOutlet instances work after old …
Browse files Browse the repository at this point in the history
…ones are destroyed

There can be timing issues with removing an old outlet and creating a
new one to replace it. Before calling `onChildOutletDestroyed`, the
`RouterOutlet` will first check to ensure that it is still the one
registered for that outlet name.

Fixes angular#36711
Fixes angular#32453
  • Loading branch information
atscott committed Jun 28, 2022
1 parent f86e094 commit 28a0b6e
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 2 deletions.
5 changes: 4 additions & 1 deletion packages/router/src/directives/router_outlet.ts
Expand Up @@ -183,7 +183,10 @@ export class RouterOutlet implements OnDestroy, OnInit, RouterOutletContract {

/** @nodoc */
ngOnDestroy(): void {
this.parentContexts.onChildOutletDestroyed(this.name);
// Ensure that the registered outlet is this one before removing it on the context.
if (this.parentContexts.getContext(this.name)?.outlet === this) {
this.parentContexts.onChildOutletDestroyed(this.name);
}
}

/** @nodoc */
Expand Down
38 changes: 37 additions & 1 deletion packages/router/test/regression_integration.spec.ts
Expand Up @@ -10,7 +10,7 @@ import {CommonModule, Location} from '@angular/common';
import {SpyLocation} from '@angular/common/testing';
import {ChangeDetectionStrategy, ChangeDetectorRef, Component, Injectable, NgModule, TemplateRef, Type, ViewChild, ViewContainerRef} from '@angular/core';
import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing';
import {Resolve, Router} from '@angular/router';
import {ChildrenOutletContexts, Resolve, Router} from '@angular/router';
import {RouterTestingModule} from '@angular/router/testing';
import {of} from 'rxjs';
import {delay, mapTo} from 'rxjs/operators';
Expand Down Expand Up @@ -339,6 +339,42 @@ describe('Integration', () => {
expect(fixture.nativeElement.innerHTML).toContain('one');
}));
});

it('should not unregister outlet if a different one already exists #36711, #32453', async () => {
@Component({
template: `
<ng-container [ngSwitch]="layout">
<router-outlet *ngSwitchCase=2></router-outlet>
<router-outlet *ngSwitchCase=1></router-outlet>
</ng-container>
`,
})
class TestCmp {
layout = 1;
}

@Component({template: ''})
class EmptyCmp {
}

TestBed.configureTestingModule({
imports: [CommonModule, RouterTestingModule.withRoutes([{path: '**', component: EmptyCmp}])],
declarations: [TestCmp, EmptyCmp]
})
const fixture = TestBed.createComponent(TestCmp);
const contexts = TestBed.inject(ChildrenOutletContexts);
await TestBed.inject(Router).navigateByUrl('/');
fixture.detectChanges();

expect(contexts.getContext('primary')).toBeDefined();
expect(contexts.getContext('primary')?.outlet).not.toBeNull();

fixture.componentInstance.layout = 2;
fixture.detectChanges();
// Destroying the first one show not clear the outlet context because the second one takes over
// as the registered outlet.
expect(contexts.getContext('primary')?.outlet).not.toBeNull();
});
});

function advance<T>(fixture: ComponentFixture<T>): void {
Expand Down

0 comments on commit 28a0b6e

Please sign in to comment.