fix(portal): inaccurate hasAttached result and portal being cleared if attached too early

src/cdk/portal/portal-directives.ts

@@ -7,15 +7,16 @@
  */
 
 import {
-    NgModule,
-    ComponentRef,
-    Directive,
-    EmbeddedViewRef,
-    TemplateRef,
-    ComponentFactoryResolver,
-    ViewContainerRef,
-    OnDestroy,
-    Input,
+  NgModule,
+  ComponentRef,
+  Directive,
+  EmbeddedViewRef,
+  TemplateRef,
+  ComponentFactoryResolver,
+  ViewContainerRef,
+  OnDestroy,
+  OnInit,
+  Input,
 } from '@angular/core';
 import {Portal, TemplatePortal, ComponentPortal, BasePortalOutlet} from './portal';
 
@@ -47,9 +48,9 @@ export class CdkPortal extends TemplatePortal<any> {
   exportAs: 'cdkPortalOutlet, cdkPortalHost',
   inputs: ['portal: cdkPortalOutlet']
 })
-export class CdkPortalOutlet extends BasePortalOutlet implements OnDestroy {
-  /** The attached portal. */
-  private _portal: Portal<any> | null = null;
+export class CdkPortalOutlet extends BasePortalOutlet implements OnInit, OnDestroy {
+  /** Whether the portal component is initialized. */
+  private _isInitialized = false;
 
   constructor(
       private _componentFactoryResolver: ComponentFactoryResolver,
@@ -69,10 +70,18 @@ export class CdkPortalOutlet extends BasePortalOutlet implements OnDestroy {
 
   /** Portal associated with the Portal outlet. */
   get portal(): Portal<any> | null {
-    return this._portal;
+    return this._attachedPortal;
   }
 
   set portal(portal: Portal<any> | null) {
+    // Ignore the cases where the `portal` is set to a falsy value before the lifecycle hooks have
+    // run. This handles the cases where the user might do something like `<div cdkPortalOutlet>`
+    // and attach a portal programmatically in the parent component. When Angular does the first CD
+    // round, it will fire the setter with empty string, causing the user's content to be cleared.
+    if (this.hasAttached() && !portal && !this._isInitialized) {
+      return;
+    }
+
     if (this.hasAttached()) {
       super.detach();
     }
@@ -81,12 +90,16 @@ export class CdkPortalOutlet extends BasePortalOutlet implements OnDestroy {
       super.attach(portal);
     }
 
-    this._portal = portal;
+    this._attachedPortal = portal;
+  }
+
+  ngOnInit() {
+    this._isInitialized = true;
   }
 
   ngOnDestroy() {
     super.dispose();
-    this._portal = null;
+    this._attachedPortal = null;
   }
 
   /**
@@ -100,18 +113,18 @@ export class CdkPortalOutlet extends BasePortalOutlet implements OnDestroy {
 
     // If the portal specifies an origin, use that as the logical location of the component
     // in the application tree. Otherwise use the location of this PortalOutlet.
-    let viewContainerRef = portal.viewContainerRef != null ?
+    const viewContainerRef = portal.viewContainerRef != null ?
         portal.viewContainerRef :
         this._viewContainerRef;
 
-    let componentFactory =
+    const componentFactory =
         this._componentFactoryResolver.resolveComponentFactory(portal.component);
-    let ref = viewContainerRef.createComponent(
+    const ref = viewContainerRef.createComponent(
         componentFactory, viewContainerRef.length,
         portal.injector || viewContainerRef.parentInjector);
 
     super.setDisposeFn(() => ref.destroy());
-    this._portal = portal;
+    this._attachedPortal = portal;
 
     return ref;
   }
@@ -126,7 +139,7 @@ export class CdkPortalOutlet extends BasePortalOutlet implements OnDestroy {
     const viewRef = this._viewContainerRef.createEmbeddedView(portal.templateRef, portal.context);
     super.setDisposeFn(() => this._viewContainerRef.clear());
 
-    this._portal = portal;
+    this._attachedPortal = portal;
 
     return viewRef;
   }

src/cdk/portal/portal.spec.ts

@@ -1,4 +1,4 @@
-import {inject, ComponentFixture, TestBed, async} from '@angular/core/testing';
+import {inject, ComponentFixture, TestBed} from '@angular/core/testing';
 import {
   NgModule,
   Component,
@@ -20,13 +20,11 @@ import {DomPortalOutlet} from './dom-portal-outlet';
 
 describe('Portals', () => {
 
-  beforeEach(async(() => {
-    TestBed.configureTestingModule({
-      imports: [PortalModule, PortalTestModule],
-    });
-
-    TestBed.compileComponents();
-  }));
+  beforeEach(() => {
+    TestBed
+      .configureTestingModule({imports: [PortalModule, PortalTestModule]})
+      .compileComponents();
+  });
 
   describe('CdkPortalOutlet', () => {
     let fixture: ComponentFixture<PortalTestApp>;
@@ -37,28 +35,33 @@ describe('Portals', () => {
 
     it('should load a component into the portal', () => {
       // Set the selectedHost to be a ComponentPortal.
-      let testAppComponent = fixture.debugElement.componentInstance;
-      testAppComponent.selectedPortal = new ComponentPortal(PizzaMsg);
+      let testAppComponent = fixture.componentInstance;
+      let componentPortal = new ComponentPortal(PizzaMsg);
+      let hostContainer = fixture.nativeElement.querySelector('.portal-container');
+
+      testAppComponent.selectedPortal = componentPortal;
       fixture.detectChanges();
 
       // Expect that the content of the attached portal is present.
-      let hostContainer = fixture.nativeElement.querySelector('.portal-container');
       expect(hostContainer.textContent).toContain('Pizza');
+      expect(testAppComponent.portalOutlet.portal).toBe(componentPortal);
     });
 
     it('should load a template into the portal', () => {
-      let testAppComponent = fixture.debugElement.componentInstance;
+      let testAppComponent = fixture.componentInstance;
       let hostContainer = fixture.nativeElement.querySelector('.portal-container');
-
       let templatePortal = new TemplatePortal(testAppComponent.templateRef, null!);
+
       testAppComponent.selectedPortal = templatePortal;
       fixture.detectChanges();
+
       // Expect that the content of the attached portal is present and no context is projected
       expect(hostContainer.textContent).toContain('Banana');
+      expect(testAppComponent.portalOutlet.portal).toBe(templatePortal);
     });
 
     it('should project template context bindings in the portal', () => {
-      let testAppComponent = fixture.debugElement.componentInstance;
+      let testAppComponent = fixture.componentInstance;
       let hostContainer = fixture.nativeElement.querySelector('.portal-container');
 
       // TemplatePortal without context:
@@ -99,7 +102,7 @@ describe('Portals', () => {
 
     it('should dispose the host when destroyed', () => {
       // Set the selectedHost to be a ComponentPortal.
-      let testAppComponent = fixture.debugElement.componentInstance;
+      let testAppComponent = fixture.componentInstance;
       testAppComponent.selectedPortal = new ComponentPortal(PizzaMsg);
 
       fixture.detectChanges();
@@ -114,7 +117,7 @@ describe('Portals', () => {
       let chocolateInjector = new ChocolateInjector(fixture.componentInstance.injector);
 
       // Set the selectedHost to be a ComponentPortal.
-      let testAppComponent = fixture.debugElement.componentInstance;
+      let testAppComponent = fixture.componentInstance;
       testAppComponent.selectedPortal = new ComponentPortal(PizzaMsg, undefined, chocolateInjector);
       fixture.detectChanges();
 
@@ -125,7 +128,7 @@ describe('Portals', () => {
     });
 
     it('should load a <ng-template> portal', () => {
-      let testAppComponent = fixture.debugElement.componentInstance;
+      let testAppComponent = fixture.componentInstance;
 
       // Detect changes initially so that the component's ViewChildren are resolved.
       fixture.detectChanges();
@@ -140,7 +143,7 @@ describe('Portals', () => {
     });
 
     it('should load a <ng-template> portal with the `*` sugar', () => {
-      let testAppComponent = fixture.debugElement.componentInstance;
+      let testAppComponent = fixture.componentInstance;
 
       // Detect changes initially so that the component's ViewChildren are resolved.
       fixture.detectChanges();
@@ -155,7 +158,7 @@ describe('Portals', () => {
     });
 
     it('should load a <ng-template> portal with a binding', () => {
-      let testAppComponent = fixture.debugElement.componentInstance;
+      let testAppComponent = fixture.componentInstance;
 
       // Detect changes initially so that the component's ViewChildren are resolved.
       fixture.detectChanges();
@@ -177,7 +180,7 @@ describe('Portals', () => {
     });
 
     it('should load a <ng-template> portal with an inner template', () => {
-      let testAppComponent = fixture.debugElement.componentInstance;
+      let testAppComponent = fixture.componentInstance;
 
       // Detect changes initially so that the component's ViewChildren are resolved.
       fixture.detectChanges();
@@ -199,7 +202,7 @@ describe('Portals', () => {
     });
 
     it('should change the attached portal', () => {
-      let testAppComponent = fixture.debugElement.componentInstance;
+      let testAppComponent = fixture.componentInstance;
 
       // Detect changes initially so that the component's ViewChildren are resolved.
       fixture.detectChanges();
@@ -219,22 +222,22 @@ describe('Portals', () => {
     });
 
     it('should detach the portal when it is set to null', () => {
-      let testAppComponent = fixture.debugElement.componentInstance;
+      let testAppComponent = fixture.componentInstance;
       testAppComponent.selectedPortal = new ComponentPortal(PizzaMsg);
 
       fixture.detectChanges();
       expect(testAppComponent.portalOutlet.hasAttached()).toBe(true);
       expect(testAppComponent.portalOutlet.portal).toBe(testAppComponent.selectedPortal);
 
-      testAppComponent.selectedPortal = null;
+      testAppComponent.selectedPortal = null!;
       fixture.detectChanges();
 
       expect(testAppComponent.portalOutlet.hasAttached()).toBe(false);
       expect(testAppComponent.portalOutlet.portal).toBeNull();
     });
 
     it('should set the `portal` when attaching a component portal programmatically', () => {
-      let testAppComponent = fixture.debugElement.componentInstance;
+      let testAppComponent = fixture.componentInstance;
       let portal = new ComponentPortal(PizzaMsg);
 
       testAppComponent.portalOutlet.attachComponentPortal(portal);
@@ -243,7 +246,7 @@ describe('Portals', () => {
     });
 
     it('should set the `portal` when attaching a template portal programmatically', () => {
-      let testAppComponent = fixture.debugElement.componentInstance;
+      let testAppComponent = fixture.componentInstance;
       fixture.detectChanges();
 
       testAppComponent.portalOutlet.attachTemplatePortal(testAppComponent.cakePortal);
@@ -252,7 +255,7 @@ describe('Portals', () => {
     });
 
     it('should clear the portal reference on destroy', () => {
-      let testAppComponent = fixture.debugElement.componentInstance;
+      let testAppComponent = fixture.componentInstance;
 
       testAppComponent.selectedPortal = new ComponentPortal(PizzaMsg);
       fixture.detectChanges();
@@ -263,6 +266,40 @@ describe('Portals', () => {
 
       expect(testAppComponent.portalOutlet.portal).toBeNull();
     });
+
+    it('should not clear programmatically-attached portals on init', () => {
+      fixture.destroy();
+
+      const unboundFixture = TestBed.createComponent(UnboundPortalTestApp);
+
+      // Note: calling `detectChanges` here will cause a false positive.
+      // What we're testing is attaching before the first CD cycle.
+      unboundFixture.componentInstance.portalOutlet.attach(new ComponentPortal(PizzaMsg));
+      unboundFixture.detectChanges();
+
+      expect(unboundFixture.nativeElement.querySelector('.portal-container').textContent)
+        .toContain('Pizza');
+    });
+
+    it('should be considered attached when attaching using `attach`', () => {
+      expect(fixture.componentInstance.portalOutlet.hasAttached()).toBe(false);
+      fixture.componentInstance.portalOutlet.attach(new ComponentPortal(PizzaMsg));
+      expect(fixture.componentInstance.portalOutlet.hasAttached()).toBe(true);
+    });
+
+    it('should be considered attached when attaching using `attachComponentPortal`', () => {
+      expect(fixture.componentInstance.portalOutlet.hasAttached()).toBe(false);
+      fixture.componentInstance.portalOutlet.attachComponentPortal(new ComponentPortal(PizzaMsg));
+      expect(fixture.componentInstance.portalOutlet.hasAttached()).toBe(true);
+    });
+
+    it('should be considered attached when attaching using `attachTemplatePortal`', () => {
+      const instance = fixture.componentInstance;
+      expect(instance.portalOutlet.hasAttached()).toBe(false);
+      instance.portalOutlet.attachTemplatePortal(new TemplatePortal(instance.templateRef, null!));
+      expect(instance.portalOutlet.hasAttached()).toBe(true);
+    });
+
   });
 
   describe('DomPortalOutlet', () => {
@@ -345,7 +382,7 @@ describe('Portals', () => {
     it('should attach and detach a template portal with a binding', () => {
       let fixture = TestBed.createComponent(PortalTestApp);
 
-      let testAppComponent = fixture.debugElement.componentInstance;
+      let testAppComponent = fixture.componentInstance;
 
       // Detect changes initially so that the component's ViewChildren are resolved.
       fixture.detectChanges();
@@ -484,7 +521,7 @@ class PortalTestApp {
   @ViewChild(CdkPortalOutlet) portalOutlet: CdkPortalOutlet;
   @ViewChild('templateRef', { read: TemplateRef }) templateRef: TemplateRef<any>;
 
-  selectedPortal: Portal<any>;
+  selectedPortal: Portal<any>|undefined;
   fruit: string = 'Banana';
   fruits = ['Apple', 'Pineapple', 'Durian'];
 
@@ -508,9 +545,27 @@ class PortalTestApp {
 
 }
 
+/** Test-bed component that contains a portal outlet and a couple of template portals. */
+@Component({
+  template: `
+    <div class="portal-container">
+      <ng-template cdkPortalOutlet></ng-template>
+    </div>
+  `,
+})
+class UnboundPortalTestApp {
+  @ViewChild(CdkPortalOutlet) portalOutlet: CdkPortalOutlet;
+}
+
 // Create a real (non-test) NgModule as a workaround for
 // https://github.com/angular/angular/issues/10760
-const TEST_COMPONENTS = [PortalTestApp, ArbitraryViewContainerRefComponent, PizzaMsg];
+const TEST_COMPONENTS = [
+  PortalTestApp,
+  UnboundPortalTestApp,
+  ArbitraryViewContainerRefComponent,
+  PizzaMsg
+];
+
 @NgModule({
   imports: [CommonModule, PortalModule],
   exports: TEST_COMPONENTS,

src/cdk/portal/portal.ts

@@ -168,7 +168,7 @@ export interface PortalOutlet {
  */
 export abstract class BasePortalOutlet implements PortalOutlet {
   /** The portal currently attached to the host. */
-  private _attachedPortal: Portal<any> | null;
+  protected _attachedPortal: Portal<any> | null;
 
   /** A function that will permanently dispose this host. */
   private _disposeFn: (() => void) | null;