diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index d86a3d0678..20969d5c6f 100644 --- a/packages/core/src/render3/di.ts +++ b/packages/core/src/render3/di.ts @@ -556,10 +556,12 @@ export function getOrCreateContainerRef(di: LInjector): viewEngine_ViewContainer ngDevMode && assertNodeOfPossibleTypes(vcRefHost, LNodeType.Container, LNodeType.Element); - const lContainer = createLContainer(vcRefHost.parent !, vcRefHost.view, undefined, vcRefHost); + const lContainer = createLContainer(vcRefHost.parent !, vcRefHost.view); const lContainerNode: LContainerNode = createLNodeObject( LNodeType.Container, vcRefHost.view, vcRefHost.parent !, undefined, lContainer, null); + vcRefHost.dynamicLContainerNode = lContainerNode; + addToViewTree(vcRefHost.view, lContainer); di.viewContainerRef = new ViewContainerRef(lContainerNode); @@ -608,6 +610,10 @@ class ViewContainerRef implements viewEngine_ViewContainerRef { const adjustedIdx = this._adjustAndAssertIndex(index); insertView(this._lContainerNode, lViewNode, adjustedIdx); + // invalidate cache of next sibling RNode (we do similar operation in the containerRefreshEnd + // instruction) + this._lContainerNode.native = undefined; + this._viewRefs.splice(adjustedIdx, 0, viewRef); (lViewNode as{parent: LNode}).parent = this._lContainerNode; diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 127a34721b..8ea257c9d9 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -318,7 +318,8 @@ export function createLNodeObject( data: state, queries: queries, tNode: null, - pNextOrParent: null + pNextOrParent: null, + dynamicLContainerNode: null }; } @@ -386,6 +387,9 @@ export function createLNode( previousOrParentNode.next, `previousOrParentNode's next property should not have been set ${index}.`); previousOrParentNode.next = node; + if (previousOrParentNode.dynamicLContainerNode) { + previousOrParentNode.dynamicLContainerNode.next = node; + } } } previousOrParentNode = node; @@ -452,9 +456,10 @@ export function renderEmbeddedTemplate( const directives = currentView && currentView.tView.directiveRegistry; const pipes = currentView && currentView.tView.pipeRegistry; - const view = createLView( - -1, renderer, createTView(directives, pipes), template, context, LViewFlags.CheckAlways); - viewNode = createLNode(null, LNodeType.View, null, view); + const tView = getOrCreateTView(template, directives, pipes); + const lView = createLView(-1, renderer, tView, template, context, LViewFlags.CheckAlways); + + viewNode = createLNode(null, LNodeType.View, null, lView); cm = true; } oldView = enterView(viewNode.data, viewNode); @@ -1311,8 +1316,7 @@ function generateInitialInputs( export function createLContainer( - parentLNode: LNode, currentView: LView, template?: ComponentTemplate, - host?: LContainerNode | LElementNode): LContainer { + parentLNode: LNode, currentView: LView, template?: ComponentTemplate): LContainer { ngDevMode && assertNotNull(parentLNode, 'containers should have a parent'); return { views: [], @@ -1324,8 +1328,7 @@ export function createLContainer( next: null, parent: currentView, dynamicViewCount: 0, - queries: null, - host: host == null ? null : host + queries: null }; } diff --git a/packages/core/src/render3/interfaces/container.ts b/packages/core/src/render3/interfaces/container.ts index 3790d3444e..46e0a54966 100644 --- a/packages/core/src/render3/interfaces/container.ts +++ b/packages/core/src/render3/interfaces/container.ts @@ -80,13 +80,6 @@ export interface LContainer { * this container are reported to queries referenced here. */ queries: LQueries|null; - - /** - * If a LContainer is created dynamically (by a directive requesting ViewContainerRef) this fields - * keeps a reference to a node on which a ViewContainerRef was requested. We need to store this - * information to find a next render sibling node. - */ - host: LContainerNode|LElementNode|null; } /** diff --git a/packages/core/src/render3/interfaces/node.ts b/packages/core/src/render3/interfaces/node.ts index 338e984dcc..0aadede6b3 100644 --- a/packages/core/src/render3/interfaces/node.ts +++ b/packages/core/src/render3/interfaces/node.ts @@ -130,6 +130,11 @@ export interface LNode { * data about this node. */ tNode: TNode|null; + + /** + * A pointer to a LContainerNode created by directives requesting ViewContainerRef + */ + dynamicLContainerNode: LContainerNode|null; } @@ -158,6 +163,7 @@ export interface LTextNode extends LNode { /** LTextNodes can be inside LElementNodes or inside LViewNodes. */ readonly parent: LElementNode|LViewNode; readonly data: null; + dynamicLContainerNode: null; } /** Abstract node which contains root nodes of a view. */ @@ -169,6 +175,7 @@ export interface LViewNode extends LNode { /** LViewNodes can only be added to LContainerNodes. */ readonly parent: LContainerNode|null; readonly data: LView; + dynamicLContainerNode: null; } /** Abstract node container which contains other views. */ @@ -199,6 +206,7 @@ export interface LProjectionNode extends LNode { /** Projections can be added to elements or views. */ readonly parent: LElementNode|LViewNode; + dynamicLContainerNode: null; } /** diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index 906185f28d..eda51bc2b2 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -125,8 +125,10 @@ function findFirstRNode(rootNode: LNode): RElement|RText|null { // A LElementNode has a matching RNode in LElementNode.native return (node as LElementNode).native; } else if (node.type === LNodeType.Container) { - // For container look at the first node of the view next - const childContainerData: LContainer = (node as LContainerNode).data; + const lContainerNode: LContainerNode = (node as LContainerNode); + const childContainerData: LContainer = lContainerNode.dynamicLContainerNode ? + lContainerNode.dynamicLContainerNode.data : + lContainerNode.data; nextNode = childContainerData.views.length ? childContainerData.views[0].child : null; } else if (node.type === LNodeType.Projection) { // For Projection look at the first projected node @@ -281,10 +283,7 @@ export function insertView( if (!beforeNode) { let containerNextNativeNode = container.native; if (containerNextNativeNode === undefined) { - // TODO(pk): this is probably too simplistic, add more tests for various host placements - // (dynamic view, projection, ...) - containerNextNativeNode = container.native = - findNextRNodeSibling(container.data.host ? container.data.host : container, null); + containerNextNativeNode = container.native = findNextRNodeSibling(container, null); } beforeNode = containerNextNativeNode; } diff --git a/packages/core/test/render3/view_container_ref_spec.ts b/packages/core/test/render3/view_container_ref_spec.ts index 5e7ee86aa0..f13dd0772d 100644 --- a/packages/core/test/render3/view_container_ref_spec.ts +++ b/packages/core/test/render3/view_container_ref_spec.ts @@ -9,7 +9,7 @@ import {TemplateRef, ViewContainerRef} from '../../src/core'; import {getOrCreateNodeInjectorForNode, getOrCreateTemplateRef} from '../../src/render3/di'; import {defineComponent, defineDirective, injectTemplateRef, injectViewContainerRef} from '../../src/render3/index'; -import {bind, container, elementEnd, elementProperty, elementStart, interpolation1, load, loadDirective, text, textBinding} from '../../src/render3/instructions'; +import {bind, container, containerRefreshEnd, containerRefreshStart, elementEnd, elementProperty, elementStart, embeddedViewEnd, embeddedViewStart, interpolation1, load, loadDirective, text, textBinding} from '../../src/render3/instructions'; import {ComponentFixture} from './render_util'; @@ -190,4 +190,161 @@ describe('ViewContainerRef', () => { expect(fixture.html).toEqual('before||after'); }); + it('should add embedded views at the right position in the DOM tree (ng-template next to other ng-template)', + () => { + let directiveInstances: TestDirective[] = []; + + class TestDirective { + static ngDirectiveDef = defineDirective({ + type: TestDirective, + selectors: [['', 'testdir', '']], + factory: () => { + const instance = new TestDirective(injectViewContainerRef(), injectTemplateRef()); + + directiveInstances.push(instance); + + return instance; + } + }); + + constructor(private _vcRef: ViewContainerRef, private _tplRef: TemplateRef<{}>) {} + + insertTpl(ctx: {}) { this._vcRef.createEmbeddedView(this._tplRef, ctx); } + + remove(index?: number) { this._vcRef.remove(index); } + } + + function EmbeddedTemplateA(ctx: any, cm: boolean) { + if (cm) { + text(0, 'A'); + } + } + + function EmbeddedTemplateB(ctx: any, cm: boolean) { + if (cm) { + text(0, 'B'); + } + } + + /** + * before| + * A + * B + * |after + */ + class TestComponent { + testDir: TestDirective; + static ngComponentDef = defineComponent({ + type: TestComponent, + selectors: [['test-cmp']], + factory: () => new TestComponent(), + template: (cmp: TestComponent, cm: boolean) => { + if (cm) { + text(0, 'before|'); + container(1, EmbeddedTemplateA, undefined, ['testdir', '']); + container(2, EmbeddedTemplateB, undefined, ['testdir', '']); + text(3, '|after'); + } + }, + directives: [TestDirective] + }); + } + + const fixture = new ComponentFixture(TestComponent); + expect(fixture.html).toEqual('before||after'); + + directiveInstances ![1].insertTpl({}); + expect(fixture.html).toEqual('before|B|after'); + + directiveInstances ![0].insertTpl({}); + expect(fixture.html).toEqual('before|AB|after'); + }); + + + it('should add embedded views at the right position in the DOM tree (ng-template next to a JS block)', + () => { + let directiveInstance: TestDirective; + + class TestDirective { + static ngDirectiveDef = defineDirective({ + type: TestDirective, + selectors: [['', 'testdir', '']], + factory: () => directiveInstance = + new TestDirective(injectViewContainerRef(), injectTemplateRef()) + }); + + constructor(private _vcRef: ViewContainerRef, private _tplRef: TemplateRef<{}>) {} + + insertTpl(ctx: {}) { this._vcRef.createEmbeddedView(this._tplRef, ctx); } + + remove(index?: number) { this._vcRef.remove(index); } + } + + function EmbeddedTemplateA(ctx: any, cm: boolean) { + if (cm) { + text(0, 'A'); + } + } + + /** + * before| + * A + * % if (condition) { + * B + * } + * |after + */ + class TestComponent { + condition = false; + testDir: TestDirective; + static ngComponentDef = defineComponent({ + type: TestComponent, + selectors: [['test-cmp']], + factory: () => new TestComponent(), + template: (cmp: TestComponent, cm: boolean) => { + if (cm) { + text(0, 'before|'); + container(1, EmbeddedTemplateA, undefined, ['testdir', '']); + container(2); + text(3, '|after'); + } + containerRefreshStart(2); + { + if (cmp.condition) { + let cm1 = embeddedViewStart(0); + { + if (cm1) { + text(0, 'B'); + } + } + embeddedViewEnd(); + } + } + containerRefreshEnd(); + }, + directives: [TestDirective] + }); + } + + const fixture = new ComponentFixture(TestComponent); + expect(fixture.html).toEqual('before||after'); + + fixture.component.condition = true; + fixture.update(); + expect(fixture.html).toEqual('before|B|after'); + + directiveInstance !.insertTpl({}); + expect(fixture.html).toEqual('before|AB|after'); + + fixture.component.condition = false; + fixture.update(); + expect(fixture.html).toEqual('before|A|after'); + + directiveInstance !.insertTpl({}); + expect(fixture.html).toEqual('before|AA|after'); + + fixture.component.condition = true; + fixture.update(); + expect(fixture.html).toEqual('before|AAB|after'); + }); });