fix(ivy): content projection should not corrupt TNode data structures (#29130)

PR Close #29130

packages/core/src/render3/instructions.ts

@@ -1308,6 +1308,7 @@ export function createTNode(
     outputs: undefined,
     tViews: null,
     next: null,
+    projectionNext: null,
     child: null,
     parent: tParent,
     stylingTemplate: null,
@@ -2550,26 +2551,24 @@ export function projectionDef(selectors?: CssSelectorList[], textSelectors?: str
 
   if (!componentNode.projection) {
     const noOfNodeBuckets = selectors ? selectors.length + 1 : 1;
-    const pData: (TNode | null)[] = componentNode.projection =
+    const projectionHeads: (TNode | null)[] = componentNode.projection =
         new Array(noOfNodeBuckets).fill(null);
-    const tails: (TNode | null)[] = pData.slice();
+    const tails: (TNode | null)[] = projectionHeads.slice();
 
     let componentChild: TNode|null = componentNode.child;
 
     while (componentChild !== null) {
       const bucketIndex =
           selectors ? matchingSelectorIndex(componentChild, selectors, textSelectors !) : 0;
-      const nextNode = componentChild.next;
 
       if (tails[bucketIndex]) {
-        tails[bucketIndex] !.next = componentChild;
+        tails[bucketIndex] !.projectionNext = componentChild;
       } else {
-        pData[bucketIndex] = componentChild;
+        projectionHeads[bucketIndex] = componentChild;
       }
-      componentChild.next = null;
       tails[bucketIndex] = componentChild;
 
-      componentChild = nextNode;
+      componentChild = componentChild.next;
     }
   }
 }

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

@@ -283,6 +283,14 @@ export interface TNode {
    */
   next: TNode|null;
 
+  /**
+   * The next projected sibling. Since in Angular content projection works on the node-by-node basis
+   * the act of projecting nodes might change nodes relationship at the insertion point (target
+   * view). At the same time we need to keep initial relationship between nodes as expressed in
+   * content view.
+   */
+  projectionNext: TNode|null;
+
   /**
    * First child of the current node.
    *

packages/core/src/render3/node_manipulation.ts

@@ -84,7 +84,7 @@ const projectionNodeStack: (LView | TNode)[] = [];
  */
 function walkTNodeTree(
     viewToWalk: LView, action: WalkTNodeTreeAction, renderer: Renderer3,
-    renderParent: RElement | null, beforeNode?: RNode | null) {
+    renderParent: RElement | null, beforeNode?: RNode | null): void {
   const rootTNode = viewToWalk[TVIEW].node as TViewNode;
   let projectionNodeIndex = -1;
   let currentView = viewToWalk;
@@ -141,11 +141,11 @@ function walkTNodeTree(
 
     if (nextTNode === null) {
       // this last node was projected, we need to get back down to its projection node
-      if (tNode.next === null && (tNode.flags & TNodeFlags.isProjected)) {
+      if (tNode.projectionNext === null && (tNode.flags & TNodeFlags.isProjected)) {
         currentView = projectionNodeStack[projectionNodeIndex--] as LView;
         tNode = projectionNodeStack[projectionNodeIndex--] as TNode;
       }
-      nextTNode = tNode.next;
+      nextTNode = (tNode.flags & TNodeFlags.isProjected) ? tNode.projectionNext : tNode.next;
 
       /**
        * Find the next node in the TNode tree, taking into account the place where a node is
@@ -158,7 +158,7 @@ function walkTNodeTree(
         // If parent is null, we're crossing the view boundary, so we should get the host TNode.
         tNode = tNode.parent || currentView[T_HOST];
 
-        if (tNode === null || tNode === rootTNode) return null;
+        if (tNode === null || tNode === rootTNode) return;
 
         // When exiting a container, the beforeNode must be restored to the previous value
         if (tNode.type === TNodeType.Container) {
@@ -176,7 +176,7 @@ function walkTNodeTree(
            */
           while (!currentView[NEXT] && currentView[PARENT] &&
                  !(tNode.parent && tNode.parent.next)) {
-            if (tNode === rootTNode) return null;
+            if (tNode === rootTNode) return;
             currentView = currentView[PARENT] as LView;
             tNode = currentView[T_HOST] !;
           }
@@ -755,7 +755,7 @@ export function appendProjectedNodes(
         nodeToProject.flags |= TNodeFlags.isProjected;
         appendProjectedNode(nodeToProject, tProjectionNode, lView, projectedView);
       }
-      nodeToProject = nodeToProject.next;
+      nodeToProject = nodeToProject.projectionNext;
     }
   }
 }

packages/core/test/linker/projection_integration_spec.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Component, ComponentFactoryResolver, ComponentRef, Directive, ElementRef, Injector, NgModule, OnInit, TemplateRef, ViewChild, ViewContainerRef, ViewEncapsulation} from '@angular/core';
+import {Component, ComponentFactoryResolver, ComponentRef, Directive, ElementRef, Injector, Input, NgModule, OnInit, TemplateRef, ViewChild, ViewContainerRef, ViewEncapsulation} from '@angular/core';
 import {ComponentFixture, TestBed} from '@angular/core/testing';
 import {By} from '@angular/platform-browser/src/dom/debug/by';
 import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter';
@@ -224,6 +224,44 @@ describe('projection', () => {
     expect(main.nativeElement).toHaveText('(, BC)');
   });
 
+  it('should redistribute non-continuous blocks of nodes when the shadow dom changes', () => {
+    @Component({
+      selector: 'child',
+      template:
+          `<ng-content></ng-content>(<ng-template [ngIf]="showing"><ng-content select="div"></ng-content></ng-template>)`
+    })
+    class Child {
+      @Input() showing !: boolean;
+    }
+
+    @Component({
+      selector: 'app',
+      template: `<child [showing]="showing">
+        <div>A</div>
+        <span>B</span>
+        <div>A</div>
+        <span>B</span>
+      </child>`
+    })
+    class App {
+      showing = false;
+    }
+
+    TestBed.configureTestingModule({declarations: [App, Child]});
+    const fixture = TestBed.createComponent(App);
+    fixture.detectChanges();
+
+    expect(fixture.nativeElement).toHaveText('BB()');
+
+    fixture.componentInstance.showing = true;
+    fixture.detectChanges();
+    expect(fixture.nativeElement).toHaveText('BB(AA)');
+
+    fixture.componentInstance.showing = false;
+    fixture.detectChanges();
+    expect(fixture.nativeElement).toHaveText('BB()');
+  });
+
   // GH-2095 - https://github.com/angular/angular/issues/2095
   // important as we are removing the ng-content element during compilation,
   // which could skrew up text node indices.