Skip to content

Commit

Permalink
fixup! fix(ivy): handle cases of inserting LView into LContainer
Browse files Browse the repository at this point in the history
  • Loading branch information
mhevery committed Nov 5, 2019
1 parent 6cbd41a commit 9465a38
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 31 deletions.
1 change: 0 additions & 1 deletion packages/core/src/render3/interfaces/projection.ts
@@ -1,4 +1,3 @@

/**
* @license
* Copyright Google Inc. All Rights Reserved.
Expand Down
64 changes: 35 additions & 29 deletions packages/core/src/render3/node_manipulation.ts
Expand Up @@ -8,7 +8,7 @@

import {ViewEncapsulation} from '../metadata/view';
import {addToArray, removeFromArray} from '../util/array_utils';
import {assertDefined, assertDomNode, assertEqual} from '../util/assert';
import {assertDefined, assertDomNode, assertEqual, assertNotSame} from '../util/assert';
import {assertLContainer, assertLView, assertTNodeForLView} from './assert';
import {attachPatchData} from './context_discovery';
import {CONTAINER_HEADER_OFFSET, LContainer, MOVED_VIEWS, NATIVE, unusedValueExportToPlacateAjd as unused1} from './interfaces/container';
Expand All @@ -21,7 +21,7 @@ import {isLContainer, isLView, isRootView} from './interfaces/type_checks';
import {CHILD_HEAD, CLEANUP, DECLARATION_LCONTAINER, FLAGS, HOST, HookData, LView, LViewFlags, NEXT, PARENT, QUERIES, RENDERER, TVIEW, T_HOST, unusedValueExportToPlacateAjd as unused5} from './interfaces/view';
import {assertNodeOfPossibleTypes, assertNodeType} from './node_assert';
import {findComponentView, getLViewParent} from './util/view_traversal_utils';
import {getNativeByTNode, getNativeByTNodeOrNull, unwrapRNode} from './util/view_utils';
import {getNativeByTNode, unwrapRNode} from './util/view_utils';

const unusedValueToPlacateAjd = unused1 + unused2 + unused3 + unused4 + unused5;

Expand Down Expand Up @@ -64,6 +64,9 @@ const enum WalkTNodeTreeAction {

/** node destruction using the renderer's API */
Destroy = 3,

/** Retrieve the first RNode in search */
RetrieveRNode = 4,
}


Expand Down Expand Up @@ -689,24 +692,9 @@ export function getFirstRNodeFromLViewInLContainer(
const lView = lContainer[i];
let tNode = lView[TVIEW].firstChild;
if (tNode !== null) {
let tNodeType = tNode.type;
if (tNodeType === TNodeType.Container) {
// In this case the first `TNode` is another `LContainer` this means we now need to
// recurse
// into it the `LView` and retrieve its `RNode`.
const childLContainer = lView[tNode.index] as LContainer;
ngDevMode && assertLContainer(childLContainer);
return getFirstRNodeFromLViewInLContainer(childLContainer, 0);
} else {
// If we have `ElementContainer` or `IcuContainer` than we need to descend into them and
// find the actual first `Element`
while (tNodeType === TNodeType.ElementContainer || tNodeType === TNodeType.IcuContainer) {
tNode = tNode.child !;
ngDevMode && assertDefined(tNode, 'ElementContainer and IcuContainer must have children');
tNodeType = tNode.type;
}
return getNativeByTNode(tNode, lView);
}
const rNode =
applyNodes(null !, WalkTNodeTreeAction.RetrieveRNode, tNode, lView, null, null, false);
if (rNode !== null) return rNode;
}
}
// If we could not find any `RNode` in any of the `LView`s, than just use the `LContainer`'s
Expand Down Expand Up @@ -737,7 +725,7 @@ export function nativeRemoveNode(renderer: Renderer3, rNode: RNode, isHostElemen
*/
function applyNodes(
renderer: Renderer3, action: WalkTNodeTreeAction, tNode: TNode | null, lView: LView,
renderParent: RElement | null, beforeNode: RNode | null, isProjection: boolean) {
renderParent: RElement | null, beforeNode: RNode | null, isProjection: boolean): RNode|null {
while (tNode != null) {
ngDevMode && assertTNodeForLView(tNode, lView);
ngDevMode && assertNodeOfPossibleTypes(
Expand All @@ -753,18 +741,29 @@ function applyNodes(
}
if ((tNode.flags & TNodeFlags.isDetached) !== TNodeFlags.isDetached) {
if (tNodeType === TNodeType.ElementContainer || tNodeType === TNodeType.IcuContainer) {
applyNodes(renderer, action, tNode.child, lView, renderParent, beforeNode, false);
const rNode =
applyNodes(renderer, action, tNode.child, lView, renderParent, beforeNode, false);
if (action == WalkTNodeTreeAction.RetrieveRNode) return rNode;
applyToElementOrContainer(action, renderer, renderParent, rawSlotValue, beforeNode);
} else if (tNodeType === TNodeType.Projection) {
applyProjectionRecursive(
const rNode = applyProjectionRecursive(
renderer, action, lView, tNode as TProjectionNode, renderParent, beforeNode);
if (action == WalkTNodeTreeAction.RetrieveRNode) return rNode;
} else {
ngDevMode && assertNodeOfPossibleTypes(tNode, TNodeType.Element, TNodeType.Container);
if (action == WalkTNodeTreeAction.RetrieveRNode) {
if (isLContainer(rawSlotValue)) {
return applyContainer(renderer, action, rawSlotValue, null, null);
} else {
return unwrapRNode(rawSlotValue);
}
}
applyToElementOrContainer(action, renderer, renderParent, rawSlotValue, beforeNode);
}
}
tNode = isProjection ? tNode.projectionNext : tNode.next;
}
return null;
}


Expand Down Expand Up @@ -792,11 +791,11 @@ function applyNodes(
*/
function applyView(
renderer: Renderer3, action: WalkTNodeTreeAction, lView: LView, renderParent: RElement | null,
beforeNode: RNode | null) {
beforeNode: RNode | null): RNode|null {
const tView = lView[TVIEW];
ngDevMode && assertNodeType(tView.node !, TNodeType.View);
const viewRootTNode: TNode|null = tView.node !.child;
applyNodes(renderer, action, viewRootTNode, lView, renderParent, beforeNode, false);
return applyNodes(renderer, action, viewRootTNode, lView, renderParent, beforeNode, false);
}

/**
Expand Down Expand Up @@ -833,7 +832,8 @@ export function applyProjection(lView: LView, tProjectionNode: TProjectionNode)
*/
function applyProjectionRecursive(
renderer: Renderer3, action: WalkTNodeTreeAction, lView: LView,
tProjectionNode: TProjectionNode, renderParent: RElement | null, beforeNode: RNode | null) {
tProjectionNode: TProjectionNode, renderParent: RElement | null,
beforeNode: RNode | null): RNode|null {
const componentLView = findComponentView(lView);
const componentNode = componentLView[T_HOST] as TElementNode;
ngDevMode &&
Expand All @@ -847,14 +847,17 @@ function applyProjectionRecursive(
// This should be refactored and cleaned up.
for (let i = 0; i < nodeToProjectOrRNodes.length; i++) {
const rNode = nodeToProjectOrRNodes[i];
if (action == WalkTNodeTreeAction.RetrieveRNode && rNode !== null) return rNode;
applyToElementOrContainer(action, renderer, renderParent, rNode, beforeNode);
}
} else {
let nodeToProject: TNode|null = nodeToProjectOrRNodes;
const projectedComponentLView = componentLView[PARENT] as LView;
applyNodes(
const rNode = applyNodes(
renderer, action, nodeToProject, projectedComponentLView, renderParent, beforeNode, true);
if (rNode !== null) return rNode;
}
return null;
}


Expand All @@ -873,7 +876,7 @@ function applyProjectionRecursive(
*/
function applyContainer(
renderer: Renderer3, action: WalkTNodeTreeAction, lContainer: LContainer,
renderParent: RElement | null, beforeNode: RNode | null | undefined) {
renderParent: RElement | null, beforeNode: RNode | null | undefined): RNode|null {
ngDevMode && assertLContainer(lContainer);
const anchor = lContainer[NATIVE]; // LContainer has its own before node.
const native = unwrapRNode(lContainer);
Expand All @@ -888,10 +891,13 @@ function applyContainer(
// see a reason why they should be different, but they are.
//
// If they are we need to process the second anchor as well.
ngDevMode && assertNotSame(action, WalkTNodeTreeAction.RetrieveRNode, 'Unexpected code.')
applyToElementOrContainer(action, renderer, renderParent, anchor, beforeNode);
}
for (let i = CONTAINER_HEADER_OFFSET; i < lContainer.length; i++) {
const lView = lContainer[i] as LView;
applyView(renderer, action, lView, renderParent, anchor);
const rNode = applyView(renderer, action, lView, renderParent, anchor);
if (action === WalkTNodeTreeAction.RetrieveRNode) return rNode;
}
return action === WalkTNodeTreeAction.RetrieveRNode ? native : null;
}
71 changes: 70 additions & 1 deletion packages/core/test/acceptance/view_insertion_spec.ts
Expand Up @@ -8,6 +8,7 @@

import {CommonModule} from '@angular/common';
import {ChangeDetectorRef, Component, Directive, EmbeddedViewRef, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core';
import {getComponent} from '@angular/core/src/render3';
import {TestBed} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
import {ivyEnabled} from '@angular/private/testing';
Expand Down Expand Up @@ -278,6 +279,75 @@ describe('view insertion', () => {
});

describe('regression', () => {
it('should allow inserting of all different types', () => {
@Component({
selector: 'test-container-insertion',
template: `
<ng-template #container></ng-template>
<ng-template #empty></ng-template>
<ng-template #basic>(basic)</ng-template>
<ng-template #nested><ng-template [ngIf]="true">(nested)</ng-template></ng-template>
<ng-template #projection><ng-content></ng-content></ng-template>
<ng-template #icu>{count, plural, =0 {(icu)} other {(icu-other)}}</ng-template>
`
})
class TestContainerInsertionComponent {
count: number = 0;

@ViewChild('container', {read: ViewContainerRef})
container: ViewContainerRef = null !;

@ViewChild('empty', {read: TemplateRef})
empty: TemplateRef<any> = null !;
@ViewChild('basic', {read: TemplateRef})
basic: TemplateRef<any> = null !;
@ViewChild('projection', {read: TemplateRef})
projection: TemplateRef<any> = null !;
@ViewChild('nested', {read: TemplateRef})
nested: TemplateRef<any> = null !;
@ViewChild('icu', {read: TemplateRef})
icu: TemplateRef<any> = null !;
}

@Component({
selector: 'test-component',
template: `
<test-container-insertion>(content)</test-container-insertion>
`
})
class TestComponent {
}

const fixture = TestBed
.configureTestingModule(
{declarations: [TestComponent, TestContainerInsertionComponent]})
.createComponent(TestComponent);
fixture.detectChanges();
const testContainerComponent = getComponent<TestContainerInsertionComponent>(
(fixture.nativeElement as HTMLElement).querySelector('test-container-insertion') !) !;

testContainerComponent.container.createEmbeddedView(testContainerComponent.empty, {}, 0);
expect(fixture.nativeElement.textContent).toBe('');

testContainerComponent.container.createEmbeddedView(testContainerComponent.basic, {}, 0);
expect(fixture.nativeElement.textContent).toBe('(basic)');

const nestedView =
testContainerComponent.container.createEmbeddedView(testContainerComponent.nested, {}, 0);
nestedView.detectChanges(); // So that `*ngIf` unrolls
expect(fixture.nativeElement.textContent).toBe('(nested)(basic)');

(global as any).debug = true;
testContainerComponent.container.createEmbeddedView(testContainerComponent.projection, {}, 0);
expect(fixture.nativeElement.textContent).toBe('(content)(nested)(basic)');

const icuView =
testContainerComponent.container.createEmbeddedView(testContainerComponent.icu, {}, 0);
icuView.detectChanges(); // So that ICU updates
expect(fixture.nativeElement.textContent).toBe('(icu)(content)(nested)(basic)');
});

it('FW-1559: should support inserting in front of nested `LContainers', () => {
@Component({
selector: 'repeater',
Expand All @@ -303,7 +373,6 @@ describe('view insertion', () => {
fixture.detectChanges();

fixture.componentInstance.rendered = ['a', 'b'];
(global as any).break = true;
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toEqual('ab');
});
Expand Down

0 comments on commit 9465a38

Please sign in to comment.