From 627daf9b6b14c01b046f1a59516abfe7834c20fd Mon Sep 17 00:00:00 2001
From: Kristiyan Kostadinov <crisbeto@users.noreply.github.com>
Date: Wed, 20 Jul 2022 14:03:16 +0200
Subject: [PATCH] fix(cdk/overlay): detach overlay when portal is destroyed
 from the outside (#25212)

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.
---
 src/cdk/overlay/overlay-ref.ts  | 20 +++++++++++++++++
 src/cdk/overlay/overlay.spec.ts | 39 ++++++++++++++++++++++++++++++++-
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/src/cdk/overlay/overlay-ref.ts b/src/cdk/overlay/overlay-ref.ts
index 9fc473a37e68..d0347057913c 100644
--- a/src/cdk/overlay/overlay-ref.ts
+++ b/src/cdk/overlay/overlay-ref.ts
@@ -158,6 +158,26 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
     }
 
     this._outsideClickDispatcher.add(this);
+
+    // TODO(crisbeto): the null check is here, because the portal outlet returns `any`.
+    // We should be guaranteed for the result to be `ComponentRef | EmbeddedViewRef`, but
+    // `instanceof EmbeddedViewRef` doesn't appear to work at the moment.
+    if (typeof attachResult?.onDestroy === 'function') {
+      // In most cases we control the portal and we know when it is being detached so that
+      // we can finish the disposal process. The exception is if the user passes in a custom
+      // `ViewContainerRef` that isn't destroyed through the overlay API. Note that we use
+      // `detach` here instead of `dispose`, because we don't know if the user intends to
+      // reattach the overlay at a later point. It also has the advantage of waiting for animations.
+      attachResult.onDestroy(() => {
+        if (this.hasAttached()) {
+          // We have to delay the `detach` call, because detaching immediately prevents
+          // other destroy hooks from running. This is likely a framework bug similar to
+          // https://github.com/angular/angular/issues/46119
+          this._ngZone.runOutsideAngular(() => Promise.resolve().then(() => this.detach()));
+        }
+      });
+    }
+
     return attachResult;
   }
 
diff --git a/src/cdk/overlay/overlay.spec.ts b/src/cdk/overlay/overlay.spec.ts
index e0afdbcf423a..bfa3355a45c6 100644
--- a/src/cdk/overlay/overlay.spec.ts
+++ b/src/cdk/overlay/overlay.spec.ts
@@ -1,4 +1,11 @@
-import {waitForAsync, fakeAsync, tick, ComponentFixture, TestBed} from '@angular/core/testing';
+import {
+  waitForAsync,
+  fakeAsync,
+  tick,
+  ComponentFixture,
+  TestBed,
+  flush,
+} from '@angular/core/testing';
 import {
   Component,
   ViewChild,
@@ -463,6 +470,36 @@ describe('Overlay', () => {
     expect(() => overlayRef.removePanelClass([''])).not.toThrow();
   });
 
+  it('should detach a component-based overlay when the view is destroyed', fakeAsync(() => {
+    const overlayRef = overlay.create();
+    const paneElement = overlayRef.overlayElement;
+
+    overlayRef.attach(componentPortal);
+    viewContainerFixture.detectChanges();
+
+    expect(paneElement.childNodes.length).not.toBe(0);
+
+    viewContainerFixture.destroy();
+    flush();
+
+    expect(paneElement.childNodes.length).toBe(0);
+  }));
+
+  it('should detach a template-based overlay when the view is destroyed', fakeAsync(() => {
+    const overlayRef = overlay.create();
+    const paneElement = overlayRef.overlayElement;
+
+    overlayRef.attach(templatePortal);
+    viewContainerFixture.detectChanges();
+
+    expect(paneElement.childNodes.length).not.toBe(0);
+
+    viewContainerFixture.destroy();
+    flush();
+
+    expect(paneElement.childNodes.length).toBe(0);
+  }));
+
   describe('positioning', () => {
     let config: OverlayConfig;