From 3355502f2fd296a556fc606ef1f68729aed20cb6 Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Mon, 6 Aug 2018 16:40:16 +0200 Subject: [PATCH] fix(ivy): support ng-container at the root of a view with delayed insertion (#25329) PR Close #25329 --- .../core/src/render3/node_manipulation.ts | 90 ++++++++++++------- .../hello_world/bundle.golden_symbols.json | 6 ++ .../bundling/todo/bundle.golden_symbols.json | 6 ++ .../core/test/render3/integration_spec.ts | 61 ++++++++++++- 4 files changed, 128 insertions(+), 35 deletions(-) diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index 17f28171f0..b8ae8a2faf 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -42,6 +42,8 @@ export function getParentLNode( node: LContainerNode | LElementNode | LElementContainerNode | LTextNode | LProjectionNode): LElementNode|LElementContainerNode|LViewNode; export function getParentLNode(node: LViewNode): LContainerNode|null; +export function getParentLNode(node: LElementContainerNode): LElementNode|LElementContainerNode| + LViewNode; export function getParentLNode(node: LNode): LElementNode|LElementContainerNode|LContainerNode| LViewNode|null; export function getParentLNode(node: LNode): LElementNode|LElementContainerNode|LContainerNode| @@ -510,6 +512,46 @@ function executePipeOnDestroys(viewData: LViewData): void { } } +function canInsertNativeChildOfElement(parent: LElementNode, currentView: LViewData): boolean { + if (parent.view !== currentView) { + // If the Parent view is not the same as current view than we are inserting across + // Views. This happens when we insert a root element of the component view into + // the component host element and it should always be eager. + return true; + } + // Parent elements can be a component which may have projection. + if (parent.data === null) { + // Parent is a regular non-component element. We should eagerly insert into it + // since we know that this relationship will never be broken. + return true; + } + + // Parent is a Component. Component's content nodes are not inserted immediately + // because they will be projected, and so doing insert at this point would be wasteful. + // Since the projection would than move it to its final destination. + return false; +} + +function canInsertNativeChildOfView(parent: LViewNode): boolean { + ngDevMode && assertNodeType(parent, TNodeType.View); + + // Because we are inserting into a `View` the `View` may be disconnected. + const grandParentContainer = getParentLNode(parent) as LContainerNode; + if (grandParentContainer == null) { + // The `View` is not inserted into a `Container` we have to delay insertion. + return false; + } + ngDevMode && assertNodeType(grandParentContainer, TNodeType.Container); + if (grandParentContainer.data[RENDER_PARENT] == null) { + // The parent `Container` itself is disconnected. So we have to delay. + return false; + } + + // The parent `Container` is in inserted state, so we can eagerly insert into + // this location. + return true; +} + /** * Returns whether a native element can be inserted into the given parent. * @@ -533,44 +575,26 @@ export function canInsertNativeNode(parent: LNode, currentView: LViewData): bool ngDevMode && assertNodeOfPossibleTypes( parent, TNodeType.Element, TNodeType.ElementContainer, TNodeType.View); - if (parent.tNode.type === TNodeType.Element || parent.tNode.type === TNodeType.ElementContainer) { - // Parent is an element. - if (parent.view !== currentView) { - // If the Parent view is not the same as current view than we are inserting across - // Views. This happens when we insert a root element of the component view into - // the component host element and it should always be eager. - return true; + if (parent.tNode.type === TNodeType.Element) { + // Parent is a regular element or a component + return canInsertNativeChildOfElement(parent as LElementNode, currentView); + } else if (parent.tNode.type === TNodeType.ElementContainer) { + // Parent is an element container (ng-container). + // Its grand-parent might be an element, view or a sequence of ng-container parents. + let grandParent = getParentLNode(parent); + while (grandParent !== null && grandParent.tNode.type === TNodeType.ElementContainer) { + grandParent = getParentLNode(grandParent); } - // Parent elements can be a component which may have projection. - if (parent.data === null) { - // Parent is a regular non-component element. We should eagerly insert into it - // since we know that this relationship will never be broken. - return true; - } else { - // Parent is a Component. Component's content nodes are not inserted immediately - // because they will be projected, and so doing insert at this point would be wasteful. - // Since the projection would than move it to its final destination. + if (grandParent === null) { return false; + } else if (grandParent.tNode.type === TNodeType.Element) { + return canInsertNativeChildOfElement(grandParent as LElementNode, currentView); + } else { + return canInsertNativeChildOfView(grandParent as LViewNode); } } else { // Parent is a View. - ngDevMode && assertNodeType(parent, TNodeType.View); - - // Because we are inserting into a `View` the `View` may be disconnected. - const grandParentContainer = getParentLNode(parent) as LContainerNode; - if (grandParentContainer == null) { - // The `View` is not inserted into a `Container` we have to delay insertion. - return false; - } - ngDevMode && assertNodeType(grandParentContainer, TNodeType.Container); - if (grandParentContainer.data[RENDER_PARENT] == null) { - // The parent `Container` itself is disconnected. So we have to delay. - return false; - } else { - // The parent `Container` is in inserted state, so we can eagerly insert into - // this location. - return true; - } + return canInsertNativeChildOfView(parent as LViewNode); } } diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json index f0cc947b6e..5e2a1f5010 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -110,6 +110,12 @@ { "name": "callHooks" }, + { + "name": "canInsertNativeChildOfElement" + }, + { + "name": "canInsertNativeChildOfView" + }, { "name": "canInsertNativeNode" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 9621c4c701..5d341add12 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -353,6 +353,12 @@ { "name": "callHooks" }, + { + "name": "canInsertNativeChildOfElement" + }, + { + "name": "canInsertNativeChildOfView" + }, { "name": "canInsertNativeNode" }, diff --git a/packages/core/test/render3/integration_spec.ts b/packages/core/test/render3/integration_spec.ts index 66e9ce7dcd..c72b1c7953 100644 --- a/packages/core/test/render3/integration_spec.ts +++ b/packages/core/test/render3/integration_spec.ts @@ -6,10 +6,11 @@ * found in the LICENSE file at https://angular.io/license */ -import {ElementRef} from '@angular/core'; +import {ElementRef, TemplateRef, ViewContainerRef} from '@angular/core'; import {RenderFlags} from '@angular/core/src/render3'; -import {AttributeMarker, defineComponent, defineDirective, injectElementRef} from '../../src/render3/index'; +import {getOrCreateNodeInjectorForNode, getOrCreateTemplateRef} from '../../src/render3/di'; +import {AttributeMarker, defineComponent, defineDirective, injectElementRef, injectTemplateRef, injectViewContainerRef} from '../../src/render3/index'; import {NO_CHANGE, bind, container, containerRefreshEnd, containerRefreshStart, element, elementAttribute, elementClassProp, elementContainerEnd, elementContainerStart, elementEnd, elementProperty, elementStart, elementStyleProp, elementStyling, elementStylingApply, embeddedViewEnd, embeddedViewStart, interpolation1, interpolation2, interpolation3, interpolation4, interpolation5, interpolation6, interpolation7, interpolation8, interpolationV, listener, load, loadDirective, projection, projectionDef, text, textBinding} from '../../src/render3/instructions'; import {InitialStylingFlags} from '../../src/render3/interfaces/definition'; import {HEADER_OFFSET} from '../../src/render3/interfaces/view'; @@ -576,6 +577,62 @@ describe('render3 integration test', () => { expect(fixture.html).toEqual(''); }); + // https://stackblitz.com/edit/angular-tfhcz1?file=src%2Fapp%2Fapp.component.ts + it('should add and remove DOM nodes when ng-container is a child of a delayed embedded view', + () => { + + class TestDirective { + constructor(private _tplRef: TemplateRef, private _vcRef: ViewContainerRef) {} + + createAndInsert() { this._vcRef.insert(this._tplRef.createEmbeddedView({})); } + + clear() { this._vcRef.clear(); } + + static ngDirectiveDef = defineDirective({ + type: TestDirective, + selectors: [['', 'testDirective', '']], + factory: () => new TestDirective(injectTemplateRef(), injectViewContainerRef()), + }); + } + + + function embeddedTemplate(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementContainerStart(0); + { text(1, 'content'); } + elementContainerEnd(); + } + } + + let testDirective: TestDirective; + + + ` + + content + + `; + const TestCmpt = createComponent('test-cmpt', function(rf: RenderFlags) { + if (rf & RenderFlags.Create) { + container(0, embeddedTemplate, null, [AttributeMarker.SelectOnly, 'testDirective']); + } + if (rf & RenderFlags.Update) { + testDirective = loadDirective(0); + } + }, [TestDirective]); + + const fixture = new ComponentFixture(TestCmpt); + expect(fixture.html).toEqual(''); + + testDirective !.createAndInsert(); + fixture.update(); + expect(fixture.html).toEqual('content'); + + testDirective !.clear(); + fixture.update(); + expect(fixture.html).toEqual(''); + }); + it('should render at the component view root', () => { /** * component template