Skip to content

Commit fe7a7a0

Browse files
committed
fix(cdk/overlay): detach overlay when portal is destroyed from the outside
Fixes that some of the overlay elements were left behind when the portal is destroyed without going through the overlay API. We need to handle this case, because in many cases users don't have access to the `OverlayRef` (e.g. `MatDialog`) so that they can call `dispose` themselves. Fixes #25163.
1 parent 5c8c7e8 commit fe7a7a0

File tree

2 files changed

+52
-0
lines changed

2 files changed

+52
-0
lines changed

src/cdk/overlay/overlay-ref.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,28 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
158158
}
159159

160160
this._outsideClickDispatcher.add(this);
161+
162+
// TODO(crisbeto): the null check is here, because the portal outlet returns `any`.
163+
// We should be guaranteed for the result to be `ComponentRef | EmbeddedViewRef`, but
164+
// `instanceof EmbeddedViewRef` doesn't appear to work at the moment.
165+
if (typeof attachResult?.onDestroy === 'function') {
166+
// In most cases we control the portal and we know when it is being detached so that
167+
// we can finish the disposal process. The exception is if the user passes in a custom
168+
// `ViewContainerRef` that isn't destroyed through the overlay API. Note that we use
169+
// `detach` here instead of `dispose`, because we don't know if the user intends to
170+
// reattach the overlay at a later point. It also has the advantage of waiting for animations.
171+
attachResult.onDestroy(() => {
172+
if (this.hasAttached()) {
173+
// We have to wrap the `detach` call in `onStable`, because detaching immediately prevents
174+
// other destroy hooks from running. This is likely a framework bug similar to
175+
// https://github.com/angular/angular/issues/46119
176+
this._ngZone.runOutsideAngular(() => {
177+
this._ngZone.onStable.pipe(take(1)).subscribe(() => this.detach());
178+
});
179+
}
180+
});
181+
}
182+
161183
return attachResult;
162184
}
163185

src/cdk/overlay/overlay.spec.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,36 @@ describe('Overlay', () => {
463463
expect(() => overlayRef.removePanelClass([''])).not.toThrow();
464464
});
465465

466+
it('should detach a component-based overlay when the view is destroyed', () => {
467+
const overlayRef = overlay.create();
468+
const paneElement = overlayRef.overlayElement;
469+
470+
overlayRef.attach(componentPortal);
471+
viewContainerFixture.detectChanges();
472+
473+
expect(paneElement.childNodes.length).not.toBe(0);
474+
475+
viewContainerFixture.destroy();
476+
zone.simulateZoneExit();
477+
478+
expect(paneElement.childNodes.length).toBe(0);
479+
});
480+
481+
it('should detach a template-based overlay when the view is destroyed', () => {
482+
const overlayRef = overlay.create();
483+
const paneElement = overlayRef.overlayElement;
484+
485+
overlayRef.attach(templatePortal);
486+
viewContainerFixture.detectChanges();
487+
488+
expect(paneElement.childNodes.length).not.toBe(0);
489+
490+
viewContainerFixture.destroy();
491+
zone.simulateZoneExit();
492+
493+
expect(paneElement.childNodes.length).toBe(0);
494+
});
495+
466496
describe('positioning', () => {
467497
let config: OverlayConfig;
468498

0 commit comments

Comments
 (0)