From e53179ef8c14f98b3cda01c553934f5e90516803 Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Tue, 29 May 2018 15:08:30 -0700 Subject: [PATCH] refactor(ivy): move parent from LNode to TNode (#24189) PR Close #24189 --- packages/core/src/render3/di.ts | 22 ++--- packages/core/src/render3/instructions.ts | 41 +++++---- packages/core/src/render3/interfaces/node.ts | 85 +++++++++++++------ .../core/src/render3/node_manipulation.ts | 19 ++++- .../hello_world/bundle.golden_symbols.json | 3 + .../bundling/todo/bundle.golden_symbols.json | 3 + packages/core/test/render3/di_spec.ts | 2 +- .../render3/node_selector_matcher_spec.ts | 1 + 8 files changed, 117 insertions(+), 59 deletions(-) diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index 05e4205888..7d66fca500 100644 --- a/packages/core/src/render3/di.ts +++ b/packages/core/src/render3/di.ts @@ -27,7 +27,7 @@ import {QueryReadType} from './interfaces/query'; import {Renderer3} from './interfaces/renderer'; import {LView, TView} from './interfaces/view'; import {assertNodeOfPossibleTypes, assertNodeType} from './node_assert'; -import {insertView, removeView} from './node_manipulation'; +import {getParentLNode, insertView, removeView} from './node_manipulation'; import {notImplemented, stringify} from './util'; import {EmbeddedViewRef, ViewRef, addDestroyable, createViewRef} from './view_ref'; @@ -102,7 +102,8 @@ export function getOrCreateNodeInjector(): LInjector { */ export function getOrCreateNodeInjectorForNode(node: LElementNode | LContainerNode): LInjector { const nodeInjector = node.nodeInjector; - const parentInjector = node.parent && node.parent.nodeInjector; + const parent = getParentLNode(node); + const parentInjector = parent && parent.nodeInjector; if (nodeInjector != parentInjector) { return nodeInjector !; } @@ -568,14 +569,15 @@ export function getOrCreateContainerRef(di: LInjector): viewEngine_ViewContainer const vcRefHost = di.node; ngDevMode && assertNodeOfPossibleTypes(vcRefHost, TNodeType.Container, TNodeType.Element); - const lContainer = createLContainer(vcRefHost.parent !, vcRefHost.view); + const hostParent = getParentLNode(vcRefHost) !; + const lContainer = createLContainer(hostParent, vcRefHost.view); const lContainerNode: LContainerNode = createLNodeObject( - TNodeType.Container, vcRefHost.view, vcRefHost.parent !, undefined, lContainer, null); + TNodeType.Container, vcRefHost.view, hostParent, undefined, lContainer, null); const hostTNode = vcRefHost.tNode; if (!hostTNode.dynamicContainerNode) { hostTNode.dynamicContainerNode = - createTNode(TNodeType.Container, hostTNode.index, null, null, null); + createTNode(TNodeType.Container, null, null, null, null, null); } lContainerNode.tNode = hostTNode.dynamicContainerNode; @@ -640,8 +642,6 @@ class ViewContainerRef implements viewEngine_ViewContainerRef { this._viewRefs.splice(adjustedIdx, 0, viewRef); - (lViewNode as{parent: LNode}).parent = this._lContainerNode; - // If the view is dynamic (has a template), it needs to be counted both at the container // level and at the node above the container. if (lViewNode.data.template !== null) { @@ -649,10 +649,10 @@ class ViewContainerRef implements viewEngine_ViewContainerRef { this._lContainerNode.data.dynamicViewCount++; // Look for the parent node and increment its dynamic view count. - if (this._lContainerNode.parent !== null && this._lContainerNode.parent.data !== null) { - ngDevMode && assertNodeOfPossibleTypes( - this._lContainerNode.parent, TNodeType.View, TNodeType.Element); - this._lContainerNode.parent.data.dynamicViewCount++; + const containerParent = getParentLNode(this._lContainerNode) as LElementNode; + if (containerParent !== null && containerParent.data !== null) { + ngDevMode && assertNodeOfPossibleTypes(containerParent, TNodeType.View, TNodeType.Element); + containerParent.data.dynamicViewCount++; } } return viewRef; diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 345e8730dd..cc90d6c24d 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -15,9 +15,9 @@ import {CssSelectorList, LProjection, NG_PROJECT_AS_ATTR_NAME} from './interface import {LQueries} from './interfaces/query'; import {CurrentMatchesList, LView, LViewFlags, LifecycleStage, RootContext, TData, TView} from './interfaces/view'; -import {LContainerNode, LElementNode, LNode, TNodeType, TNodeFlags, LProjectionNode, LTextNode, LViewNode, TNode, TContainerNode, InitialInputData, InitialInputs, PropertyAliases, PropertyAliasValue,} from './interfaces/node'; +import {LContainerNode, LElementNode, LNode, TNodeType, TNodeFlags, LProjectionNode, LTextNode, LViewNode, TNode, TContainerNode, InitialInputData, InitialInputs, PropertyAliases, PropertyAliasValue, TElementNode,} from './interfaces/node'; import {assertNodeType} from './node_assert'; -import {appendChild, insertView, appendProjectedNode, removeView, canInsertNativeNode, createTextNode, getNextLNode, getChildLNode} from './node_manipulation'; +import {appendChild, insertView, appendProjectedNode, removeView, canInsertNativeNode, createTextNode, getNextLNode, getChildLNode, getParentLNode} from './node_manipulation'; import {isNodeMatchingSelectorList, matchingSelectorIndex} from './node_selector_matcher'; import {ComponentDef, ComponentTemplate, DirectiveDef, DirectiveDefList, DirectiveDefListOrFactory, PipeDefList, PipeDefListOrFactory, RenderFlags} from './interfaces/definition'; import {RElement, RText, Renderer3, RendererFactory3, ProceduralRenderer3, RendererStyleFlags3, isProceduralRenderer} from './interfaces/renderer'; @@ -337,13 +337,12 @@ export function createLView( * (same properties assigned in the same order). */ export function createLNodeObject( - type: TNodeType, currentView: LView, parent: LNode, native: RText | RElement | null | undefined, - state: any, + type: TNodeType, currentView: LView, parent: LNode | null, + native: RText | RElement | null | undefined, state: any, queries: LQueries | null): LElementNode<extNode&LViewNode&LContainerNode&LProjectionNode { return { native: native as any, view: currentView, - parent: parent as any, nodeInjector: parent ? parent.nodeInjector : null, data: state, queries: queries, @@ -382,7 +381,11 @@ export function createLNode( name: string | null, attrs: string[] | null, state?: null | LView | LContainer | LProjection): LElementNode<extNode&LViewNode&LContainerNode&LProjectionNode { const parent = isParent ? previousOrParentNode : - previousOrParentNode && previousOrParentNode.parent as LNode; + previousOrParentNode && getParentLNode(previousOrParentNode) !as LNode; + // Parents cannot cross component boundaries because components will be used in multiple places, + // so it's only set if the view is the same. + const tParent = + parent && parent.view === currentView ? parent.tNode as TElementNode | TContainerNode : null; let queries = (isParent ? currentQueries : previousOrParentNode && previousOrParentNode.queries) || parent && parent.queries && parent.queries.child(); @@ -393,7 +396,7 @@ export function createLNode( if (index === null || type === TNodeType.View) { // View nodes are not stored in data because they can be added / removed at runtime (which // would cause indices to change). Their TNodes are instead stored in TView.node. - node.tNode = (state as LView).tView.node || createTNode(type, index, null, null, null); + node.tNode = (state as LView).tView.node || createTNode(type, index, null, null, tParent, null); } else { // This is an element or container or projection node ngDevMode && assertDataNext(index); @@ -401,7 +404,7 @@ export function createLNode( // Every node adds a value to the static data array to avoid a sparse array if (index >= tData.length) { - const tNode = tData[index] = createTNode(type, index, name, attrs, null); + const tNode = tData[index] = createTNode(type, index, name, attrs, tParent, null); if (!isParent && previousOrParentNode) { const previousTNode = previousOrParentNode.tNode; previousTNode.next = tNode; @@ -596,7 +599,7 @@ export function elementStart( createLNode(index, TNodeType.Element, native !, name, attrs || null, null); if (attrs) setUpAttributes(native, attrs); - appendChild(node.parent !, native, currentView); + appendChild(getParentLNode(node), native, currentView); createDirectivesAndLocals(index, name, attrs, localRefs, false); return native; } @@ -954,7 +957,7 @@ export function elementEnd() { isParent = false; } else { ngDevMode && assertHasParent(); - previousOrParentNode = previousOrParentNode.parent !; + previousOrParentNode = getParentLNode(previousOrParentNode) as LElementNode; } ngDevMode && assertNodeType(previousOrParentNode, TNodeType.Element); const queries = previousOrParentNode.queries; @@ -1038,12 +1041,13 @@ export function elementProperty( * @param index The index of the TNode in TView.data * @param tagName The tag name of the node * @param attrs The attributes defined on this node + * @param parent The parent of this node * @param tViews Any TViews attached to this node * @returns the TNode object */ export function createTNode( type: TNodeType, index: number | null, tagName: string | null, attrs: string[] | null, - tViews: TView[] | null): TNode { + parent: TElementNode | TContainerNode | null, tViews: TView[] | null): TNode { ngDevMode && ngDevMode.tNode++; return { type: type, @@ -1058,6 +1062,7 @@ export function createTNode( tViews: tViews, next: null, child: null, + parent: parent, dynamicContainerNode: null }; } @@ -1257,7 +1262,7 @@ export function text(index: number, value?: any): void { // Text nodes are self closing. isParent = false; - appendChild(node.parent !, textNode, currentView); + appendChild(getParentLNode(node), textNode, currentView); } /** @@ -1485,7 +1490,7 @@ export function container( currentView.bindingStartIndex, -1, 'container nodes should be created before any bindings'); - const currentParent = isParent ? previousOrParentNode : previousOrParentNode.parent !; + const currentParent = isParent ? previousOrParentNode : getParentLNode(previousOrParentNode) !; const lContainer = createLContainer(currentParent, currentView, template); const node = createLNode( @@ -1542,7 +1547,7 @@ export function containerRefreshEnd(): void { } else { ngDevMode && assertNodeType(previousOrParentNode, TNodeType.View); ngDevMode && assertHasParent(); - previousOrParentNode = previousOrParentNode.parent !; + previousOrParentNode = getParentLNode(previousOrParentNode) !; } ngDevMode && assertNodeType(previousOrParentNode, TNodeType.Container); const container = previousOrParentNode as LContainerNode; @@ -1609,7 +1614,7 @@ function scanForView( */ export function embeddedViewStart(viewBlockId: number): RenderFlags { const container = - (isParent ? previousOrParentNode : previousOrParentNode.parent !) as LContainerNode; + (isParent ? previousOrParentNode : getParentLNode(previousOrParentNode)) as LContainerNode; ngDevMode && assertNodeType(container, TNodeType.Container); const lContainer = container.data; let viewNode: LViewNode|null = scanForView(container, lContainer.nextIndex, viewBlockId); @@ -1662,7 +1667,7 @@ export function embeddedViewEnd(): void { refreshView(); isParent = false; const viewNode = previousOrParentNode = currentView.node as LViewNode; - const containerNode = previousOrParentNode.parent as LContainerNode; + const containerNode = getParentLNode(previousOrParentNode) as LContainerNode; if (containerNode) { ngDevMode && assertNodeType(viewNode, TNodeType.View); ngDevMode && assertNodeType(containerNode, TNodeType.Container); @@ -1834,7 +1839,6 @@ export function projection( // `` has no content isParent = false; - const currentParent = node.parent; // re-distribution of projectable nodes is memorized on a component's view level const componentNode = findComponentHost(currentView); @@ -1856,6 +1860,7 @@ export function projection( } } + const currentParent = getParentLNode(node); if (canInsertNativeNode(currentParent, currentView)) { ngDevMode && assertNodeType(currentParent, TNodeType.Element); // process each node in the list of projected nodes: @@ -2413,7 +2418,7 @@ export function assertPreviousIsParent() { } function assertHasParent() { - assertNotNull(previousOrParentNode.parent, 'previousOrParentNode should have a parent'); + assertNotNull(getParentLNode(previousOrParentNode), 'previousOrParentNode should have a parent'); } function assertDataInRange(index: number, arr?: any[]) { diff --git a/packages/core/src/render3/interfaces/node.ts b/packages/core/src/render3/interfaces/node.ts index 0701f960eb..c7d316ab92 100644 --- a/packages/core/src/render3/interfaces/node.ts +++ b/packages/core/src/render3/interfaces/node.ts @@ -65,12 +65,6 @@ export interface LNode { */ readonly native: RElement|RText|null|undefined; - /** - * We need a reference to a node's parent so we can append the node to its parent's native - * element at the appropriate time. - */ - readonly parent: LNode|null; - /** * If regular LElementNode, then `data` will be null. * If LElementNode with component, then `data` contains LView. @@ -127,18 +121,12 @@ export interface LElementNode extends LNode { /** If Component then data has LView (light DOM) */ readonly data: LView|null; - - /** LElementNodes can be inside other LElementNodes or inside LViewNodes. */ - readonly parent: LElementNode|LViewNode; } /** LNode representing a #text node. */ export interface LTextNode extends LNode { /** The text node associated with this node. */ native: RText; - - /** LTextNodes can be inside LElementNodes or inside LViewNodes. */ - readonly parent: LElementNode|LViewNode; readonly data: null; dynamicLContainerNode: null; } @@ -146,9 +134,6 @@ export interface LTextNode extends LNode { /** Abstract node which contains root nodes of a view. */ export interface LViewNode extends LNode { readonly native: null; - - /** LViewNodes can only be added to LContainerNodes. */ - readonly parent: LContainerNode|null; readonly data: LView; dynamicLContainerNode: null; } @@ -164,19 +149,12 @@ export interface LContainerNode extends LNode { */ native: RElement|RText|null|undefined; readonly data: LContainer; - - /** Containers can be added to elements or views. */ - readonly parent: LElementNode|LViewNode|null; } export interface LProjectionNode extends LNode { readonly native: null; - readonly data: LProjection; - - /** Projections can be added to elements or views. */ - readonly parent: LElementNode|LViewNode; dynamicLContainerNode: null; } @@ -201,7 +179,7 @@ export interface TNode { * This is necessary to get from any TNode to its corresponding LNode when * traversing the node tree. * - * If null, this is a view node created from a dynamically created view. + * If null, this is a dynamically created container node or embedded view node. */ index: number|null; @@ -307,6 +285,22 @@ export interface TNode { */ child: TNode|null; + /** + * Parent node (in the same view only). + * + * We need a reference to a node's parent so we can append the node to its parent's native + * element at the appropriate time. + * + * If the parent would be in a different view (e.g. component host), this property will be null. + * It's important that we don't try to cross component boundaries when retrieving the parent + * because the parent will change (e.g. index, attrs) depending on where the component was + * used (and thus shouldn't be stored on TNode). In these cases, we retrieve the parent through + * LView.node instead (which will be instance-specific). + * + * If this is an inline view node (V), the parent will be its container. + */ + parent: TElementNode|TContainerNode|null; + /** * A pointer to a TContainerNode created by directives requesting ViewContainerRef */ @@ -315,31 +309,72 @@ export interface TNode { /** Static data for an LElementNode */ export interface TElementNode extends TNode { - child: TContainerNode|TElementNode|TProjectionNode|null; + /** Index in the data[] array */ + index: number; + child: TElementNode|TTextNode|TContainerNode|TProjectionNode|null; + /** + * Element nodes will have parents unless they are the first node of a component or + * embedded view (which means their parent is in a different view and must be + * retrieved using LView.node). + */ + parent: TElementNode|null; tViews: null; } /** Static data for an LTextNode */ export interface TTextNode extends TNode { + /** Index in the data[] array */ + index: number; child: null; + /** + * Text nodes will have parents unless they are the first node of a component or + * embedded view (which means their parent is in a different view and must be + * retrieved using LView.node). + */ + parent: TElementNode|null; tViews: null; } /** Static data for an LContainerNode */ export interface TContainerNode extends TNode { + /** + * If number, index in the data[] array. + * + * If null, this is a dynamically created container node that isn't stored in + * data[] (e.g. when you inject ViewContainerRef) . + */ + index: number|null; child: null; + + /** + * Container nodes will have parents unless: + * + * - They are the first node of a component or embedded view + * - They are dynamically created + */ + parent: TElementNode|null; tViews: TView|TView[]|null; } /** Static data for an LViewNode */ export interface TViewNode extends TNode { - child: TContainerNode|TElementNode|TProjectionNode|null; + /** If null, it's a dynamically created view*/ + index: number|null; + child: TElementNode|TTextNode|TContainerNode|TProjectionNode|null; + parent: TContainerNode|null; tViews: null; } /** Static data for an LProjectionNode */ export interface TProjectionNode extends TNode { + /** Index in the data[] array */ child: null; + /** + * Projection nodes will have parents unless they are the first node of a component + * or embedded view (which means their parent is in a different view and must be + * retrieved using LView.node). + */ + parent: TElementNode|null; tViews: null; } diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index 3ae554dd92..cb0bc5bdee 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -52,7 +52,7 @@ function findNextRNodeSibling(node: LNode | null, stopNode: LNode | null): RElem } currentSibling = getNextLNode(currentSibling); } - const parentNode = currentNode.parent; + const parentNode = getParentLNode(currentNode); currentNode = null; if (parentNode) { const parentType = parentNode.tNode.type; @@ -84,6 +84,17 @@ export function getChildLNode(node: LNode): LNode|null { return null; } +/** Retrieves the parent LNode of a given node. */ +export function getParentLNode(node: LElementNode | LTextNode | LProjectionNode): LElementNode| + LViewNode; +export function getParentLNode(node: LViewNode): LContainerNode|null; +export function getParentLNode(node: LNode): LElementNode|LContainerNode|LViewNode|null; +export function getParentLNode(node: LNode): LElementNode|LContainerNode|LViewNode|null { + if (node.tNode.index === null) return null; + const parent = node.tNode.parent; + return parent ? node.view.data[parent.index as number] : node.view.node; +} + /** * 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). @@ -122,7 +133,7 @@ function getNextOrParentSiblingNode(initialNode: LNode, rootNode: LNode): LNode| 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; + node = node.pNextOrParent || getParentLNode(node); if (node === rootNode) { return null; } @@ -371,7 +382,7 @@ export function getParentState(state: LViewOrLContainer, rootView: LView): LView if ((node = (state as LView) !.node) && node.tNode.type === TNodeType.View) { // if it's an embedded view, the state needs to go up to the container, in case the // container has a next - return node.parent !.data as any; + return getParentLNode(node) !.data as any; } else { // otherwise, use parent view for containers or component views return state.parent === rootView ? null : state.parent; @@ -476,7 +487,7 @@ export function appendChild(parent: LNode, child: RNode | null, currentView: LVi * @param currentView Current LView */ export function insertChild(node: LNode, currentView: LView): void { - const parent = node.parent !; + const parent = getParentLNode(node) !; if (canInsertNativeNode(parent, currentView)) { let nativeSibling: RNode|null = findNextRNodeSibling(node, null); const renderer = currentView.renderer; 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 63f67c0bda..3a97e3864e 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -110,6 +110,9 @@ { "name": "getOrCreateTView" }, + { + "name": "getParentLNode" + }, { "name": "getRenderFlags" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 7cdd8fb784..34d33906bc 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -422,6 +422,9 @@ { "name": "getOrCreateTemplateRef" }, + { + "name": "getParentLNode" + }, { "name": "getParentState" }, diff --git a/packages/core/test/render3/di_spec.ts b/packages/core/test/render3/di_spec.ts index cc3bded98b..73dc163330 100644 --- a/packages/core/test/render3/di_spec.ts +++ b/packages/core/test/render3/di_spec.ts @@ -1372,7 +1372,7 @@ describe('di', () => { // Simulate the situation where the previous parent is not initialized. // This happens on first bootstrap because we don't init existing values // so that we have smaller HelloWorld. - (parent as{parent: any}).parent = undefined; + (parent.tNode as{parent: any}).parent = undefined; const injector = getOrCreateNodeInjector(); expect(injector).not.toBe(null); diff --git a/packages/core/test/render3/node_selector_matcher_spec.ts b/packages/core/test/render3/node_selector_matcher_spec.ts index d98944e264..53b432ea56 100644 --- a/packages/core/test/render3/node_selector_matcher_spec.ts +++ b/packages/core/test/render3/node_selector_matcher_spec.ts @@ -23,6 +23,7 @@ function testLStaticData(tagName: string, attrs: string[] | null): TNode { tViews: null, next: null, child: null, + parent: null, dynamicContainerNode: null }; }