From 994264c0ba3b65652d513f011287fa85ef6e9786 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Fri, 7 Jun 2019 20:46:11 -0700 Subject: [PATCH] refactor(ivy): simplify `walkTNodeTree` method for readability (#31065) PR Close #31065 --- integration/_payload-limits.json | 4 +- packages/core/src/render3/i18n.ts | 32 +- .../src/render3/instructions/lview_debug.ts | 83 ++-- .../src/render3/instructions/projection.ts | 6 +- .../core/src/render3/instructions/shared.ts | 1 - packages/core/src/render3/node_assert.ts | 1 + .../core/src/render3/node_manipulation.ts | 373 +++++++----------- packages/core/test/acceptance/i18n_spec.ts | 76 +++- .../cyclic_import/bundle.golden_symbols.json | 6 - .../hello_world/bundle.golden_symbols.json | 6 - .../bundling/todo/bundle.golden_symbols.json | 39 +- 11 files changed, 302 insertions(+), 325 deletions(-) diff --git a/integration/_payload-limits.json b/integration/_payload-limits.json index 75925ffbb3..ba86fd7470 100644 --- a/integration/_payload-limits.json +++ b/integration/_payload-limits.json @@ -12,7 +12,7 @@ "master": { "uncompressed": { "runtime": 1440, - "main": 13498, + "main": 13164, "polyfills": 45340 } } @@ -34,4 +34,4 @@ } } } -} +} \ No newline at end of file diff --git a/packages/core/src/render3/i18n.ts b/packages/core/src/render3/i18n.ts index 397986b14f..b97cf55650 100644 --- a/packages/core/src/render3/i18n.ts +++ b/packages/core/src/render3/i18n.ts @@ -24,11 +24,10 @@ import {RComment, RElement, RText} from './interfaces/renderer'; import {SanitizerFn} from './interfaces/sanitization'; import {isLContainer} from './interfaces/type_checks'; import {BINDING_INDEX, HEADER_OFFSET, LView, RENDERER, TVIEW, TView, T_HOST} from './interfaces/view'; -import {appendChild, appendProjectedNodes, createTextNode, nativeRemoveNode} from './node_manipulation'; +import {appendChild, applyProjection, createTextNode, nativeRemoveNode} from './node_manipulation'; import {getIsParent, getLView, getPreviousOrParentTNode, setIsNotParent, setPreviousOrParentTNode} from './state'; import {NO_CHANGE} from './tokens'; import {renderStringify} from './util/misc_utils'; -import {findComponentView} from './util/view_traversal_utils'; import {getNativeByIndex, getNativeByTNode, getTNode, load} from './util/view_utils'; @@ -490,7 +489,7 @@ function i18nStartFirstPass( } function appendI18nNode( - tNode: TNode, parentTNode: TNode, previousTNode: TNode | null, viewData: LView): TNode { + tNode: TNode, parentTNode: TNode, previousTNode: TNode | null, lView: LView): TNode { ngDevMode && ngDevMode.rendererMoveNode++; const nextNode = tNode.next; if (!previousTNode) { @@ -508,7 +507,7 @@ function appendI18nNode( tNode.next = null; } - if (parentTNode !== viewData[T_HOST]) { + if (parentTNode !== lView[T_HOST]) { tNode.parent = parentTNode as TElementNode; } @@ -523,18 +522,16 @@ function appendI18nNode( // If the placeholder to append is a projection, we need to move the projected nodes instead if (tNode.type === TNodeType.Projection) { - const tProjectionNode = tNode as TProjectionNode; - appendProjectedNodes( - viewData, tProjectionNode, tProjectionNode.projection, findComponentView(viewData)); + applyProjection(lView, tNode as TProjectionNode); return tNode; } - appendChild(getNativeByTNode(tNode, viewData), tNode, viewData); + appendChild(getNativeByTNode(tNode, lView), tNode, lView); - const slotValue = viewData[tNode.index]; + const slotValue = lView[tNode.index]; if (tNode.type !== TNodeType.Container && isLContainer(slotValue)) { // Nodes that inject ViewContainerRef also have a comment node that should be moved - appendChild(slotValue[NATIVE], tNode, viewData); + appendChild(slotValue[NATIVE], tNode, lView); } return tNode; } @@ -687,7 +684,7 @@ function i18nEndFirstPass(tView: TView) { // Remove deleted nodes for (let i = rootIndex + 1; i <= lastCreatedNode.index - HEADER_OFFSET; i++) { if (visitedNodes.indexOf(i) === -1) { - removeNode(i, viewData); + removeNode(i, viewData, /* markAsDetached */ true); } } } @@ -862,7 +859,10 @@ function readUpdateOpCodes( switch (removeOpCode & I18nMutateOpCode.MASK_OPCODE) { case I18nMutateOpCode.Remove: const nodeIndex = removeOpCode >>> I18nMutateOpCode.SHIFT_REF; - removeNode(nodeIndex, viewData); + // Remove DOM element, but do *not* mark TNode as detached, since we are + // just switching ICU cases (while keeping the same TNode), so a DOM element + // representing a new ICU case will be re-created. + removeNode(nodeIndex, viewData, /* markAsDetached */ false); break; case I18nMutateOpCode.RemoveNestedIcu: const nestedIcuNodeIndex = @@ -905,7 +905,7 @@ function readUpdateOpCodes( } } -function removeNode(index: number, viewData: LView) { +function removeNode(index: number, viewData: LView, markAsDetached: boolean) { const removedPhTNode = getTNode(index, viewData); const removedPhRNode = getNativeByIndex(index, viewData); if (removedPhRNode) { @@ -920,8 +920,10 @@ function removeNode(index: number, viewData: LView) { } } - // Define this node as detached so that we don't risk projecting it - removedPhTNode.flags |= TNodeFlags.isDetached; + if (markAsDetached) { + // Define this node as detached to avoid projecting it later + removedPhTNode.flags |= TNodeFlags.isDetached; + } ngDevMode && ngDevMode.rendererRemoveNode++; } diff --git a/packages/core/src/render3/instructions/lview_debug.ts b/packages/core/src/render3/instructions/lview_debug.ts index b554974306..47feb1c363 100644 --- a/packages/core/src/render3/instructions/lview_debug.ts +++ b/packages/core/src/render3/instructions/lview_debug.ts @@ -102,6 +102,12 @@ export const TViewConstructor = class TView implements ITView { public firstChild: TNode|null, // public schemas: SchemaMetadata[]|null, // ) {} + + get template_(): string { + const buf: string[] = []; + processTNodeChildren(this.firstChild, buf); + return buf.join(''); + } }; export const TNodeConstructor = class TNode implements ITNode { @@ -161,8 +167,34 @@ export const TNodeConstructor = class TNode implements ITNode { if (this.flags & TNodeFlags.isProjected) flags.push('TNodeFlags.isProjected'); return flags.join('|'); } + + get template_(): string { + const buf: string[] = []; + buf.push('<', this.tagName || this.type_); + if (this.attrs) { + for (let i = 0; i < this.attrs.length;) { + const attrName = this.attrs[i++]; + if (typeof attrName == 'number') { + break; + } + const attrValue = this.attrs[i++]; + buf.push(' ', attrName as string, '="', attrValue as string, '"'); + } + } + buf.push('>'); + processTNodeChildren(this.child, buf); + buf.push(''); + return buf.join(''); + } }; +function processTNodeChildren(tNode: TNode | null, buf: string[]) { + while (tNode) { + buf.push((tNode as any as{template_: string}).template_); + tNode = tNode.next; + } +} + const TViewData = ngDevMode && createNamedArrayType('TViewData'); let TVIEWDATA_EMPTY: unknown[]; // can't initialize here or it will not be tree shaken, because `LView` @@ -226,8 +258,8 @@ function toHtml(value: any, includeChildren: boolean = false): string|null { if (includeChildren || isTextNode) { return outerHTML; } else { - const innerHTML = node.innerHTML; - return outerHTML.split(innerHTML)[0] || null; + const innerHTML = '>' + node.innerHTML + '<'; + return (outerHTML.split(innerHTML)[0]) + '>'; } } else { return null; @@ -257,6 +289,7 @@ export class LViewDebug { } get parent(): LViewDebug|LContainerDebug|null { return toDebug(this._raw_lView[PARENT]); } get host(): string|null { return toHtml(this._raw_lView[HOST], true); } + get html(): string { return (this.nodes || []).map(node => toHtml(node.native, true)).join(''); } get context(): {}|null { return this._raw_lView[CONTEXT]; } /** * The tree of nodes associated with the current `LView`. The nodes have been normalized into a @@ -267,38 +300,30 @@ export class LViewDebug { const tNode = lView[TVIEW].firstChild; return toDebugNodes(tNode, lView); } - /** - * Additional information which is hidden behind a property. The extra level of indirection is - * done so that the debug view would not be cluttered with properties which are only rarely - * relevant to the developer. - */ - get __other__() { - return { - tView: this._raw_lView[TVIEW], - cleanup: this._raw_lView[CLEANUP], - injector: this._raw_lView[INJECTOR], - rendererFactory: this._raw_lView[RENDERER_FACTORY], - renderer: this._raw_lView[RENDERER], - sanitizer: this._raw_lView[SANITIZER], - childHead: toDebug(this._raw_lView[CHILD_HEAD]), - next: toDebug(this._raw_lView[NEXT]), - childTail: toDebug(this._raw_lView[CHILD_TAIL]), - declarationView: toDebug(this._raw_lView[DECLARATION_VIEW]), - queries: null, - tHost: this._raw_lView[T_HOST], - bindingIndex: this._raw_lView[BINDING_INDEX], - }; - } + + get tView() { return this._raw_lView[TVIEW]; } + get cleanup() { return this._raw_lView[CLEANUP]; } + get injector() { return this._raw_lView[INJECTOR]; } + get rendererFactory() { return this._raw_lView[RENDERER_FACTORY]; } + get renderer() { return this._raw_lView[RENDERER]; } + get sanitizer() { return this._raw_lView[SANITIZER]; } + get childHead() { return toDebug(this._raw_lView[CHILD_HEAD]); } + get next() { return toDebug(this._raw_lView[NEXT]); } + get childTail() { return toDebug(this._raw_lView[CHILD_TAIL]); } + get declarationView() { return toDebug(this._raw_lView[DECLARATION_VIEW]); } + get queries() { return this._raw_lView[QUERIES]; } + get tHost() { return this._raw_lView[T_HOST]; } + get bindingIndex() { return this._raw_lView[BINDING_INDEX]; } /** * Normalized view of child views (and containers) attached at this location. */ get childViews(): Array { const childViews: Array = []; - let child = this.__other__.childHead; + let child = this.childHead; while (child) { childViews.push(child); - child = child.__other__.next; + child = child.next; } return childViews; } @@ -359,11 +384,7 @@ export class LContainerDebug { get movedViews(): LView[]|null { return this._raw_lContainer[MOVED_VIEWS]; } get host(): RElement|RComment|LView { return this._raw_lContainer[HOST]; } get native(): RComment { return this._raw_lContainer[NATIVE]; } - get __other__() { - return { - next: toDebug(this._raw_lContainer[NEXT]), - }; - } + get next() { return toDebug(this._raw_lContainer[NEXT]); } } /** diff --git a/packages/core/src/render3/instructions/projection.ts b/packages/core/src/render3/instructions/projection.ts index 87bfb0628b..cbbd897602 100644 --- a/packages/core/src/render3/instructions/projection.ts +++ b/packages/core/src/render3/instructions/projection.ts @@ -8,13 +8,15 @@ import {TAttributes, TElementNode, TNode, TNodeType} from '../interfaces/node'; import {ProjectionSlots} from '../interfaces/projection'; import {TVIEW, T_HOST} from '../interfaces/view'; -import {appendProjectedNodes} from '../node_manipulation'; +import {applyProjection} from '../node_manipulation'; import {getProjectAsAttrValue, isNodeMatchingSelectorList, isSelectorInSelectorList} from '../node_selector_matcher'; import {getLView, setIsNotParent} from '../state'; import {findComponentView} from '../util/view_traversal_utils'; + import {getOrCreateTNode} from './shared'; + /** * Checks a given node against matching projection slots and returns the * determined slot index. Returns "null" if no slot matched the given node. @@ -134,6 +136,6 @@ export function ɵɵprojection( // We might need to delay the projection of nodes if they are in the middle of an i18n block if (!delayProjection) { // re-distribution of projectable nodes is stored on a component's view level - appendProjectedNodes(lView, tProjectionNode, selectorIndex, findComponentView(lView)); + applyProjection(lView, tProjectionNode); } } diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index 8157c7730c..5fa4f23819 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -235,7 +235,6 @@ function createTNodeAtIndex( if (index === 0 || !tView.firstChild) { tView.firstChild = tNode; } - // Now link ourselves into the tree. if (previousOrParentTNode) { if (isParent && previousOrParentTNode.child == null && (tNode.parent !== null || previousOrParentTNode.type === TNodeType.View)) { diff --git a/packages/core/src/render3/node_assert.ts b/packages/core/src/render3/node_assert.ts index dc3e6198f3..ce8ef229f4 100644 --- a/packages/core/src/render3/node_assert.ts +++ b/packages/core/src/render3/node_assert.ts @@ -25,6 +25,7 @@ export function assertNodeOfPossibleTypes(tNode: TNode, ...types: TNodeType[]) { function typeName(type: TNodeType): string { if (type == TNodeType.Projection) return 'Projection'; if (type == TNodeType.Container) return 'Container'; + if (type == TNodeType.IcuContainer) return 'IcuContainer'; if (type == TNodeType.View) return 'View'; if (type == TNodeType.Element) return 'Element'; if (type == TNodeType.ElementContainer) return 'ElementContainer'; diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index 05fe6294ef..e6cafe1127 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -8,13 +8,13 @@ import {ViewEncapsulation} from '../metadata/view'; import {addToArray, removeFromArray} from '../util/array_utils'; -import {assertDefined, assertDomNode} from '../util/assert'; -import {assertLContainer, assertLView} from './assert'; +import {assertDefined, assertDomNode, assertEqual} 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'; import {ComponentDef} from './interfaces/definition'; import {NodeInjectorFactory} from './interfaces/injector'; -import {TElementContainerNode, TElementNode, TIcuContainerNode, TNode, TNodeFlags, TNodeType, TProjectionNode, TViewNode, unusedValueExportToPlacateAjd as unused2} from './interfaces/node'; +import {TElementNode, TNode, TNodeFlags, TNodeType, TProjectionNode, TViewNode, unusedValueExportToPlacateAjd as unused2} from './interfaces/node'; import {unusedValueExportToPlacateAjd as unused3} from './interfaces/projection'; import {ProceduralRenderer3, RElement, RNode, RText, Renderer3, isProceduralRenderer, unusedValueExportToPlacateAjd as unused4} from './interfaces/renderer'; import {isLContainer, isLView, isRootView} from './interfaces/type_checks'; @@ -45,20 +45,26 @@ export function getLContainer(tNode: TViewNode, embeddedView: LView): LContainer * Retrieves render parent for a given view. * Might be null if a view is not yet attached to any container. */ -function getContainerRenderParent(tViewNode: TViewNode, view: LView): RElement|null { +export function getContainerRenderParent(tViewNode: TViewNode, view: LView): RElement|null { const container = getLContainer(tViewNode, view); return container ? nativeParentNode(view[RENDERER], container[NATIVE]) : null; } const enum WalkTNodeTreeAction { - /** node insert in the native environment */ - Insert = 0, + /** node create in the native environment. Run on initial creation. */ + Create = 0, + + /** + * node insert in the native environment. + * Run when existing node has been detached and needs to be re-attached. + */ + Insert = 1, /** node detach from the native environment */ - Detach = 1, + Detach = 2, /** node destruction using the renderer's API */ - Destroy = 2, + Destroy = 3, } @@ -67,7 +73,7 @@ const enum WalkTNodeTreeAction { * NOTE: for performance reasons, the possible actions are inlined within the function instead of * being passed as an argument. */ -function executeActionOnElementOrContainer( +function applyToElementOrContainer( action: WalkTNodeTreeAction, renderer: Renderer3, parent: RElement | null, lNodeToHandle: RNode | LContainer | LView, beforeNode?: RNode | null) { // If this slot was allocated for a text node dynamically created by i18n, the text node itself @@ -78,8 +84,7 @@ function executeActionOnElementOrContainer( let lContainer: LContainer|undefined; let isComponent = false; // We are expecting an RNode, but in the case of a component or LContainer the `RNode` is - // wrapped - // in an array which needs to be unwrapped. We need to know if it is a component and if + // wrapped in an array which needs to be unwrapped. We need to know if it is a component and if // it has LContainer so that we can process all of those cases appropriately. if (isLContainer(lNodeToHandle)) { lContainer = lNodeToHandle; @@ -91,8 +96,14 @@ function executeActionOnElementOrContainer( const rNode: RNode = unwrapRNode(lNodeToHandle); ngDevMode && assertDomNode(rNode); - if (action === WalkTNodeTreeAction.Insert) { - nativeInsertBefore(renderer, parent !, rNode, beforeNode || null); + if (action === WalkTNodeTreeAction.Create && parent !== null) { + if (beforeNode == null) { + nativeAppendChild(renderer, parent, rNode); + } else { + nativeInsertBefore(renderer, parent, rNode, beforeNode || null); + } + } else if (action === WalkTNodeTreeAction.Insert && parent !== null) { + nativeInsertBefore(renderer, parent, rNode, beforeNode || null); } else if (action === WalkTNodeTreeAction.Detach) { nativeRemoveNode(renderer, rNode, isComponent); } else if (action === WalkTNodeTreeAction.Destroy) { @@ -100,7 +111,7 @@ function executeActionOnElementOrContainer( (renderer as ProceduralRenderer3).destroyNode !(rNode); } if (lContainer != null) { - executeActionOnContainer(renderer, action, lContainer, parent, beforeNode); + applyContainer(renderer, action, lContainer, parent, beforeNode); } } } @@ -123,15 +134,15 @@ export function createTextNode(value: any, renderer: Renderer3): RText { */ export function addRemoveViewFromContainer( lView: LView, insertMode: true, beforeNode: RNode | null): void; -export function addRemoveViewFromContainer(lView: LView, insertMode: false): void; +export function addRemoveViewFromContainer(lView: LView, insertMode: false, beforeNode: null): void; export function addRemoveViewFromContainer( - lView: LView, insertMode: boolean, beforeNode?: RNode | null): void { + lView: LView, insertMode: boolean, beforeNode: RNode | null): void { const renderParent = getContainerRenderParent(lView[TVIEW].node as TViewNode, lView); ngDevMode && assertNodeType(lView[TVIEW].node as TNode, TNodeType.View); if (renderParent) { const renderer = lView[RENDERER]; const action = insertMode ? WalkTNodeTreeAction.Insert : WalkTNodeTreeAction.Detach; - executeActionOnView(renderer, action, lView, renderParent, beforeNode); + applyView(renderer, action, lView, renderParent, beforeNode); } } @@ -141,7 +152,7 @@ export function addRemoveViewFromContainer( * @param lView the `LView` to be detached. */ export function renderDetachView(lView: LView) { - executeActionOnView(lView[RENDERER], WalkTNodeTreeAction.Detach, lView, null, null); + applyView(lView[RENDERER], WalkTNodeTreeAction.Detach, lView, null, null); } /** @@ -290,7 +301,7 @@ export function detachView(lContainer: LContainer, removeIndex: number): LView|u lContainer[indexInContainer - 1][NEXT] = viewToDetach[NEXT] as LView; } const removedLView = removeFromArray(lContainer, CONTAINER_HEADER_OFFSET + removeIndex); - addRemoveViewFromContainer(viewToDetach, false); + addRemoveViewFromContainer(viewToDetach, false, null); // notify query that a view has been removed const lQueries = removedLView[QUERIES]; @@ -327,7 +338,7 @@ export function destroyLView(lView: LView) { if (!(lView[FLAGS] & LViewFlags.Destroyed)) { const renderer = lView[RENDERER]; if (isProceduralRenderer(renderer) && renderer.destroyNode) { - executeActionOnView(renderer, WalkTNodeTreeAction.Destroy, lView, null, null); + applyView(renderer, WalkTNodeTreeAction.Destroy, lView, null, null); } destroyViewTree(lView); @@ -480,12 +491,16 @@ function getRenderParent(tNode: TNode, currentView: LView): RElement|null { // Skip over element and ICU containers as those are represented by a comment node and // can't be used as a render parent. - const parent = getHighestElementOrICUContainer(tNode); - const renderParent = parent.parent; + let parentTNode = tNode.parent; + while (parentTNode != null && (parentTNode.type === TNodeType.ElementContainer || + parentTNode.type === TNodeType.IcuContainer)) { + tNode = parentTNode; + parentTNode = tNode.parent; + } - // If the parent is null, then we are inserting across views: either into an embedded view or a - // component view. - if (renderParent == null) { + // If the parent tNode is null, then we are inserting across views: either into an embedded view + // or a component view. + if (parentTNode == null) { const hostTNode = currentView[T_HOST] !; if (hostTNode.type === TNodeType.View) { // We are inserting a root element of an embedded view We might delay insertion of children @@ -502,17 +517,17 @@ function getRenderParent(tNode: TNode, currentView: LView): RElement|null { return getHostNative(currentView); } } else { - const isIcuCase = parent && parent.type === TNodeType.IcuContainer; + const isIcuCase = tNode && tNode.type === TNodeType.IcuContainer; // If the parent of this node is an ICU container, then it is represented by comment node and we - // need to use it as an anchor. If it is projected then its direct parent node is the renderer. - if (isIcuCase && parent.flags & TNodeFlags.isProjected) { - return getNativeByTNode(parent, currentView).parentNode as RElement; + // need to use it as an anchor. If it is projected then it's direct parent node is the renderer. + if (isIcuCase && tNode.flags & TNodeFlags.isProjected) { + return getNativeByTNode(tNode, currentView).parentNode as RElement; } - ngDevMode && assertNodeType(renderParent, TNodeType.Element); - if (renderParent.flags & TNodeFlags.isComponent && !isIcuCase) { + ngDevMode && assertNodeType(parentTNode, TNodeType.Element); + if (parentTNode.flags & TNodeFlags.isComponent) { const tData = currentView[TVIEW].data; - const tNode = tData[renderParent.index] as TNode; + const tNode = tData[parentTNode.index] as TNode; const encapsulation = (tData[tNode.directiveStart] as ComponentDef).encapsulation; // We've got a parent which is an element in the current view. We just need to verify if the @@ -527,7 +542,7 @@ function getRenderParent(tNode: TNode, currentView: LView): RElement|null { } } - return getNativeByTNode(renderParent, currentView) as RElement; + return getNativeByTNode(parentTNode, currentView) as RElement; } } @@ -560,6 +575,7 @@ export function nativeInsertBefore( function nativeAppendChild(renderer: Renderer3, parent: RElement, child: RNode): void { ngDevMode && ngDevMode.rendererAppendChild++; + ngDevMode && assertDefined(parent, 'parent node must be defined'); if (isProceduralRenderer(renderer)) { renderer.appendChild(parent, child); } else { @@ -608,7 +624,8 @@ export function nativeNextSibling(renderer: Renderer3, node: RNode): RNode|null */ function getNativeAnchorNode(parentTNode: TNode, lView: LView): RNode|null { if (parentTNode.type === TNodeType.View) { - const lContainer = getLContainer(parentTNode as TViewNode, lView) !; + const lContainer = getLContainer(parentTNode as TViewNode, lView); + if (lContainer === null) return null; const index = lContainer.indexOf(lView, CONTAINER_HEADER_OFFSET) - CONTAINER_HEADER_OFFSET; return getBeforeNodeForView(index, lContainer); } else if ( @@ -636,8 +653,8 @@ export function appendChild(childEl: RNode | RNode[], childTNode: TNode, current const parentTNode: TNode = childTNode.parent || currentView[T_HOST] !; const anchorNode = getNativeAnchorNode(parentTNode, currentView); if (Array.isArray(childEl)) { - for (let nativeNode of childEl) { - nativeAppendOrInsertBefore(renderer, renderParent, nativeNode, anchorNode); + for (let i = 0; i < childEl.length; i++) { + nativeAppendOrInsertBefore(renderer, renderParent, childEl[i], anchorNode); } } else { nativeAppendOrInsertBefore(renderer, renderParent, childEl, anchorNode); @@ -645,20 +662,6 @@ export function appendChild(childEl: RNode | RNode[], childTNode: TNode, current } } -/** - * Gets the top-level element or an ICU container if those containers are nested. - * - * @param tNode The starting TNode for which we should skip element and ICU containers - * @returns The TNode of the highest level ICU container or element container - */ -function getHighestElementOrICUContainer(tNode: TNode): TNode { - while (tNode.parent != null && (tNode.parent.type === TNodeType.ElementContainer || - tNode.parent.type === TNodeType.IcuContainer)) { - tNode = tNode.parent; - } - return tNode; -} - export function getBeforeNodeForView(viewIndexInContainer: number, lContainer: LContainer): RNode| null { const nextViewIndex = CONTAINER_HEADER_OFFSET + viewIndexInContainer + 1; @@ -689,111 +692,46 @@ export function nativeRemoveNode(renderer: Renderer3, rNode: RNode, isHostElemen } } -/** - * Appends nodes to a target projection place. Nodes to insert were previously re-distribution and - * stored on a component host level. - * @param lView A LView where nodes are inserted (target LView) - * @param tProjectionNode A projection node where previously re-distribution should be appended - * (target insertion place) - * @param selectorIndex A bucket from where nodes to project should be taken - * @param componentView A where projectable nodes were initially created (source view) - */ -export function appendProjectedNodes( - lView: LView, tProjectionNode: TProjectionNode, selectorIndex: number, - componentView: LView): void { - const projectedView = componentView[PARENT] !as LView; - const componentNode = componentView[T_HOST] as TElementNode; - let nodeToProject = (componentNode.projection as(TNode | null)[])[selectorIndex]; - if (Array.isArray(nodeToProject)) { - appendChild(nodeToProject, tProjectionNode, lView); - } else { - while (nodeToProject) { - if (!(nodeToProject.flags & TNodeFlags.isDetached)) { - if (nodeToProject.type === TNodeType.Projection) { - appendProjectedNodes( - lView, tProjectionNode, (nodeToProject as TProjectionNode).projection, - findComponentView(projectedView)); - } else { - // This flag must be set now or we won't know that this node is projected - // if the nodes are inserted into a container later. - nodeToProject.flags |= TNodeFlags.isProjected; - appendProjectedNode(nodeToProject, tProjectionNode, lView, projectedView); - } +/** + * Performs the operation of `action` on the node. Typically this involves inserting or removing + * nodes on the LView or projection boundary. + */ +function applyNodes( + renderer: Renderer3, action: WalkTNodeTreeAction, tNode: TNode | null, lView: LView, + renderParent: RElement | null, beforeNode: RNode | null, isProjection: boolean) { + while (tNode != null) { + ngDevMode && assertTNodeForLView(tNode, lView); + ngDevMode && assertNodeOfPossibleTypes( + tNode, TNodeType.Container, TNodeType.Element, TNodeType.ElementContainer, + TNodeType.Projection, TNodeType.Projection, TNodeType.IcuContainer); + const rawSlotValue = lView[tNode.index]; + const tNodeType = tNode.type; + if (isProjection) { + if (action === WalkTNodeTreeAction.Create) { + rawSlotValue && attachPatchData(unwrapRNode(rawSlotValue), lView); + tNode.flags |= TNodeFlags.isProjected; } - nodeToProject = nodeToProject.projectionNext; } - } -} - -/** - * Loops over all children of a TNode container and appends them to the DOM - * - * @param ngContainerChildTNode The first child of the TNode container - * @param tProjectionNode The projection (ng-content) TNode - * @param currentView Current LView - * @param projectionView Projection view (view above current) - */ -function appendProjectedChildren( - ngContainerChildTNode: TNode | null, tProjectionNode: TNode, currentView: LView, - projectionView: LView) { - while (ngContainerChildTNode) { - appendProjectedNode(ngContainerChildTNode, tProjectionNode, currentView, projectionView); - ngContainerChildTNode = ngContainerChildTNode.next; - } -} - -/** - * Appends a projected node to the DOM, or in the case of a projected container, - * appends the nodes from all of the container's active views to the DOM. - * - * @param projectedTNode The TNode to be projected - * @param tProjectionNode The projection (ng-content) TNode - * @param currentView Current LView - * @param projectionView Projection view (view above current) - */ -function appendProjectedNode( - projectedTNode: TNode, tProjectionNode: TNode, currentView: LView, - projectionView: LView): void { - const native = getNativeByTNode(projectedTNode, projectionView); - appendChild(native, tProjectionNode, currentView); - - // the projected contents are processed while in the shadow view (which is the currentView) - // therefore we need to extract the view where the host element lives since it's the - // logical container of the content projected views - attachPatchData(native, projectionView); - - const nodeOrContainer = projectionView[projectedTNode.index]; - if (projectedTNode.type === TNodeType.Container) { - // The node we are adding is a container and we are adding it to an element which - // is not a component (no more re-projection). - // Alternatively a container is projected at the root of a component's template - // and can't be re-projected (as not content of any component). - // Assign the final projection location in those cases. - for (let i = CONTAINER_HEADER_OFFSET; i < nodeOrContainer.length; i++) { - addRemoveViewFromContainer(nodeOrContainer[i], true, nodeOrContainer[NATIVE]); - } - } else if (projectedTNode.type === TNodeType.IcuContainer) { - // The node we are adding is an ICU container which is why we also need to project all the - // children nodes that might have been created previously and are linked to this anchor - let ngContainerChildTNode: TNode|null = projectedTNode.child as TNode; - appendProjectedChildren( - ngContainerChildTNode, ngContainerChildTNode, projectionView, projectionView); - } else { - if (projectedTNode.type === TNodeType.ElementContainer) { - appendProjectedChildren(projectedTNode.child, tProjectionNode, currentView, projectionView); - } - - if (isLContainer(nodeOrContainer)) { - appendChild(nodeOrContainer[NATIVE], tProjectionNode, currentView); + if ((tNode.flags & TNodeFlags.isDetached) !== TNodeFlags.isDetached) { + if (tNodeType === TNodeType.ElementContainer || tNodeType === TNodeType.IcuContainer) { + applyNodes(renderer, action, tNode.child, lView, renderParent, beforeNode, false); + applyToElementOrContainer(action, renderer, renderParent, rawSlotValue, beforeNode); + } else if (tNodeType === TNodeType.Projection) { + applyProjectionRecursive( + renderer, action, lView, tNode as TProjectionNode, renderParent, beforeNode); + } else { + ngDevMode && assertNodeOfPossibleTypes(tNode, TNodeType.Element, TNodeType.Container); + applyToElementOrContainer(action, renderer, renderParent, rawSlotValue, beforeNode); + } } + tNode = isProjection ? tNode.projectionNext : tNode.next; } } /** - * `executeActionOnView` performs an operation on the view as specified in `action` (insert, detach, - * destroy) + * `applyView` performs operation on the view as specified in `action` (insert, detach, destroy) * * Inserting a view without projection or containers at top level is simple. Just iterate over the * root nodes of the View, and for each node perform the `action`. @@ -805,10 +743,8 @@ function appendProjectedNode( * complication is that the nodes we are projecting can themselves have Containers * or other Projections. * - * As you can see this is a very recursive problem. While the recursive implementation is not the - * most efficient one, trying to unroll the nodes non-recursively results in very complex code that - * is very hard (to maintain). We are sacrificing a bit of performance for readability using a - * recursive implementation. + * As you can see this is a very recursive problem. Yes recursion is not most efficient but the + * code is complicated enough that trying to implemented with recursion becomes unmaintainable. * * @param renderer Renderer to use * @param action action to perform (insert, detach, destroy) @@ -816,64 +752,76 @@ function appendProjectedNode( * @param renderParent parent DOM element for insertion/removal. * @param beforeNode Before which node the insertions should happen. */ -function executeActionOnView( +function applyView( renderer: Renderer3, action: WalkTNodeTreeAction, lView: LView, renderParent: RElement | null, - beforeNode: RNode | null | undefined) { + beforeNode: RNode | null) { const tView = lView[TVIEW]; ngDevMode && assertNodeType(tView.node !, TNodeType.View); - let viewRootTNode: TNode|null = tView.node !.child; - while (viewRootTNode !== null) { - executeActionOnNode(renderer, action, lView, viewRootTNode, renderParent, beforeNode); - viewRootTNode = viewRootTNode.next; - } + const viewRootTNode: TNode|null = tView.node !.child; + applyNodes(renderer, action, viewRootTNode, lView, renderParent, beforeNode, false); } /** - * `executeActionOnProjection` performs an operation on the projection specified by `action` - * (insert, detach, destroy). + * `applyProjection` performs operation on the projection. * * Inserting a projection requires us to locate the projected nodes from the parent component. The * complication is that those nodes themselves could be re-projected from their parent component. * - * @param renderer Renderer to use + * @param lView The LView which needs to be inserted, detached, destroyed. + * @param tProjectionNode node to project + */ +export function applyProjection(lView: LView, tProjectionNode: TProjectionNode) { + const renderer = lView[RENDERER]; + const renderParent = getRenderParent(tProjectionNode, lView); + const parentTNode = tProjectionNode.parent || lView[T_HOST] !; + let beforeNode = getNativeAnchorNode(parentTNode, lView); + applyProjectionRecursive( + renderer, WalkTNodeTreeAction.Create, lView, tProjectionNode, renderParent, beforeNode); +} + +/** + * `applyProjectionRecursive` performs operation on the projection specified by `action` (insert, + * detach, destroy) + * + * Inserting a projection requires us to locate the projected nodes from the parent component. The + * complication is that those nodes themselves could be re-projected from their parent component. + * + * @param renderer Render to use * @param action action to perform (insert, detach, destroy) * @param lView The LView which needs to be inserted, detached, destroyed. - * @param tProjectionNode projection TNode to process + * @param tProjectionNode node to project * @param renderParent parent DOM element for insertion/removal. * @param beforeNode Before which node the insertions should happen. */ -function executeActionOnProjection( +function applyProjectionRecursive( renderer: Renderer3, action: WalkTNodeTreeAction, lView: LView, - tProjectionNode: TProjectionNode, renderParent: RElement | null, - beforeNode: RNode | null | undefined) { + tProjectionNode: TProjectionNode, renderParent: RElement | null, beforeNode: RNode | null) { const componentLView = findComponentView(lView); const componentNode = componentLView[T_HOST] as TElementNode; - ngDevMode && assertDefined( - componentNode.projection, - 'Element nodes for which projection is processed must have projection defined.'); - const nodeToProject = componentNode.projection ![tProjectionNode.projection]; - if (nodeToProject !== undefined) { - if (Array.isArray(nodeToProject)) { - for (let i = 0; i < nodeToProject.length; i++) { - const rNode = nodeToProject[i]; - ngDevMode && assertDomNode(rNode); - executeActionOnElementOrContainer(action, renderer, renderParent, rNode, beforeNode); - } - } else { - let projectionTNode: TNode|null = nodeToProject; - const projectedComponentLView = componentLView[PARENT] as LView; - while (projectionTNode !== null) { - executeActionOnNode( - renderer, action, projectedComponentLView, projectionTNode, renderParent, beforeNode); - projectionTNode = projectionTNode.projectionNext; - } + ngDevMode && + assertEqual(typeof tProjectionNode.projection, 'number', 'expecting projection index'); + const nodeToProjectOrRNodes = componentNode.projection ![tProjectionNode.projection] !; + if (Array.isArray(nodeToProjectOrRNodes)) { + // This should not exist, it is a bit of a hack. When we bootstrap a top level node and we + // need to support passing projectable nodes, so we cheat and put them in the TNode + // of the Host TView. (Yes we put instance info at the T Level). We can get away with it + // because we know that that TView is not shared and therefore it will not be a problem. + // This should be refactored and cleaned up. + for (let i = 0; i < nodeToProjectOrRNodes.length; i++) { + const rNode = nodeToProjectOrRNodes[i]; + applyToElementOrContainer(action, renderer, renderParent, rNode, beforeNode); } + } else { + let nodeToProject: TNode|null = nodeToProjectOrRNodes; + const projectedComponentLView = componentLView[PARENT] as LView; + applyNodes( + renderer, action, nodeToProject, projectedComponentLView, renderParent, beforeNode, true); } } /** - * `executeActionOnContainer` performs an operation on the container and its views as specified by + * `applyContainer` performs an operation on the container and its views as specified by * `action` (insert, detach, destroy) * * Inserting a Container is complicated by the fact that the container may have Views which @@ -885,7 +833,7 @@ function executeActionOnProjection( * @param renderParent parent DOM element for insertion/removal. * @param beforeNode Before which node the insertions should happen. */ -function executeActionOnContainer( +function applyContainer( renderer: Renderer3, action: WalkTNodeTreeAction, lContainer: LContainer, renderParent: RElement | null, beforeNode: RNode | null | undefined) { ngDevMode && assertLContainer(lContainer); @@ -895,58 +843,17 @@ function executeActionOnContainer( // Asking for a ViewContainerRef on an element will result in a creation of a separate anchor node // (comment in the DOM) that will be different from the LContainer's host node. In this particular // case we need to execute action on 2 nodes: - // - container's host node (this is done in the executeNodeAction) + // - container's host node (this is done in the executeActionOnElementOrContainer) // - container's host node (this is done here) if (anchor !== native) { - executeActionOnElementOrContainer(action, renderer, renderParent, anchor, beforeNode); + // This is very strange to me (Misko). I would expect that the native is same as anchor. I don't + // see a reason why they should be different, but they are. + // + // If they are we need to process the second anchor as well. + applyToElementOrContainer(action, renderer, renderParent, anchor, beforeNode); } for (let i = CONTAINER_HEADER_OFFSET; i < lContainer.length; i++) { const lView = lContainer[i] as LView; - executeActionOnView(renderer, action, lView, renderParent, anchor); - } -} - - -/** - * `executeActionOnElementContainerOrIcuContainer` performs an operation on the ng-container node - * and its child nodes as specified by the `action` (insert, detach, destroy). - * - * @param renderer Renderer to use - * @param action action to perform (insert, detach, destroy) - * @param lView The LView which needs to be inserted, detached, destroyed. - * @param tNode The TNode associated with the `ElementContainer` or `IcuContainer`. - * @param renderParent parent DOM element for insertion/removal. - * @param beforeNode Before which node the insertions should happen. - */ -function executeActionOnElementContainerOrIcuContainer( - renderer: Renderer3, action: WalkTNodeTreeAction, lView: LView, - tNode: TElementContainerNode | TIcuContainerNode, renderParent: RElement | null, - beforeNode: RNode | null | undefined) { - const node = lView[tNode.index]; - executeActionOnElementOrContainer(action, renderer, renderParent, node, beforeNode); - let childTNode: TNode|null = tNode.child; - while (childTNode) { - executeActionOnNode(renderer, action, lView, childTNode, renderParent, beforeNode); - childTNode = childTNode.next; - } -} - -function executeActionOnNode( - renderer: Renderer3, action: WalkTNodeTreeAction, lView: LView, tNode: TNode, - renderParent: RElement | null, beforeNode: RNode | null | undefined): void { - const nodeType = tNode.type; - if (!(tNode.flags & TNodeFlags.isDetached)) { - if (nodeType === TNodeType.ElementContainer || nodeType === TNodeType.IcuContainer) { - executeActionOnElementContainerOrIcuContainer( - renderer, action, lView, tNode as TElementContainerNode | TIcuContainerNode, renderParent, - beforeNode); - } else if (nodeType === TNodeType.Projection) { - executeActionOnProjection( - renderer, action, lView, tNode as TProjectionNode, renderParent, beforeNode); - } else { - ngDevMode && assertNodeOfPossibleTypes(tNode, TNodeType.Element, TNodeType.Container); - executeActionOnElementOrContainer( - action, renderer, renderParent, lView[tNode.index], beforeNode); - } + applyView(renderer, action, lView, renderParent, anchor); } } diff --git a/packages/core/test/acceptance/i18n_spec.ts b/packages/core/test/acceptance/i18n_spec.ts index c77b6a8dcb..e7044a2a94 100644 --- a/packages/core/test/acceptance/i18n_spec.ts +++ b/packages/core/test/acceptance/i18n_spec.ts @@ -770,7 +770,7 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { @Component({ selector: 'my-app', template: ` - { + { count, plural, =1 {ONE} @@ -780,6 +780,7 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { }) class App { count = 1; + condition = true; } TestBed.configureTestingModule({ @@ -788,25 +789,40 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { const fixture = TestBed.createComponent(App); fixture.detectChanges(); expect(fixture.debugElement.nativeElement.innerHTML) - .toBe('
ONE
'); + .toContain('
ONE
'); fixture.componentRef.instance.count = 2; fixture.detectChanges(); expect(fixture.debugElement.nativeElement.innerHTML) - .toBe('
OTHER
'); + .toContain('
OTHER
'); + + // destroy component + fixture.componentInstance.condition = false; + fixture.detectChanges(); + expect(fixture.debugElement.nativeElement.textContent).toBe(''); + + // render it again and also change ICU case + fixture.componentInstance.condition = true; + fixture.componentInstance.count = 1; + fixture.detectChanges(); + expect(fixture.debugElement.nativeElement.innerHTML) + .toContain('
ONE
'); }); it('with nested ICU expression and inside a container when creating a view via vcr.createEmbeddedView', () => { + let dir: Dir|null = null; @Directive({ selector: '[someDir]', }) class Dir { constructor( private readonly viewContainerRef: ViewContainerRef, - private readonly templateRef: TemplateRef) {} + private readonly templateRef: TemplateRef) { + dir = this; + } - ngOnInit() { this.viewContainerRef.createEmbeddedView(this.templateRef); } + attachEmbeddedView() { this.viewContainerRef.createEmbeddedView(this.templateRef); } } @Component({ @@ -845,6 +861,11 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { const fixture = TestBed.createComponent(App); fixture.componentRef.instance.count = 2; fixture.detectChanges(); + expect(fixture.debugElement.nativeElement.innerHTML) + .toBe(''); + + dir !.attachEmbeddedView(); + fixture.detectChanges(); expect(fixture.debugElement.nativeElement.innerHTML) .toBe( '
2 animals!
'); @@ -1520,6 +1541,51 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { fixture.detectChanges(); expect(fixture.nativeElement.innerHTML).toEqual(`
Contenu enfant
`); }); + + it('should display/destroy projected i18n content', () => { + + @Component({ + selector: 'app', + template: ` + () + ` + }) + class MyContentApp { + } + + @Component({ + selector: 'my-app', + template: ` + {type, select, A {A} B {B} other {other}} + ` + }) + class MyApp { + type = 'A'; + condition = true; + } + + TestBed.configureTestingModule({declarations: [MyApp, MyContentApp]}); + + const fixture = TestBed.createComponent(MyApp); + fixture.detectChanges(); + + expect(fixture.nativeElement.textContent).toContain('(A)'); + + // change `condition` to remove + fixture.componentInstance.condition = false; + fixture.detectChanges(); + + // should not contain 'A' + expect(fixture.nativeElement.textContent).toBe(''); + + // display again + fixture.componentInstance.type = 'B'; + fixture.componentInstance.condition = true; + fixture.detectChanges(); + + // expect that 'B' is now displayed + expect(fixture.nativeElement.textContent).toContain('(B)'); + }); }); describe('queries', () => { diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json index 83513bac59..3c0e5f02f1 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -149,9 +149,6 @@ { "name": "__self" }, - { - "name": "__values" - }, { "name": "__window" }, @@ -317,9 +314,6 @@ { "name": "getElementDepthCount" }, - { - "name": "getHighestElementOrICUContainer" - }, { "name": "getHostNative" }, 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 8846c561f3..1490c0f856 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -131,9 +131,6 @@ { "name": "__self" }, - { - "name": "__values" - }, { "name": "__window" }, @@ -254,9 +251,6 @@ { "name": "getDirectiveDef" }, - { - "name": "getHighestElementOrICUContainer" - }, { "name": "getHostNative" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index b2ff8da0ed..5cdbbbe518 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -317,9 +317,6 @@ { "name": "__spread" }, - { - "name": "__values" - }, { "name": "__window" }, @@ -464,9 +461,24 @@ { "name": "appendChild" }, + { + "name": "applyContainer" + }, + { + "name": "applyNodes" + }, + { + "name": "applyProjectionRecursive" + }, { "name": "applyStyling" }, + { + "name": "applyToElementOrContainer" + }, + { + "name": "applyView" + }, { "name": "assertTemplate" }, @@ -620,24 +632,6 @@ { "name": "enterView" }, - { - "name": "executeActionOnContainer" - }, - { - "name": "executeActionOnElementContainerOrIcuContainer" - }, - { - "name": "executeActionOnElementOrContainer" - }, - { - "name": "executeActionOnNode" - }, - { - "name": "executeActionOnProjection" - }, - { - "name": "executeActionOnView" - }, { "name": "executeContentQueries" }, @@ -773,9 +767,6 @@ { "name": "getGuardMask" }, - { - "name": "getHighestElementOrICUContainer" - }, { "name": "getHostNative" },