Skip to content

Commit

Permalink
fix(ivy): support ng-container at the root of a view with delayed ins…
Browse files Browse the repository at this point in the history
…ertion
  • Loading branch information
pkozlowski-opensource committed Aug 6, 2018
1 parent 795e1e8 commit 7bd6b3f
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 35 deletions.
90 changes: 57 additions & 33 deletions packages/core/src/render3/node_manipulation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down Expand Up @@ -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.
*
Expand All @@ -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(parent);
}
// 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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@
{
"name": "callHooks"
},
{
"name": "canInsertNativeChildOfElement"
},
{
"name": "canInsertNativeChildOfView"
},
{
"name": "canInsertNativeNode"
},
Expand Down
6 changes: 6 additions & 0 deletions packages/core/test/bundling/todo/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,12 @@
{
"name": "callHooks"
},
{
"name": "canInsertNativeChildOfElement"
},
{
"name": "canInsertNativeChildOfView"
},
{
"name": "canInsertNativeNode"
},
Expand Down
61 changes: 59 additions & 2 deletions packages/core/test/render3/integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<any>, 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;


`<ng-template testDirective>
<ng-container>
content
</ng-container>
</ng-template>`;
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<TestDirective>(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', () => {
/**
* <ng-container>component template</ng-container>
Expand Down

0 comments on commit 7bd6b3f

Please sign in to comment.