fix(portal-host): unable to clear and portal reference not being set (#3302)

* Fixes not being able to clear an attached `PortalHost` by setting its contents to `null`.
* Fixes the `portal` reference not being set, if the portal host gets populated programmatically.
* Clears references to the attached portal on destroy.
* Switches to calling the inherited methods via `super`, instead of `this`, making it easier to spot which ones are inherited.

Author: crisbeto

src/lib/core/portal/portal-directives.ts

@@ -63,14 +63,21 @@ export class PortalHostDirective extends BasePortalHost implements OnDestroy {
     return this._portal;
   }
 
-  set portal(p: Portal<any>) {
-    if (p) {
-      this._replaceAttachedPortal(p);
+  set portal(portal: Portal<any>) {
+    if (this.hasAttached()) {
+      super.detach();
+    }
+
+    if (portal) {
+      super.attach(portal);
     }
+
+    this._portal = portal;
   }
 
   ngOnDestroy() {
     super.dispose();
+    this._portal = null;
   }
 
   /**
@@ -94,6 +101,8 @@ export class PortalHostDirective extends BasePortalHost implements OnDestroy {
         portal.injector || viewContainerRef.parentInjector);
 
     super.setDisposeFn(() => ref.destroy());
+    this._portal = portal;
+
     return ref;
   }
 
@@ -107,21 +116,11 @@ export class PortalHostDirective extends BasePortalHost implements OnDestroy {
     this._viewContainerRef.createEmbeddedView(portal.templateRef);
     super.setDisposeFn(() => this._viewContainerRef.clear());
 
+    this._portal = portal;
+
     // TODO(jelbourn): return locals from view
     return new Map<string, any>();
   }
-
-  /** Detaches the currently attached Portal (if there is one) and attaches the given Portal. */
-  private _replaceAttachedPortal(p: Portal<any>): void {
-    if (this.hasAttached()) {
-      super.detach();
-    }
-
-    if (p) {
-      super.attach(p);
-      this._portal = p;
-    }
-  }
 }
 
 

src/lib/core/portal/portal.spec.ts

@@ -2,6 +2,7 @@ import {inject, ComponentFixture, TestBed, async} from '@angular/core/testing';
 import {
   NgModule,
   Component,
+  ViewChild,
   ViewChildren,
   QueryList,
   ViewContainerRef,
@@ -10,7 +11,7 @@ import {
   Injector,
   ApplicationRef,
 } from '@angular/core';
-import {TemplatePortalDirective, PortalModule} from './portal-directives';
+import {TemplatePortalDirective, PortalHostDirective, PortalModule} from './portal-directives';
 import {Portal, ComponentPortal} from './portal';
 import {DomPortalHost} from './dom-portal-host';
 
@@ -141,6 +142,52 @@ describe('Portals', () => {
 
       expect(hostContainer.textContent).toContain('Pizza');
     });
+
+    it('should detach the portal when it is set to null', () => {
+      let testAppComponent = fixture.debugElement.componentInstance;
+      testAppComponent.selectedPortal = new ComponentPortal(PizzaMsg);
+
+      fixture.detectChanges();
+      expect(testAppComponent.portalHost.hasAttached()).toBe(true);
+      expect(testAppComponent.portalHost.portal).toBe(testAppComponent.selectedPortal);
+
+      testAppComponent.selectedPortal = null;
+      fixture.detectChanges();
+
+      expect(testAppComponent.portalHost.hasAttached()).toBe(false);
+      expect(testAppComponent.portalHost.portal).toBeNull();
+    });
+
+    it('should set the `portal` when attaching a component portal programmatically', () => {
+      let testAppComponent = fixture.debugElement.componentInstance;
+      let portal = new ComponentPortal(PizzaMsg);
+
+      testAppComponent.portalHost.attachComponentPortal(portal);
+
+      expect(testAppComponent.portalHost.portal).toBe(portal);
+    });
+
+    it('should set the `portal` when attaching a template portal programmatically', () => {
+      let testAppComponent = fixture.debugElement.componentInstance;
+      fixture.detectChanges();
+
+      testAppComponent.portalHost.attachTemplatePortal(testAppComponent.cakePortal);
+
+      expect(testAppComponent.portalHost.portal).toBe(testAppComponent.cakePortal);
+    });
+
+    it('should clear the portal reference on destroy', () => {
+      let testAppComponent = fixture.debugElement.componentInstance;
+
+      testAppComponent.selectedPortal = new ComponentPortal(PizzaMsg);
+      fixture.detectChanges();
+
+      expect(testAppComponent.portalHost.portal).toBeTruthy();
+
+      fixture.destroy();
+
+      expect(testAppComponent.portalHost.portal).toBeNull();
+    });
   });
 
   describe('DomPortalHost', () => {
@@ -342,6 +389,7 @@ class ArbitraryViewContainerRefComponent {
 })
 class PortalTestApp {
   @ViewChildren(TemplatePortalDirective) portals: QueryList<TemplatePortalDirective>;
+  @ViewChild(PortalHostDirective) portalHost: PortalHostDirective;
   selectedPortal: Portal<any>;
   fruit: string = 'Banana';