fix(ivy): ViewContainerRef.destroy should properly clean the DOM (angular#29414)

PR Close angular#29414

Author: marclaval

packages/common/test/directives/ng_switch_spec.ts

@@ -7,7 +7,7 @@
  */
 
 import {CommonModule} from '@angular/common';
-import {Attribute, Component, Directive} from '@angular/core';
+import {Attribute, Component, Directive, TemplateRef, ViewChild} from '@angular/core';
 import {ComponentFixture, TestBed} from '@angular/core/testing';
 import {expect} from '@angular/platform-browser/testing/src/matchers';
 
@@ -26,7 +26,7 @@ import {expect} from '@angular/platform-browser/testing/src/matchers';
 
     beforeEach(() => {
       TestBed.configureTestingModule({
-        declarations: [TestComponent],
+        declarations: [TestComponent, ComplexComponent],
         imports: [CommonModule],
       });
     });
@@ -171,6 +171,20 @@ import {expect} from '@angular/platform-browser/testing/src/matchers';
         getComponent().switchValue = 'b';
         detectChangesAndExpectText('when b1;when b2;');
       });
+
+      it('should support nested NgSwitch on ng-container with ngTemplateOutlet', () => {
+        fixture = TestBed.createComponent(ComplexComponent);
+        detectChangesAndExpectText('Foo');
+
+        fixture.componentInstance.state = 'case2';
+        detectChangesAndExpectText('Bar');
+
+        fixture.componentInstance.state = 'notACase';
+        detectChangesAndExpectText('Default');
+
+        fixture.componentInstance.state = 'case1';
+        detectChangesAndExpectText('Foo');
+      });
     });
   });
 }
@@ -182,6 +196,38 @@ class TestComponent {
   when2: any = null;
 }
 
+@Component({
+  selector: 'complex-cmp',
+  template: `
+<div [ngSwitch]="state">
+  <ng-container *ngSwitchCase="'case1'" [ngSwitch]="true">
+    <ng-container *ngSwitchCase="true" [ngTemplateOutlet]="foo"></ng-container>
+    <span *ngSwitchDefault>Should never render</span>
+  </ng-container>
+  <ng-container *ngSwitchCase="'case2'" [ngSwitch]="true">
+    <ng-container *ngSwitchCase="true" [ngTemplateOutlet]="bar"></ng-container>
+    <span *ngSwitchDefault>Should never render</span>
+  </ng-container>
+  <ng-container *ngSwitchDefault [ngSwitch]="false">
+    <ng-container *ngSwitchCase="true" [ngTemplateOutlet]="foo"></ng-container>
+    <span *ngSwitchDefault>Default</span>
+  </ng-container>
+</div>
+
+<ng-template #foo>
+  <span>Foo</span>
+</ng-template>
+<ng-template #bar>
+  <span>Bar</span>
+</ng-template>
+`
+})
+class ComplexComponent {
+  @ViewChild('foo') foo !: TemplateRef<any>;
+  @ViewChild('bar') bar !: TemplateRef<any>;
+  state: string = 'case1';
+}
+
 function createTestComponent(template: string): ComponentFixture<TestComponent> {
   return TestBed.overrideComponent(TestComponent, {set: {template: template}})
       .createComponent(TestComponent);

packages/core/src/render3/instructions/instructions.ts

@@ -1952,7 +1952,7 @@ function generateInitialInputs(
  */
 export function createLContainer(
     hostNative: RElement | RComment | StylingContext | LView, currentView: LView, native: RComment,
-    isForViewContainerRef?: boolean): LContainer {
+    tNode: TNode, isForViewContainerRef?: boolean): LContainer {
   ngDevMode && assertDomNode(native);
   ngDevMode && assertLView(currentView);
   const lContainer: LContainer = [
@@ -1962,8 +1962,9 @@ export function createLContainer(
     currentView,                     // parent
     null,                            // next
     null,                            // queries
-    [],                              // views
+    tNode,                           // t_host
     native,                          // native
+    [],                              // views
   ];
   ngDevMode && attachLContainerDebug(lContainer);
   return lContainer;
@@ -2037,7 +2038,8 @@ function containerInternal(
   const comment = lView[RENDERER].createComment(ngDevMode ? 'container' : '');
   ngDevMode && ngDevMode.rendererCreateComment++;
   const tNode = createNodeAtIndex(index, TNodeType.Container, comment, tagName, attrs);
-  const lContainer = lView[adjustedIndex] = createLContainer(lView[adjustedIndex], lView, comment);
+  const lContainer = lView[adjustedIndex] =
+      createLContainer(lView[adjustedIndex], lView, comment, tNode);
 
   appendChild(comment, tNode, lView);
 

packages/core/src/render3/interfaces/container.ts

@@ -6,10 +6,12 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {TNode} from './node';
 import {LQueries} from './query';
 import {RComment, RElement} from './renderer';
 import {StylingContext} from './styling';
-import {HOST, LView, NEXT, PARENT, QUERIES} from './view';
+import {HOST, LView, NEXT, PARENT, QUERIES, T_HOST} from './view';
+
 
 /**
  * Special location which allows easy identification of type. If we have an array which was
@@ -23,10 +25,10 @@ export const TYPE = 1;
  * Uglify will inline these when minifying so there shouldn't be a cost.
  */
 export const ACTIVE_INDEX = 2;
-// PARENT, NEXT, and QUERIES are indices 3, 4, and 5.
+// PARENT, NEXT, QUERIES and T_HOST are indices 3, 4, 5 and 6.
 // As we already have these constants in LView, we don't need to re-create them.
-export const VIEWS = 6;
 export const NATIVE = 7;
+export const VIEWS = 8;
 
 /**
  * The state associated with a container.
@@ -83,17 +85,22 @@ export interface LContainer extends Array<any> {
   // `[QUERIES]` in it which are not needed for `LContainer` (only needed for Template)
 
   /**
-   * A list of the container's currently active child views. Views will be inserted
-   * here as they are added and spliced from here when they are removed. We need
-   * to keep a record of current views so we know which views are already in the DOM
-   * (and don't need to be re-added) and so we can remove views from the DOM when they
-   * are no longer required.
+   * Pointer to the `TNode` which represents the host of the container.
    */
-  [VIEWS]: LView[];
+  [T_HOST]: TNode;
 
   /** The comment element that serves as an anchor for this LContainer. */
   readonly[NATIVE]:
       RComment;  // TODO(misko): remove as this value can be gotten by unwrapping `[HOST]`
+
+  /**
+*A list of the container's currently active child views. Views will be inserted
+*here as they are added and spliced from here when they are removed. We need
+*to keep a record of current views so we know which views are already in the DOM
+*(and don't need to be re-added) and so we can remove views from the DOM when they
+*are no longer required.
+*/
+  [VIEWS]: LView[];
 }
 
 // Note: This hack is necessary so we don't erroneously get a circular dependency

packages/core/src/render3/node_manipulation.ts

@@ -91,14 +91,22 @@ function walkTNodeTree(
   let tNode: TNode|null = rootTNode.child as TNode;
   while (tNode) {
     let nextTNode: TNode|null = null;
-    if (tNode.type === TNodeType.Element) {
+    if (tNode.type === TNodeType.Element || tNode.type === TNodeType.ElementContainer) {
       executeNodeAction(
           action, renderer, renderParent, getNativeByTNode(tNode, currentView), tNode, beforeNode);
       const nodeOrContainer = currentView[tNode.index];
       if (isLContainer(nodeOrContainer)) {
         // This element has an LContainer, and its comment needs to be handled
         executeNodeAction(
             action, renderer, renderParent, nodeOrContainer[NATIVE], tNode, beforeNode);
+        if (nodeOrContainer[VIEWS].length) {
+          currentView = nodeOrContainer[VIEWS][0];
+          nextTNode = currentView[TVIEW].node;
+
+          // When the walker enters a container, then the beforeNode has to become the local native
+          // comment node.
+          beforeNode = nodeOrContainer[NATIVE];
+        }
       }
     } else if (tNode.type === TNodeType.Container) {
       const lContainer = currentView ![tNode.index] as LContainer;
@@ -133,9 +141,8 @@ function walkTNodeTree(
           nextTNode = currentView[TVIEW].data[head.index] as TNode;
         }
       }
-
     } else {
-      // Otherwise, this is a View or an ElementContainer
+      // Otherwise, this is a View
       nextTNode = tNode.child;
     }
 
@@ -145,7 +152,14 @@ function walkTNodeTree(
         currentView = projectionNodeStack[projectionNodeIndex--] as LView;
         tNode = projectionNodeStack[projectionNodeIndex--] as TNode;
       }
-      nextTNode = (tNode.flags & TNodeFlags.isProjected) ? tNode.projectionNext : tNode.next;
+
+      if (tNode.flags & TNodeFlags.isProjected) {
+        nextTNode = tNode.projectionNext;
+      } else if (tNode.type === TNodeType.ElementContainer) {
+        nextTNode = tNode.child || tNode.next;
+      } else {
+        nextTNode = tNode.next;
+      }
 
       /**
        * Find the next node in the TNode tree, taking into account the place where a node is
@@ -172,19 +186,26 @@ function walkTNodeTree(
            * chain until:
            * - we find an lView with a next pointer
            * - or find a tNode with a parent that has a next pointer
+           * - or find a lContainer
            * - or reach root TNode (in which case we exit, since we traversed all nodes)
            */
           while (!currentView[NEXT] && currentView[PARENT] &&
                  !(tNode.parent && tNode.parent.next)) {
             if (tNode === rootTNode) return;
             currentView = currentView[PARENT] as LView;
+            if (isLContainer(currentView)) {
+              tNode = currentView[T_HOST] !;
+              currentView = currentView[PARENT];
+              beforeNode = currentView[tNode.index][NATIVE];
+              break;
+            }
             tNode = currentView[T_HOST] !;
           }
           if (currentView[NEXT]) {
             currentView = currentView[NEXT] as LView;
             nextTNode = currentView[T_HOST];
           } else {
-            nextTNode = tNode.next;
+            nextTNode = tNode.type === TNodeType.ElementContainer && tNode.child || tNode.next;
           }
         } else {
           nextTNode = tNode.next;

packages/core/src/render3/view_engine_compatibility.ts

@@ -321,7 +321,7 @@ export function createContainerRef(
     }
 
     hostView[hostTNode.index] = lContainer =
-        createLContainer(slotValue, hostView, commentNode, true);
+        createLContainer(slotValue, hostView, commentNode, hostTNode, true);
 
     addToViewTree(hostView, lContainer);
   }