From a294e0dd79bbc6969d63ba528b321300b681a224 Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Fri, 29 Jun 2018 16:44:54 -0700 Subject: [PATCH] fix(ivy): correct position for re-projected containers (#24721) PR Close #24721 --- packages/core/src/render3/i18n.ts | 10 +- packages/core/src/render3/instructions.ts | 45 ++- .../core/src/render3/node_manipulation.ts | 2 +- packages/core/test/render3/content_spec.ts | 320 +++++++++++++++++- .../test/render3/view_container_ref_spec.ts | 32 +- 5 files changed, 356 insertions(+), 53 deletions(-) diff --git a/packages/core/src/render3/i18n.ts b/packages/core/src/render3/i18n.ts index 350b1a4f28..4262df274a 100644 --- a/packages/core/src/render3/i18n.ts +++ b/packages/core/src/render3/i18n.ts @@ -9,7 +9,7 @@ import {assertEqual, assertLessThan} from './assert'; import {NO_CHANGE, bindingUpdated, createLNode, getPreviousOrParentNode, getRenderer, getViewData, load, resetApplicationState} from './instructions'; import {RENDER_PARENT} from './interfaces/container'; -import {LContainerNode, LElementNode, LNode, TNodeType} from './interfaces/node'; +import {LContainerNode, LElementNode, LNode, TContainerNode, TNodeType} from './interfaces/node'; import {BINDING_INDEX, HEADER_OFFSET, TVIEW} from './interfaces/view'; import {appendChild, createTextNode, getParentLNode, removeChild} from './node_manipulation'; import {stringify} from './util'; @@ -241,7 +241,8 @@ function appendI18nNode(node: LNode, parentNode: LNode, previousNode: LNode) { appendChild(parentNode, node.native || null, viewData); // On first pass, re-organize node tree to put this node in the correct position. - if (node.view[TVIEW].firstTemplatePass) { + const firstTemplatePass = node.view[TVIEW].firstTemplatePass; + if (firstTemplatePass) { node.tNode.next = null; if (previousNode === parentNode && node.tNode !== parentNode.tNode.child) { node.tNode.next = parentNode.tNode.child; @@ -257,7 +258,10 @@ function appendI18nNode(node: LNode, parentNode: LNode, previousNode: LNode) { // (node.native as RComment).textContent = 'test'; // console.log(node.native); appendChild(parentNode, node.dynamicLContainerNode.native || null, viewData); - node.pNextOrParent = node.dynamicLContainerNode; + if (firstTemplatePass) { + node.tNode.dynamicContainerNode = node.dynamicLContainerNode.tNode; + node.dynamicLContainerNode.tNode.parent = node.tNode as TContainerNode; + } return node.dynamicLContainerNode; } diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 208e91d740..39777958b4 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -1920,7 +1920,7 @@ export function projectionDef( * @param appendedFirst First node of the linked list to append. * @param appendedLast Last node of the linked list to append. */ -function appendToProjectionNode( +function addToProjectionList( projectionNode: LProjectionNode, appendedFirst: LElementNode | LTextNode | LContainerNode | null, appendedLast: LElementNode | LTextNode | LContainerNode | null) { @@ -1965,36 +1965,33 @@ export function projection( const distributedNodes = loadInternal(localIndex, componentLView) as Array; const nodesForSelector = distributedNodes[selectorIndex]; - // build the linked list of projected nodes: + const currentParent = getParentLNode(node); + const canInsert = canInsertNativeNode(currentParent, viewData); + const renderParent = currentParent.tNode.type === TNodeType.View ? + (getParentLNode(currentParent) as LContainerNode).data[RENDER_PARENT] ! : + currentParent as LElementNode; + for (let i = 0; i < nodesForSelector.length; i++) { const nodeToProject = nodesForSelector[i]; + let head = nodeToProject as LTextNode | LElementNode | LContainerNode | null; + let tail = nodeToProject as LTextNode | LElementNode | LContainerNode | null; + if (nodeToProject.tNode.type === TNodeType.Projection) { - // Reprojecting a projection -> append the list of previously projected nodes const previouslyProjected = (nodeToProject as LProjectionNode).data; - appendToProjectionNode(node, previouslyProjected.head, previouslyProjected.tail); - } else { - // Projecting a single node - appendToProjectionNode( - node, nodeToProject as LTextNode | LElementNode | LContainerNode, - nodeToProject as LTextNode | LElementNode | LContainerNode); + head = previouslyProjected.head; + tail = previouslyProjected.tail; } - } - const currentParent = getParentLNode(node); - if (canInsertNativeNode(currentParent, viewData)) { - ngDevMode && assertNodeOfPossibleTypes(currentParent, TNodeType.Element, TNodeType.View); - // process each node in the list of projected nodes: - let nodeToProject: LNode|null = node.data.head; - const lastNodeToProject = node.data.tail; - const renderParent = currentParent.tNode.type === TNodeType.View ? - (getParentLNode(currentParent) as LContainerNode).data[RENDER_PARENT] ! : - currentParent as LElementNode; + addToProjectionList(node, head, tail); - while (nodeToProject) { - appendProjectedNode( - nodeToProject as LTextNode | LElementNode | LContainerNode, currentParent, viewData, - renderParent); - nodeToProject = nodeToProject === lastNodeToProject ? null : nodeToProject.pNextOrParent; + if (canInsert) { + let currentNode: LNode|null = head; + while (currentNode) { + appendProjectedNode( + currentNode as LTextNode | LElementNode | LContainerNode, currentParent, viewData, + renderParent); + currentNode = currentNode === tail ? null : currentNode.pNextOrParent; + } } } } diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index d577a15c9b..95a4a35715 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -618,7 +618,7 @@ export function appendProjectedNode( lContainer[RENDER_PARENT] = renderParent; const views = lContainer[VIEWS]; for (let i = 0; i < views.length; i++) { - addRemoveViewFromContainer(node as LContainerNode, views[i], true, null); + addRemoveViewFromContainer(node as LContainerNode, views[i], true, node.native); } } if (node.dynamicLContainerNode) { diff --git a/packages/core/test/render3/content_spec.ts b/packages/core/test/render3/content_spec.ts index a6c3316285..8f16a1fa00 100644 --- a/packages/core/test/render3/content_spec.ts +++ b/packages/core/test/render3/content_spec.ts @@ -45,12 +45,15 @@ describe('content projection', () => { }); it('should project content when root.', () => { + /** */ const Child = createComponent('child', function(rf: RenderFlags, ctx: any) { if (rf & RenderFlags.Create) { projectionDef(0); projection(1, 0); } }); + + /** content */ const Parent = createComponent('parent', function(rf: RenderFlags, ctx: any) { if (rf & RenderFlags.Create) { elementStart(0, 'child'); @@ -64,6 +67,7 @@ describe('content projection', () => { }); it('should re-project content when root.', () => { + /**
*/ const GrandChild = createComponent('grand-child', function(rf: RenderFlags, ctx: any) { if (rf & RenderFlags.Create) { projectionDef(0); @@ -72,6 +76,8 @@ describe('content projection', () => { elementEnd(); } }); + + /** */ const Child = createComponent('child', function(rf: RenderFlags, ctx: any) { if (rf & RenderFlags.Create) { projectionDef(0); @@ -80,6 +86,8 @@ describe('content projection', () => { elementEnd(); } }, [GrandChild]); + + /** HelloWorld! */ const Parent = createComponent('parent', function(rf: RenderFlags, ctx: any) { if (rf & RenderFlags.Create) { elementStart(0, 'child'); @@ -137,6 +145,59 @@ describe('content projection', () => { .toEqual('
content
'); }); + it('should project components that have their own projection', () => { + /**
*/ + const Child = createComponent('child', (rf: RenderFlags, ctx: any) => { + if (rf & RenderFlags.Create) { + projectionDef(0); + elementStart(1, 'div'); + { projection(2, 0); } + elementEnd(); + } + }); + + /**

*/ + const ProjectedComp = createComponent('projected-comp', (rf: RenderFlags, ctx: any) => { + if (rf & RenderFlags.Create) { + projectionDef(0); + elementStart(1, 'p'); + projection(2, 0); + elementEnd(); + } + }); + + /** + * + * + *
Some content
+ * Other content + * + * + */ + const Parent = createComponent('parent', (rf: RenderFlags, ctx: any) => { + if (rf & RenderFlags.Create) { + elementStart(0, 'child'); + { + elementStart(1, 'projected-comp'); + { + elementStart(2, 'div'); + text(3, 'Some content'); + elementEnd(); + text(4, 'Other content'); + } + + elementEnd(); + } + elementEnd(); + } + }, [Child, ProjectedComp]); + + const parent = renderComponent(Parent); + expect(toHtml(parent)) + .toEqual( + '

Some content
Other content

'); + }); + it('should project content with container.', () => { /**
*/ const Child = createComponent('child', function(rf: RenderFlags, ctx: any) { @@ -366,8 +427,8 @@ describe('content projection', () => { /** *
* % if (!skipContent) { - * - * % } + * + * % } *
*/ const Child = createComponent('child', function(rf: RenderFlags, ctx: any) { @@ -414,7 +475,89 @@ describe('content projection', () => { expect(toHtml(parent)).toEqual('
'); }); - it('should project containers into embedded views', () => { + it('should project containers into containers', () => { + /** + *
+ * Before (inside) + * % if (!skipContent) { + * + * % } + * After (inside) + *
+ */ + const Child = createComponent('child', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + projectionDef(0); + elementStart(1, 'div'); + { + text(2, 'Before (inside)-'); + container(3); + text(4, '-After (inside)'); + } + elementEnd(); + } + if (rf & RenderFlags.Update) { + containerRefreshStart(3); + { + if (!ctx.skipContent) { + let rf0 = embeddedViewStart(0); + if (rf0 & RenderFlags.Create) { + projection(0, 0); + } + embeddedViewEnd(); + } + } + containerRefreshEnd(); + } + }); + + /** + * + * Before text- + * % if (!skipContent) { + * content + * % } + * -After text + * + */ + const Parent = createComponent('parent', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'child'); + { + text(1, 'Before text-'); + container(2); + text(3, '-After text'); + } + elementEnd(); + } + if (rf & RenderFlags.Update) { + containerRefreshStart(2); + { + if (!ctx.skipContent) { + let rf0 = embeddedViewStart(0); + if (rf0 & RenderFlags.Create) { + text(0, 'content'); + } + embeddedViewEnd(); + } + } + containerRefreshEnd(); + } + }, [Child]); + + const fixture = new ComponentFixture(Parent); + expect(fixture.html) + .toEqual( + '
Before (inside)-Before text-content-After text-After (inside)
'); + + fixture.component.skipContent = true; + fixture.update(); + expect(fixture.html) + .toEqual( + '
Before (inside)-Before text--After text-After (inside)
'); + }); + + it('should re-project containers into containers', () => { /** *
* % if (!skipContent) { @@ -446,24 +589,31 @@ describe('content projection', () => { /** * + * Before text * % if (!skipContent) { - * content + * * % } + * -After text * */ const Parent = createComponent('parent', function(rf: RenderFlags, ctx: any) { if (rf & RenderFlags.Create) { - elementStart(0, 'child'); - { container(1); } + projectionDef(0); + elementStart(1, 'child'); + { + text(2, 'Before text'); + container(3); + text(4, '-After text'); + } elementEnd(); } if (rf & RenderFlags.Update) { - containerRefreshStart(1); + containerRefreshStart(3); { if (!ctx.skipContent) { let rf0 = embeddedViewStart(0); if (rf0 & RenderFlags.Create) { - text(0, 'content'); + projection(0, 0); } embeddedViewEnd(); } @@ -472,14 +622,33 @@ describe('content projection', () => { } }, [Child]); - const fixture = new ComponentFixture(Parent); - expect(fixture.html).toEqual('
content
'); + let parent: any; + /**

text

 */ + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'parent'); + { + elementStart(1, 'p'); + { text(2, 'text'); } + elementEnd(); + } + elementEnd(); + // testing + parent = loadDirective(0); + } + }, [Parent]); - fixture.component.skipContent = true; + const fixture = new ComponentFixture(App); + expect(fixture.html) + .toEqual('
Before text

text

-After text
'); + + parent.skipContent = true; fixture.update(); - expect(fixture.html).toEqual('
'); + expect(fixture.html) + .toEqual('
Before text-After text
'); }); + it('should support projection in embedded views when ng-content is a root node of an embedded view, with other nodes after', () => { let childCmptInstance: any; @@ -487,8 +656,8 @@ describe('content projection', () => { /** *
* % if (!skipContent) { - * before--after - * % } + * before--after + * % } *
*/ const Child = createComponent('child', function(rf: RenderFlags, ctx: any) { @@ -633,6 +802,129 @@ describe('content projection', () => { expect(toHtml(parent)).toEqual('
content
'); }); + it('should project with multiple instances of a component with projection', () => { + const ProjectionComp = createComponent('projection-comp', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + projectionDef(0); + text(1, 'Before'); + projection(2, 0); + text(3, 'After'); + } + }); + + /** + * + *
A
+ *

123

+ * + * + *
B
+ *

456

+ * + */ + const AppComp = createComponent('app-comp', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'projection-comp'); + elementStart(1, 'div'); + text(2, 'A'); + elementEnd(); + elementStart(3, 'p'); + text(4, '123'); + elementEnd(); + elementEnd(); + elementStart(5, 'projection-comp'); + elementStart(6, 'div'); + text(7, 'B'); + elementEnd(); + elementStart(8, 'p'); + text(9, '456'); + elementEnd(); + elementEnd(); + } + }, [ProjectionComp]); + + const fixture = new ComponentFixture(AppComp); + fixture.update(); + expect(fixture.html) + .toEqual( + 'Before
A

123

AfterBefore
B

456

After'); + }); + + it('should re-project with multiple instances of a component with projection', () => { + /** + * Before + * + * After + */ + const ProjectionComp = createComponent('projection-comp', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + projectionDef(0); + text(1, 'Before'); + projection(2, 0); + text(3, 'After'); + } + }); + + /** + * + *
A
+ * + *

123

+ * + * + *
B
+ *

456

+ * + */ + const ProjectionParent = createComponent('parent-comp', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + projectionDef(0); + elementStart(1, 'projection-comp'); + elementStart(2, 'div'); + text(3, 'A'); + elementEnd(); + projection(4, 0); + elementStart(5, 'p'); + text(6, '123'); + elementEnd(); + elementEnd(); + elementStart(7, 'projection-comp'); + elementStart(8, 'div'); + text(9, 'B'); + elementEnd(); + elementStart(10, 'p'); + text(11, '456'); + elementEnd(); + elementEnd(); + } + }, [ProjectionComp]); + + /** + * + * **ABC** + * + * + * **DEF** + * + */ + const AppComp = createComponent('app-comp', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'parent-comp'); + text(1, '**ABC**'); + elementEnd(); + elementStart(2, 'parent-comp'); + text(3, '**DEF**'); + elementEnd(); + } + }, [ProjectionParent]); + + const fixture = new ComponentFixture(AppComp); + fixture.update(); + expect(fixture.html) + .toEqual( + 'Before
A
**ABC**

123

AfterBefore
B

456

AfterBefore
A
**DEF**

123

AfterBefore
B

456

After'); + }); + describe('with selectors', () => { it('should project nodes using attribute selectors', () => { diff --git a/packages/core/test/render3/view_container_ref_spec.ts b/packages/core/test/render3/view_container_ref_spec.ts index 34a300ab4b..75ed3c4502 100644 --- a/packages/core/test/render3/view_container_ref_spec.ts +++ b/packages/core/test/render3/view_container_ref_spec.ts @@ -727,9 +727,12 @@ describe('ViewContainerRef', () => { @Component({ selector: 'child-with-view', template: ` - % if (show) { - - % }` + Before (inside)- + % if (show) { + + % } + After (inside) + ` }) class ChildWithView { show: boolean = true; @@ -740,10 +743,12 @@ describe('ViewContainerRef', () => { template: (rf: RenderFlags, cmp: ChildWithView) => { if (rf & RenderFlags.Create) { projectionDef(0); - container(1); + text(1, 'Before (inside)-'); + container(2); + text(3, 'After (inside)'); } if (rf & RenderFlags.Update) { - containerRefreshStart(1); + containerRefreshStart(2); if (cmp.show) { let rf0 = embeddedViewStart(0); if (rf0 & RenderFlags.Create) { @@ -764,7 +769,9 @@ describe('ViewContainerRef', () => { {{name}} + Before projected
blah
+ After projected ` }) class Parent { @@ -777,15 +784,17 @@ describe('ViewContainerRef', () => { if (rf & RenderFlags.Create) { container(0, embeddedTemplate); elementStart(1, 'child-with-view'); - elementStart(2, 'header', ['vcref', '']); - text(3, 'blah'); + text(2, 'Before projected'); + elementStart(3, 'header', ['vcref', '']); + text(4, 'blah'); elementEnd(); + text(5, 'After projected-'); elementEnd(); } if (rf & RenderFlags.Update) { const tplRef = getOrCreateTemplateRef(getOrCreateNodeInjectorForNode(load(0))); - elementProperty(2, 'tplRef', bind(tplRef)); - elementProperty(2, 'name', bind(cmp.name)); + elementProperty(3, 'tplRef', bind(tplRef)); + elementProperty(3, 'name', bind(cmp.name)); } }, directives: [ChildWithView, DirectiveWithVCRef] @@ -794,13 +803,14 @@ describe('ViewContainerRef', () => { const fixture = new ComponentFixture(Parent); expect(fixture.html) - .toEqual('
blah
'); + .toEqual( + 'Before (inside)-Before projected
blah
After projected-After (inside)'); directiveInstance !.vcref.createEmbeddedView(directiveInstance !.tplRef, fixture.component); fixture.update(); expect(fixture.html) .toEqual( - '
blah
bar'); + 'Before (inside)-Before projected
blah
barAfter projected-After (inside)'); }); describe('with select', () => {