From 1278cca883b0c430204d5843abcad6cfb6b6f0e7 Mon Sep 17 00:00:00 2001 From: David-Emmanuel Divernois Date: Thu, 25 Jan 2018 15:32:21 +0100 Subject: [PATCH] perf(ivy): removes generation of comments (#21638) PR Close #21638 --- integration/_payload-limits.json | 4 +- packages/core/src/render3/instructions.ts | 85 ++++++-- .../core/src/render3/interfaces/container.ts | 10 + packages/core/src/render3/interfaces/node.ts | 22 +- .../core/src/render3/interfaces/projection.ts | 13 +- .../core/src/render3/interfaces/renderer.ts | 4 - .../core/src/render3/node_manipulation.ts | 195 ++++++++++++------ packages/core/test/render3/component_spec.ts | 68 +++++- packages/core/test/render3/content_spec.ts | 55 +++++ .../core/test/render3/integration_spec.ts | 138 ++++++++++++- packages/core/test/render3/query_spec.ts | 5 +- 11 files changed, 481 insertions(+), 118 deletions(-) diff --git a/integration/_payload-limits.json b/integration/_payload-limits.json index 6b879a1a8b..21bb8222e4 100644 --- a/integration/_payload-limits.json +++ b/integration/_payload-limits.json @@ -18,14 +18,14 @@ "hello_world__render3__closure": { "master": { "uncompressed": { - "bundle": 7674 + "bundle": 8153 } } }, "hello_world__render3__rollup": { "master": { "uncompressed": { - "bundle": 58662 + "bundle": 59334 } } } diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 92b1b769f7..69e28a56ac 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -19,7 +19,7 @@ import {assertNodeType, assertNodeOfPossibleTypes} from './node_assert'; import {appendChild, insertChild, insertView, processProjectedNode, removeView} from './node_manipulation'; import {isNodeMatchingSelector} from './node_selector_matcher'; import {ComponentDef, ComponentTemplate, ComponentType, DirectiveDef, DirectiveType} from './interfaces/definition'; -import {RComment, RElement, RText, Renderer3, RendererFactory3, ProceduralRenderer3, ObjectOrientedRenderer3, RendererStyleFlags3} from './interfaces/renderer'; +import {RElement, RText, Renderer3, RendererFactory3, ProceduralRenderer3, ObjectOrientedRenderer3, RendererStyleFlags3} from './interfaces/renderer'; import {isDifferent, stringify} from './util'; import {executeViewHooks, executeContentHooks, queueLifecycleHooks, queueInitHooks, executeInitHooks} from './hooks'; @@ -194,15 +194,14 @@ export function createLNode( export function createLNode( index: null, type: LNodeFlags.View, native: null, lView: LView): LViewNode; export function createLNode( - index: number, type: LNodeFlags.Container, native: RComment, + index: number, type: LNodeFlags.Container, native: null, lContainer: LContainer): LContainerNode; export function createLNode( index: number, type: LNodeFlags.Projection, native: null, lProjection: LProjection): LProjectionNode; export function createLNode( - index: number | null, type: LNodeFlags, native: RText | RElement | RComment | null, - state?: null | LView | LContainer | LProjection): LElementNode<extNode&LViewNode& - LContainerNode&LProjectionNode { + index: number | null, type: LNodeFlags, native: RText | RElement | null, state?: null | LView | + LContainer | LProjection): LElementNode<extNode&LViewNode&LContainerNode&LProjectionNode { const parent = isParent ? previousOrParentNode : previousOrParentNode && previousOrParentNode.parent as LNode; let query = (isParent ? currentQuery : previousOrParentNode && previousOrParentNode.query) || @@ -218,7 +217,8 @@ export function createLNode( nodeInjector: parent ? parent.nodeInjector : null, data: isState ? state as any : null, query: query, - tNode: null + tNode: null, + pNextOrParent: null }; if ((type & LNodeFlags.ViewOrElement) === LNodeFlags.ViewOrElement && isState) { @@ -989,11 +989,14 @@ export function container( // If the direct parent of the container is a view, its views (including its comment) // will need to be added through insertView() when its parent view is being inserted. // For now, it is marked "headless" so we know to append its views later. - let comment = renderer.createComment(ngDevMode ? 'container' : ''); let renderParent: LElementNode|null = null; const currentParent = isParent ? previousOrParentNode : previousOrParentNode.parent !; ngDevMode && assertNotEqual(currentParent, null, 'currentParent'); - if (appendChild(currentParent, comment, currentView)) { + + if ((currentParent.flags & LNodeFlags.TYPE_MASK) === LNodeFlags.Element && + (currentParent.view !== + currentView /* Crossing View Boundaries, it is Component, but add Element of View */ + || currentParent.data === null /* Regular Element. */)) { // we are adding to an Element which is either: // - Not a component (will not be re-projected, just added) // - View of the Component @@ -1007,10 +1010,11 @@ export function container( next: null, parent: currentView, dynamicViewCount: 0, - query: null + query: null, + nextNative: undefined }; - const node = createLNode(index, LNodeFlags.Container, comment, lContainer); + const node = createLNode(index, LNodeFlags.Container, null, lContainer); if (node.tNode == null) { // TODO(misko): implement queryName caching @@ -1044,7 +1048,11 @@ export function containerRefreshStart(index: number): void { previousOrParentNode = data[index] as LNode; ngDevMode && assertNodeType(previousOrParentNode, LNodeFlags.Container); isParent = true; - (previousOrParentNode as LContainerNode).data.nextIndex = 0; + const container = previousOrParentNode as LContainerNode; + container.data.nextIndex = 0; + ngDevMode && + assertEqual( + container.data.nextNative === undefined, true, 'container.data.nextNative === undefined'); // We need to execute init hooks here so ngOnInit hooks are called in top level views // before they are called in embedded views (for backwards compatibility). @@ -1066,6 +1074,7 @@ export function containerRefreshEnd(): void { } ngDevMode && assertNodeType(previousOrParentNode, LNodeFlags.Container); const container = previousOrParentNode as LContainerNode; + container.data.nextNative = undefined; ngDevMode && assertNodeType(container, LNodeFlags.Container); const nextIndex = container.data.nextIndex; while (nextIndex < container.data.views.length) { @@ -1253,6 +1262,34 @@ export function projectionDef(index: number, selectors?: CssSelector[]): void { data[index] = distributedNodes; } +/** + * Updates the linked list of a projection node, by appending another linked list. + * + * @param projectionNode Projection node whose projected nodes linked list has to be updated + * @param appendedFirst First node of the linked list to append. + * @param appendedLast Last node of the linked list to append. + */ +function appendToProjectionNode( + projectionNode: LProjectionNode, + appendedFirst: LElementNode | LTextNode | LContainerNode | null, + appendedLast: LElementNode | LTextNode | LContainerNode | null) { + // appendedFirst can be null if and only if appendedLast is also null + ngDevMode && + assertEqual(!appendedFirst === !appendedLast, true, '!appendedFirst === !appendedLast'); + if (!appendedLast) { + // nothing to append + return; + } + const projectionNodeData = projectionNode.data; + if (projectionNodeData.last) { + projectionNodeData.last.pNextOrParent = appendedFirst; + } else { + projectionNodeData.first = appendedFirst; + } + projectionNodeData.last = appendedLast; + appendedLast.pNextOrParent = projectionNode; +} + /** * Inserts previously re-distributed projected nodes. This instruction must be preceded by a call * to the projectionDef instruction. @@ -1262,8 +1299,7 @@ export function projectionDef(index: number, selectors?: CssSelector[]): void { * @param selectorIndex - 0 means without any selector */ export function projection(nodeIndex: number, localIndex: number, selectorIndex: number = 0): void { - const projectedNodes: LProjection = []; - const node = createLNode(nodeIndex, LNodeFlags.Projection, null, projectedNodes); + const node = createLNode(nodeIndex, LNodeFlags.Projection, null, {first: null, last: null}); isParent = false; // self closing const currentParent = node.parent; @@ -1274,20 +1310,27 @@ export function projection(nodeIndex: number, localIndex: number, selectorIndex: const nodesForSelector = valueInData(componentNode.data !.data !, localIndex)[selectorIndex]; + // build the linked list of projected nodes: for (let i = 0; i < nodesForSelector.length; i++) { const nodeToProject = nodesForSelector[i]; if ((nodeToProject.flags & LNodeFlags.TYPE_MASK) === LNodeFlags.Projection) { - const previouslyProjectedNodes = (nodeToProject as LProjectionNode).data; - for (let j = 0; j < previouslyProjectedNodes.length; j++) { - processProjectedNode( - projectedNodes, previouslyProjectedNodes[j], currentParent, currentView); - } + const previouslyProjected = (nodeToProject as LProjectionNode).data; + appendToProjectionNode(node, previouslyProjected.first, previouslyProjected.last); } else { - processProjectedNode( - projectedNodes, nodeToProject as LElementNode | LTextNode | LContainerNode, currentParent, - currentView); + appendToProjectionNode( + node, nodeToProject as LTextNode | LElementNode | LContainerNode, + nodeToProject as LTextNode | LElementNode | LContainerNode); } } + + // process each node in the list of projected nodes: + let nodeToProject: LNode|null = node.data.first; + const lastNodeToProject = node.data.last; + while (nodeToProject) { + processProjectedNode( + nodeToProject as LTextNode | LElementNode | LContainerNode, currentParent, currentView); + nodeToProject = nodeToProject === lastNodeToProject ? null : nodeToProject.pNextOrParent; + } } /** diff --git a/packages/core/src/render3/interfaces/container.ts b/packages/core/src/render3/interfaces/container.ts index 1a5edd8398..e10722e6c3 100644 --- a/packages/core/src/render3/interfaces/container.ts +++ b/packages/core/src/render3/interfaces/container.ts @@ -9,6 +9,7 @@ import {ComponentTemplate} from './definition'; import {LElementNode, LViewNode} from './node'; import {LQuery} from './query'; +import {RNode} from './renderer'; import {LView, TView} from './view'; @@ -80,6 +81,15 @@ export interface LContainer { * this container are reported to queries referenced here. */ query: LQuery|null; + + /* + * Caches the reference of the first native node following this container in the same native + * parent. + * This is reset to undefined in containerRefreshEnd. + * When it is undefined, it means the value has not been computed yet. + * Otherwise, it contains the result of findBeforeNode(container, null). + */ + nextNative: RNode|null|undefined; } /** diff --git a/packages/core/src/render3/interfaces/node.ts b/packages/core/src/render3/interfaces/node.ts index 8f188738f0..a0a40906c3 100644 --- a/packages/core/src/render3/interfaces/node.ts +++ b/packages/core/src/render3/interfaces/node.ts @@ -11,7 +11,7 @@ import {DirectiveDef} from './definition'; import {LInjector} from './injector'; import {LProjection} from './projection'; import {LQuery} from './query'; -import {RComment, RElement, RText} from './renderer'; +import {RElement, RText} from './renderer'; import {LView, TData, TView} from './view'; @@ -74,9 +74,8 @@ export interface LNode { * The associated DOM node. Storing this allows us to: * - append children to their element parents in the DOM (e.g. `parent.native.appendChild(...)`) * - retrieve the sibling elements of text nodes whose creation / insertion has been delayed - * - mark locations where child views should be inserted (for containers) */ - readonly native: RElement|RText|RComment|null; + readonly native: RElement|RText|null; /** * We need a reference to a node's parent so we can append the node to its parent's native @@ -122,6 +121,14 @@ export interface LNode { */ query: LQuery|null; + /** + * If this node is projected, pointer to the next node in the same projection parent + * (which is a container, an element, or a text node), or to the parent projection node + * if this is the last node in the projection. + * If this node is not projected, this field is null. + */ + pNextOrParent: LNode|null; + /** * Pointer to the corresponding TNode object, which stores static * data about this node. @@ -170,14 +177,7 @@ export interface LViewNode extends LNode { /** Abstract node container which contains other views. */ export interface LContainerNode extends LNode { - /** - * This comment node is appended to the container's parent element to mark where - * in the DOM the container's child views should be added. - * - * If the container is a root node of a view, this comment will not be appended - * until the parent view is processed. - */ - readonly native: RComment; + readonly native: null; readonly data: LContainer; child: null; next: LContainerNode|LElementNode|LTextNode|LProjectionNode|null; diff --git a/packages/core/src/render3/interfaces/projection.ts b/packages/core/src/render3/interfaces/projection.ts index f438d0f9cf..a34e45b601 100644 --- a/packages/core/src/render3/interfaces/projection.ts +++ b/packages/core/src/render3/interfaces/projection.ts @@ -9,14 +9,13 @@ import {LContainerNode, LElementNode, LTextNode} from './node'; /** - * An LProjection is just an array of projected nodes. - * - * It would be nice if we could not need an array, but since a projected node can be - * re-projected, the same node can be part of more than one LProjectionNode which makes - * list approach not possible. + * An LProjection is a pointer to the first and the last projected nodes. + * It is a linked list (using the pNextOrParent property). */ -export type LProjection = Array; - +export interface LProjection { + first: LElementNode|LTextNode|LContainerNode|null; + last: LElementNode|LTextNode|LContainerNode|null; +} /** * Parsed selector in the following format: diff --git a/packages/core/src/render3/interfaces/renderer.ts b/packages/core/src/render3/interfaces/renderer.ts index 0f88662fbe..cb7f249a1b 100644 --- a/packages/core/src/render3/interfaces/renderer.ts +++ b/packages/core/src/render3/interfaces/renderer.ts @@ -35,7 +35,6 @@ export type Renderer3 = ObjectOrientedRenderer3 | ProceduralRenderer3; * (reducing payload size). * */ export interface ObjectOrientedRenderer3 { - createComment(data: string): RComment; createElement(tagName: string): RElement; createTextNode(data: string): RText; @@ -52,7 +51,6 @@ export interface ObjectOrientedRenderer3 { export interface ProceduralRenderer3 { destroy(): void; createElement(name: string, namespace?: string|null): RElement; - createComment(value: string): RComment; createText(value: string): RText; /** * This property is allowed to be null / undefined, @@ -138,8 +136,6 @@ export interface RDomTokenList { export interface RText extends RNode { textContent: string|null; } -export interface RComment extends RNode {} - // Note: This hack is necessary so we don't erroneously get a circular dependency // failure based on types. export const unusedValueExportToPlacateAjd = 1; diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index d3a55d411f..20e30cf1c2 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -6,65 +6,135 @@ * found in the LICENSE file at https://angular.io/license */ -import {assertNotNull} from './assert'; +import {assertNotEqual, assertNotNull} from './assert'; import {LContainer, unusedValueExportToPlacateAjd as unused1} from './interfaces/container'; import {LContainerNode, LElementNode, LNode, LNodeFlags, LProjectionNode, LTextNode, LViewNode, unusedValueExportToPlacateAjd as unused2} from './interfaces/node'; -import {LProjection, unusedValueExportToPlacateAjd as unused3} from './interfaces/projection'; -import {ProceduralRenderer3, RComment, RElement, RNode, RText, unusedValueExportToPlacateAjd as unused4} from './interfaces/renderer'; +import {unusedValueExportToPlacateAjd as unused3} from './interfaces/projection'; +import {ProceduralRenderer3, RElement, RNode, RText, unusedValueExportToPlacateAjd as unused4} from './interfaces/renderer'; import {HookData, LView, LViewOrLContainer, TView, unusedValueExportToPlacateAjd as unused5} from './interfaces/view'; import {assertNodeType} from './node_assert'; const unusedValueToPlacateAjd = unused1 + unused2 + unused3 + unused4 + unused5; /** - * Finds the closest DOM node above a given container in the hierarchy. + * Returns the first DOM node following the given logical node in the same parent DOM element. * - * This is necessary to add or remove elements from the DOM when a view - * is added or removed from the container. e.g. parent.removeChild(...) + * This is needed in order to insert the given node with insertBefore. * - * @param containerNode The container node whose parent must be found - * @returns Closest DOM node above the container + * @param node The node whose following DOM node must be found. + * @param stopNode A parent node at which the lookup in the tree should be stopped, or null if the + * lookup should not be stopped until the result is found. + * @returns Node before which the provided node should be inserted or null if the lookup was stopped + * or if there is no native node after the given logical node in the same native parent. */ -export function findNativeParent(containerNode: LContainerNode): RNode|null { - let container: LContainerNode|null = containerNode; - while (container) { - ngDevMode && assertNodeType(container, LNodeFlags.Container); - const renderParent = container.data.renderParent; - if (renderParent !== null) { - return renderParent.native; +function findBeforeNode(node: LNode | null, stopNode: LNode | null): RNode|null { + let currentNode = node; + while (currentNode && currentNode !== stopNode) { + const currentNodeType = currentNode.flags && LNodeFlags.TYPE_MASK; + let pNextOrParent = currentNode.pNextOrParent; + if (pNextOrParent) { + let pNextOrParentType = pNextOrParent.flags & LNodeFlags.TYPE_MASK; + while (pNextOrParentType !== LNodeFlags.Projection) { + const nativeNode = findFirstNativeNode(pNextOrParent); + if (nativeNode) { + return nativeNode; + } + pNextOrParent = pNextOrParent.pNextOrParent !; + } + currentNode = pNextOrParent; + } else { + let currentSibling = currentNode.next; + while (currentSibling) { + const nativeNode = findFirstNativeNode(currentSibling); + if (nativeNode) { + return nativeNode; + } + currentSibling = currentSibling.next; + } + const parentNode = currentNode.parent; + currentNode = null; + if (parentNode) { + const parentType = parentNode.flags & LNodeFlags.TYPE_MASK; + if (parentType === LNodeFlags.Container || parentType === LNodeFlags.View) { + currentNode = parentNode; + } + } } - const viewOrElement: LViewNode|LElementNode = container.parent !; - ngDevMode && assertNotNull(viewOrElement, 'container.parent'); - if ((viewOrElement.flags & LNodeFlags.TYPE_MASK) === LNodeFlags.Element) { - // we are an LElement, which means we are past the last LContainer. - // This means than we have not been projected so just ignore. - return null; - } - ngDevMode && assertNodeType(viewOrElement, LNodeFlags.View); - container = (viewOrElement as LViewNode).parent; } return null; } /** - * Finds the DOM element before which a certain view should be inserting its - * child elements. - * - * If the view has a next (e.g. for loop), elements should be inserted before - * the next view's first child element. Otherwise, the container's comment - * anchor is the marker. - * - * @param index The index of the view to check - * @param lContainer parent LContainer - * @param native Comment anchor for container - * @returns The DOM element for which the view should insert elements + * Get the next node in the LNode tree, taking into account the place where a node is + * projected (in the shadow DOM) rather than where it comes from (in the light DOM). + * If the node is not projected, the return value is simply node.next. + * If the node is projected, the return value is node.pNextOrParent if node.pNextOrParent is + * not a projection node (which marks the end of the linked list). + * Otherwise the return value is null. + * @param node The node whose next node in the LNode tree must be found. + * @return The next sibling in the LNode tree. */ -function findBeforeNode(index: number, lContainer: LContainer, native: RNode): RNode { - const views = lContainer.views; - // Find the node to insert in front of - return index + 1 < views.length ? - (views[index + 1].child as LTextNode | LElementNode | LContainerNode).native : - native; +function getNextNode(node: LNode): LNode|null { + const pNextOrParent = node.pNextOrParent; + if (pNextOrParent) { + const type = pNextOrParent.flags & LNodeFlags.TYPE_MASK; + return type === LNodeFlags.Projection ? null : pNextOrParent; + } else { + return node.next; + } +} + +/** + * Find the next node in the LNode tree, taking into account the place where a node is + * projected (in the shadow DOM) rather than where it comes from (in the light DOM). + * If there is no sibling node, this function goes to the next sibling of the parent node... + * until it reaches rootNode (at which point null is returned). + * + * @param initialNode The node whose following node in the LNode tree must be found. + * @param rootNode The root node at which the lookup should stop. + * @return The following node in the LNode tree. + */ +function findFollowingNode(initialNode: LNode, rootNode: LNode): LNode|null { + let node: LNode|null = initialNode; + let nextNode = getNextNode(node); + while (node && !nextNode) { + // if node.pNextOrParent is not null here, it is not the next node + // (because, at this point, nextNode is null, so it is the parent) + node = node.pNextOrParent || node.parent; + if (node === rootNode) node = null; + nextNode = node && getNextNode(node); + } + return nextNode; +} + +/** + * Returns the first DOM node inside the given logical node. + * + * @param node The node whose first DOM node must be found + * @returns The first native node of the given logical node or null if there is none. + */ +function findFirstNativeNode(rootNode: LNode): RNode|null { + let node: LNode|null = rootNode; + while (node) { + const type = node.flags & LNodeFlags.TYPE_MASK; + let nextNode: LNode|null = null; + if (type === LNodeFlags.Element) { + return node.native; + } else if (type === LNodeFlags.Container) { + const childContainerData: LContainer = (node as LContainerNode).data; + nextNode = childContainerData.views.length ? childContainerData.views[0].child : null; + } else if (type === LNodeFlags.Projection) { + nextNode = (node as LProjectionNode).data.first; + } else { + nextNode = (node as LViewNode).child; + } + if (nextNode === null) { + node = findFollowingNode(node, rootNode); + } else { + node = nextNode; + } + } + return null; } /** @@ -89,7 +159,8 @@ export function addRemoveViewFromContainer( beforeNode?: RNode | null): void { ngDevMode && assertNodeType(container, LNodeFlags.Container); ngDevMode && assertNodeType(rootNode, LNodeFlags.View); - const parent = findNativeParent(container); + const parentNode = container.data.renderParent; + const parent = parentNode ? parentNode.native : null; let node: LNode|null = rootNode.child; if (parent) { while (node) { @@ -111,26 +182,15 @@ export function addRemoveViewFromContainer( // if we get to a container, it must be a root node of a view because we are only // propagating down into child views / containers and not child elements const childContainerData: LContainer = (node as LContainerNode).data; - insertMode ? (isFnRenderer ? - (renderer as ProceduralRenderer3) - .appendChild !(parent as RElement, node.native !) : - parent.appendChild(node.native !)) : - (isFnRenderer ? - (renderer as ProceduralRenderer3) - .removeChild !(parent as RElement, node.native !) : - parent.removeChild(node.native !)); + childContainerData.renderParent = parentNode; nextNode = childContainerData.views.length ? childContainerData.views[0].child : null; } else if (type === LNodeFlags.Projection) { - nextNode = (node as LProjectionNode).data[0]; + nextNode = (node as LProjectionNode).data.first; } else { nextNode = (node as LViewNode).child; } if (nextNode === null) { - while (node && !node.next) { - node = node.parent; - if (node === rootNode) node = null; - } - node = node && node.next; + node = findFollowingNode(node, rootNode); } else { node = nextNode; } @@ -218,8 +278,15 @@ export function insertView( // and we should wait until that parent processes its nodes (otherwise, we will insert this view's // nodes twice - once now and once when its parent inserts its views). if (container.data.renderParent !== null) { - addRemoveViewFromContainer( - container, newView, true, findBeforeNode(index, state, container.native)); + let beforeNode = findBeforeNode(newView, container); + if (!beforeNode) { + let containerNextNativeNode = container.data.nextNative; + if (containerNextNativeNode === undefined) { + containerNextNativeNode = container.data.nextNative = findBeforeNode(container, null); + } + beforeNode = containerNextNativeNode; + } + addRemoveViewFromContainer(container, newView, true, beforeNode); } return newView; @@ -389,11 +456,7 @@ export function insertChild(node: LNode, currentView: LView): void { || parent.data === null /* Regular Element. */)) { // We only add element if not in View or not projected. - let sibling = node.next; - let nativeSibling: RNode|null = null; - while (sibling && (nativeSibling = sibling.native) === null) { - sibling = sibling.next; - } + let nativeSibling: RNode|null = findBeforeNode(node, null); const renderer = currentView.renderer; (renderer as ProceduralRenderer3).listen ? (renderer as ProceduralRenderer3) @@ -407,14 +470,13 @@ export function insertChild(node: LNode, currentView: LView): void { * appends the nodes from all of the container's active views to the DOM. Also stores the * node in the given projectedNodes array. * - * @param projectedNodes Array to store the projected node * @param node The node to process * @param currentParent The last parent element to be processed * @param currentView Current LView */ export function processProjectedNode( - projectedNodes: LProjection, node: LElementNode | LTextNode | LContainerNode, - currentParent: LViewNode | LElementNode, currentView: LView): void { + node: LElementNode | LTextNode | LContainerNode, currentParent: LViewNode | LElementNode, + currentView: LView): void { if ((node.flags & LNodeFlags.TYPE_MASK) === LNodeFlags.Container && (currentParent.flags & LNodeFlags.TYPE_MASK) === LNodeFlags.Element && (currentParent.data === null || currentParent.data === currentView)) { @@ -430,6 +492,5 @@ export function processProjectedNode( addRemoveViewFromContainer(node as LContainerNode, views[i], true, null); } } - projectedNodes.push(node); appendChild(currentParent, node.native, currentView); } diff --git a/packages/core/test/render3/component_spec.ts b/packages/core/test/render3/component_spec.ts index fac4e51796..8b0ef56eb7 100644 --- a/packages/core/test/render3/component_spec.ts +++ b/packages/core/test/render3/component_spec.ts @@ -7,11 +7,11 @@ */ import {ViewEncapsulation} from '../../src/core'; -import {E, T, b, defineComponent, e, markDirty, r, t} from '../../src/render3/index'; +import {C, E, T, V, b, cR, cr, defineComponent, e, markDirty, p, r, t, v} from '../../src/render3/index'; import {createRendererType2} from '../../src/view/index'; import {getRendererFactory2} from './imported_renderer2'; -import {containerEl, renderComponent, requestAnimationFrame, toHtml} from './render_util'; +import {containerEl, renderComponent, renderToHtml, requestAnimationFrame, toHtml} from './render_util'; describe('component', () => { class CounterComponent { @@ -59,6 +59,70 @@ describe('component', () => { }); +describe('component with a container', () => { + + function showItems(ctx: {items: string[]}, cm: boolean) { + if (cm) { + C(0); + } + cR(0); + { + for (const item of ctx.items) { + const cm0 = V(0); + { + if (cm0) { + T(0); + } + t(0, b(item)); + } + v(); + } + } + cr(); + } + + class WrapperComponent { + items: string[]; + static ngComponentDef = defineComponent({ + type: WrapperComponent, + tag: 'wrapper', + template: function ChildComponentTemplate(ctx: {items: string[]}, cm: boolean) { + if (cm) { + C(0); + } + cR(0); + { + const cm0 = V(0); + { showItems({items: ctx.items}, cm0); } + v(); + } + cr(); + }, + factory: () => new WrapperComponent, + inputs: {items: 'items'} + }); + } + + function template(ctx: {items: string[]}, cm: boolean) { + if (cm) { + E(0, WrapperComponent); + e(); + } + p(0, 'items', b(ctx.items)); + WrapperComponent.ngComponentDef.h(1, 0); + r(1, 0); + } + + it('should re-render on input change', () => { + const ctx: {items: string[]} = {items: ['a']}; + expect(renderToHtml(template, ctx)).toEqual('a'); + + ctx.items = [...ctx.items, 'b']; + expect(renderToHtml(template, ctx)).toEqual('ab'); + }); + +}); + // TODO: add tests with Native once tests are run in real browser (domino doesn't support shadow // root) describe('encapsulation', () => { diff --git a/packages/core/test/render3/content_spec.ts b/packages/core/test/render3/content_spec.ts index 7fa6a7edbf..b94b08713d 100644 --- a/packages/core/test/render3/content_spec.ts +++ b/packages/core/test/render3/content_spec.ts @@ -377,6 +377,61 @@ describe('content projection', () => { expect(toHtml(parent)).toEqual('
'); }); + it('should support projection in embedded views when ng-content is a root node of an embedded view, with other nodes after', + () => { + let childCmptInstance: any; + + /** + *
+ * % if (!skipContent) { + * before--after + * % } + *
+ */ + const Child = createComponent('child', function(ctx: any, cm: boolean) { + if (cm) { + pD(0); + E(1, 'div'); + { C(2); } + e(); + } + cR(2); + { + if (!ctx.skipContent) { + if (V(0)) { + T(0, 'before-'); + P(1, 0); + T(2, '-after'); + } + v(); + } + } + cr(); + }); + + /** + * content + */ + const Parent = createComponent('parent', function(ctx: any, cm: boolean) { + if (cm) { + E(0, Child); + { + childCmptInstance = m(1); + T(2, 'content'); + } + e(); + } + Child.ngComponentDef.h(1, 0); + r(1, 0); + }); + const parent = renderComponent(Parent); + expect(toHtml(parent)).toEqual('
before-content-after
'); + + childCmptInstance.skipContent = true; + detectChanges(parent); + expect(toHtml(parent)).toEqual('
'); + }); + it('should project nodes into the last ng-content', () => { /** *
diff --git a/packages/core/test/render3/integration_spec.ts b/packages/core/test/render3/integration_spec.ts index 2939216058..887904b556 100644 --- a/packages/core/test/render3/integration_spec.ts +++ b/packages/core/test/render3/integration_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {C, E, NC, T, V, a, b, b1, b2, b3, b4, b5, b6, b7, b8, bV, cR, cr, defineComponent, e, k, m, p, r, s, t, v} from '../../src/render3/index'; +import {C, E, NC, P, T, V, a, b, b1, b2, b3, b4, b5, b6, b7, b8, bV, cR, cr, defineComponent, e, k, m, p, pD, r, s, t, v} from '../../src/render3/index'; import {NO_CHANGE} from '../../src/render3/instructions'; import {containerEl, renderToHtml} from './render_util'; @@ -393,6 +393,142 @@ describe('render3 integration test', () => { }); + describe('tree', () => { + interface Tree { + beforeLabel?: string; + subTrees?: Tree[]; + afterLabel?: string; + } + + interface ParentCtx { + beforeTree: Tree; + projectedTree: Tree; + afterTree: Tree; + } + + function showLabel(ctx: {label: string | undefined}, cm: boolean) { + if (cm) { + C(0); + } + cR(0); + { + if (ctx.label != null) { + if (V(0)) { + T(0); + } + t(0, b(ctx.label)); + v(); + } + } + cr(); + } + + function showTree(ctx: {tree: Tree}, cm: boolean) { + if (cm) { + C(0); + C(1); + C(2); + } + cR(0); + { + const cm0 = V(0); + { showLabel({label: ctx.tree.beforeLabel}, cm0); } + v(); + } + cr(); + cR(1); + { + for (let subTree of ctx.tree.subTrees || []) { + const cm0 = V(0); + { showTree({tree: subTree}, cm0); } + v(); + } + } + cr(); + cR(2); + { + const cm0 = V(0); + { showLabel({label: ctx.tree.afterLabel}, cm0); } + v(); + } + cr(); + } + + class ChildComponent { + beforeTree: Tree; + afterTree: Tree; + static ngComponentDef = defineComponent({ + tag: 'child', + type: ChildComponent, + template: function ChildComponentTemplate( + ctx: {beforeTree: Tree, afterTree: Tree}, cm: boolean) { + if (cm) { + pD(0); + C(1); + P(2, 0); + C(3); + } + cR(1); + { + const cm0 = V(0); + { showTree({tree: ctx.beforeTree}, cm0); } + v(); + } + cr(); + cR(3); + { + const cm0 = V(0); + { showTree({tree: ctx.afterTree}, cm0); } + v(); + } + cr(); + }, + factory: () => new ChildComponent, + inputs: {beforeTree: 'beforeTree', afterTree: 'afterTree'} + }); + } + + function parentTemplate(ctx: ParentCtx, cm: boolean) { + if (cm) { + E(0, ChildComponent); + { C(2); } + e(); + } + p(0, 'beforeTree', b(ctx.beforeTree)); + p(0, 'afterTree', b(ctx.afterTree)); + cR(2); + { + const cm0 = V(0); + { showTree({tree: ctx.projectedTree}, cm0); } + v(); + } + cr(); + ChildComponent.ngComponentDef.h(1, 0); + r(1, 0); + } + + it('should work with a tree', () => { + + const ctx: ParentCtx = { + beforeTree: {subTrees: [{beforeLabel: 'a'}]}, + projectedTree: {beforeLabel: 'p'}, + afterTree: {afterLabel: 'z'} + }; + expect(renderToHtml(parentTemplate, ctx)).toEqual('apz'); + ctx.projectedTree = {subTrees: [{}, {}, {subTrees: [{}, {}]}, {}]}; + ctx.beforeTree.subTrees !.push({afterLabel: 'b'}); + expect(renderToHtml(parentTemplate, ctx)).toEqual('abz'); + ctx.projectedTree.subTrees ![1].afterLabel = 'h'; + expect(renderToHtml(parentTemplate, ctx)).toEqual('abhz'); + ctx.beforeTree.subTrees !.push({beforeLabel: 'c'}); + expect(renderToHtml(parentTemplate, ctx)).toEqual('abchz'); + + // To check the context easily: + // console.log(JSON.stringify(ctx)); + }); + + }); + describe('element bindings', () => { describe('elementAttribute', () => { diff --git a/packages/core/test/render3/query_spec.ts b/packages/core/test/render3/query_spec.ts index 16beab207e..12301b80ef 100644 --- a/packages/core/test/render3/query_spec.ts +++ b/packages/core/test/render3/query_spec.ts @@ -296,7 +296,7 @@ describe('query', () => { expect(isViewContainerRef(query.first)).toBeTruthy(); }); - it('should read ElementRef with a native element pointing to comment DOM node from containers', + it('should no longer read ElementRef with a native element pointing to comment DOM node from containers', () => { /** * @@ -316,8 +316,7 @@ describe('query', () => { const cmptInstance = renderComponent(Cmpt); const query = (cmptInstance.query as QueryList); expect(query.length).toBe(1); - expect(isElementRef(query.first)).toBeTruthy(); - expect(query.first.nativeElement.nodeType).toBe(8); // Node.COMMENT_NODE = 8 + expect(query.first.nativeElement).toBe(null); }); it('should read TemplateRef from container nodes by default', () => {