From 22ddbf4b0281095a5b459568e671f9e37139336b Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Wed, 6 Mar 2019 14:01:02 +0100 Subject: [PATCH] fix(ivy): content projection should not corrupt TNode data structures (#29130) PR Close #29130 --- packages/core/src/render3/instructions.ts | 13 +++--- packages/core/src/render3/interfaces/node.ts | 8 ++++ .../core/src/render3/node_manipulation.ts | 12 +++--- .../linker/projection_integration_spec.ts | 40 ++++++++++++++++++- 4 files changed, 59 insertions(+), 14 deletions(-) diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index d40746a4e4..495c26b244 100644 --- a/packages/core/src/render3/instructions.ts +++ b/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; } } } diff --git a/packages/core/src/render3/interfaces/node.ts b/packages/core/src/render3/interfaces/node.ts index 4f8ac12766..7e4af970fc 100644 --- a/packages/core/src/render3/interfaces/node.ts +++ b/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. * diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index 59322330d5..2fb868b5e0 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/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; } } } diff --git a/packages/core/test/linker/projection_integration_spec.ts b/packages/core/test/linker/projection_integration_spec.ts index 65a2233fdc..9ef90d3195 100644 --- a/packages/core/test/linker/projection_integration_spec.ts +++ b/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: + `()` + }) + class Child { + @Input() showing !: boolean; + } + + @Component({ + selector: 'app', + template: ` +
A
+ B +
A
+ B + ` + }) + 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.