From 821665768186a78837671975c26f67ed37cb82eb Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Wed, 16 May 2018 05:56:01 -0700 Subject: [PATCH] refactor(ivy): add tNodes for view nodes and hosts (#24113) PR Close #24113 --- packages/core/src/render3/component.ts | 2 +- packages/core/src/render3/di.ts | 20 +++-- packages/core/src/render3/instructions.ts | 79 +++++++++++-------- packages/core/src/render3/interfaces/node.ts | 12 ++- packages/core/src/render3/interfaces/view.ts | 12 +++ .../core/src/render3/node_manipulation.ts | 2 +- packages/core/src/render3/query.ts | 5 +- .../core/test/render3/instructions_spec.ts | 6 +- .../core/test/render3/integration_spec.ts | 2 +- .../render3/node_selector_matcher_spec.ts | 3 +- 10 files changed, 92 insertions(+), 51 deletions(-) diff --git a/packages/core/src/render3/component.ts b/packages/core/src/render3/component.ts index a0cc5a4602..53f7d15565 100644 --- a/packages/core/src/render3/component.ts +++ b/packages/core/src/render3/component.ts @@ -187,7 +187,7 @@ export function LifecycleHooksFeature(component: any, def: ComponentDef): v // Root component is always created at dir index 0 queueInitHooks(0, def.onInit, def.doCheck, elementNode.view.tView); - queueLifecycleHooks(elementNode.tNode !.flags, elementNode.view); + queueLifecycleHooks(elementNode.tNode.flags, elementNode.view); } /** diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index 030afd5ac6..a62b8a17d7 100644 --- a/packages/core/src/render3/di.ts +++ b/packages/core/src/render3/di.ts @@ -19,7 +19,7 @@ import {EmbeddedViewRef as viewEngine_EmbeddedViewRef, ViewRef as viewEngine_Vie import {Type} from '../type'; import {assertGreaterThan, assertLessThan, assertNotNull} from './assert'; -import {addToViewTree, assertPreviousIsParent, createLContainer, createLNodeObject, createTView, getDirectiveInstance, getPreviousOrParentNode, getRenderer, isComponent, renderEmbeddedTemplate, resolveDirective} from './instructions'; +import {addToViewTree, assertPreviousIsParent, createLContainer, createLNodeObject, createTNode, createTView, getDirectiveInstance, getPreviousOrParentNode, getRenderer, isComponent, renderEmbeddedTemplate, resolveDirective} from './instructions'; import {ComponentTemplate, DirectiveDef, DirectiveDefList, PipeDefList} from './interfaces/definition'; import {LInjector} from './interfaces/injector'; import {LContainerNode, LElementNode, LNode, LNodeType, LViewNode, TNodeFlags} from './interfaces/node'; @@ -254,7 +254,7 @@ export function injectAttribute(attrName: string): string|undefined { ngDevMode && assertPreviousIsParent(); const lElement = getPreviousOrParentNode() as LElementNode; ngDevMode && assertNodeType(lElement, LNodeType.Element); - const tElement = lElement.tNode !; + const tElement = lElement.tNode; ngDevMode && assertNotNull(tElement, 'expecting tNode'); const attrs = tElement.attrs; if (attrs) { @@ -278,7 +278,7 @@ export function getOrCreateChangeDetectorRef( if (di.changeDetectorRef) return di.changeDetectorRef; const currentNode = di.node; - if (isComponent(currentNode.tNode !)) { + if (isComponent(currentNode.tNode)) { return di.changeDetectorRef = createViewRef(currentNode.data as LView, context); } else if (currentNode.type === LNodeType.Element) { return di.changeDetectorRef = getOrCreateHostChangeDetector(currentNode.view.node); @@ -298,7 +298,7 @@ function getOrCreateHostChangeDetector(currentNode: LViewNode | LElementNode): createViewRef( hostNode.data as LView, hostNode.view - .directives ![hostNode.tNode !.flags >> TNodeFlags.DirectiveStartingIndexShift]); + .directives ![hostNode.tNode.flags >> TNodeFlags.DirectiveStartingIndexShift]); } /** @@ -361,7 +361,7 @@ export function getOrCreateInjectable( // At this point, we have an injector which *may* contain the token, so we step through the // directives associated with the injector's corresponding node to get the directive instance. const node = injector.node; - const nodeFlags = node.tNode !.flags; + const nodeFlags = node.tNode.flags; const count = nodeFlags & TNodeFlags.DirectiveCountMask; if (count !== 0) { @@ -572,8 +572,12 @@ export function getOrCreateContainerRef(di: LInjector): viewEngine_ViewContainer const lContainerNode: LContainerNode = createLNodeObject( LNodeType.Container, vcRefHost.view, vcRefHost.parent !, undefined, lContainer, null); - // TODO(kara): Separate into own TNode when moving parent/child properties - lContainerNode.tNode = vcRefHost.tNode; + const hostTNode = vcRefHost.tNode; + if (!hostTNode.dynamicContainerNode) { + hostTNode.dynamicContainerNode = createTNode(hostTNode.index, null, null, null); + } + + lContainerNode.tNode = hostTNode.dynamicContainerNode; vcRefHost.dynamicLContainerNode = lContainerNode; addToViewTree(vcRefHost.view, lContainer); @@ -699,7 +703,7 @@ export function getOrCreateTemplateRef(di: LInjector): viewEngine_TemplateRef if (!di.templateRef) { ngDevMode && assertNodeType(di.node, LNodeType.Container); const hostNode = di.node as LContainerNode; - const hostTNode = hostNode.tNode !; + const hostTNode = hostNode.tNode; const hostTView = hostNode.view.tView; if (!hostTNode.tViews) { hostTNode.tViews = createTView(hostTView.directiveRegistry, hostTView.pipeRegistry); diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 9768799c7c..4ee9a40b64 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -349,7 +349,7 @@ export function createLNodeObject( nodeInjector: parent ? parent.nodeInjector : null, data: state, queries: queries, - tNode: null, + tNode: null !, pNextOrParent: null, dynamicLContainerNode: null }; @@ -371,7 +371,7 @@ export function createLNode( index: number | null, type: LNodeType.Element, native: RElement | RText | null, name: string | null, attrs: string[] | null, lView?: LView | null): LElementNode; export function createLNode( - index: null, type: LNodeType.View, native: null, name: null, attrs: null, + index: number | null, type: LNodeType.View, native: null, name: null, attrs: null, lView: LView): LViewNode; export function createLNode( index: number, type: LNodeType.Container, native: undefined, name: string | null, @@ -392,14 +392,12 @@ export function createLNode( const node = createLNodeObject(type, currentView, parent, native, isState ? state as any : null, queries); - if ((type & LNodeType.ViewOrElement) === LNodeType.ViewOrElement && isState) { - // Bit of a hack to bust through the readonly because there is a circular dep between - // LView and LNode. - ngDevMode && assertNull((state as LView).node, 'LView.node should not have been initialized'); - (state as LView as{node: LNode}).node = node; - } - if (index != null) { - // We are Element or Container + if (index === null || type === LNodeType.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(index, null, null, null); + } else { + // This is an element or container or projection node ngDevMode && assertDataNext(index); data[index] = node; @@ -407,7 +405,9 @@ export function createLNode( if (index >= tData.length) { const tNode = tData[index] = createTNode(index, name, attrs, null); if (!isParent && previousOrParentNode) { - previousOrParentNode.tNode !.next = tNode; + const previousTNode = previousOrParentNode.tNode; + previousTNode.next = tNode; + if (previousTNode.dynamicContainerNode) previousTNode.dynamicContainerNode.next = tNode; } } node.tNode = tData[index] as TNode; @@ -427,6 +427,16 @@ export function createLNode( } } } + + // View nodes and host elements need to set their host node (components set host nodes later) + if ((type & LNodeType.ViewOrElement) === LNodeType.ViewOrElement && isState) { + // Bit of a hack to bust through the readonly because there is a circular dep between + // LView and LNode. + ngDevMode && assertNull((state as LView).node, 'LView.node should not have been initialized'); + (state as{node: LNode}).node = node; + if (firstTemplatePass) (state as LView).tView.node = node.tNode; + } + previousOrParentNode = node; isParent = true; return node; @@ -613,7 +623,7 @@ function createDirectivesAndLocals( const node = previousOrParentNode; if (firstTemplatePass) { ngDevMode && ngDevMode.firstTemplatePass++; - cacheMatchingDirectivesForNode(node.tNode !, currentView.tView, localRefs || null); + cacheMatchingDirectivesForNode(node.tNode, currentView.tView, localRefs || null); } else { instantiateDirectivesDirectly(); } @@ -708,7 +718,7 @@ export function isComponent(tNode: TNode): boolean { * This function instantiates the given directives. */ function instantiateDirectivesDirectly() { - const tNode = previousOrParentNode.tNode !; + const tNode = previousOrParentNode.tNode; const count = tNode.flags & TNodeFlags.DirectiveCountMask; if (count > 0) { @@ -758,7 +768,7 @@ function saveNameToExportMap( * to data[] in the same order as they are loaded in the template with load(). */ function saveResolvedLocalsInData(): void { - const localNames = previousOrParentNode.tNode !.localNames; + const localNames = previousOrParentNode.tNode.localNames; if (localNames) { for (let i = 0; i < localNames.length; i += 2) { const index = localNames[i + 1] as number; @@ -796,6 +806,7 @@ export function createTView( defs: DirectiveDefListOrFactory | null, pipes: PipeDefListOrFactory | null): TView { ngDevMode && ngDevMode.tView++; return { + node: null !, data: [], directives: null, firstTemplatePass: true, @@ -879,7 +890,7 @@ export function hostElement( def.onPush ? LViewFlags.Dirty : LViewFlags.CheckAlways, sanitizer)); if (firstTemplatePass) { - node.tNode !.flags = TNodeFlags.isComponent; + node.tNode.flags = TNodeFlags.isComponent; if (def.diPublic) def.diPublic(def); currentView.tView.directives = [def]; } @@ -918,11 +929,11 @@ export function listener( cleanupFns.push(eventName, native, wrappedListener, useCapture); } - let tNode: TNode|null = node.tNode !; + let tNode: TNode|null = node.tNode; if (tNode.outputs === undefined) { // if we create TNode here, inputs must be undefined so we know they still need to be // checked - tNode.outputs = generatePropertyAliases(node.tNode !.flags, BindingDirection.Output); + tNode.outputs = generatePropertyAliases(node.tNode.flags, BindingDirection.Output); } const outputs = tNode.outputs; @@ -955,7 +966,7 @@ export function elementEnd() { ngDevMode && assertNodeType(previousOrParentNode, LNodeType.Element); const queries = previousOrParentNode.queries; queries && queries.addNode(previousOrParentNode); - queueLifecycleHooks(previousOrParentNode.tNode !.flags, currentView); + queueLifecycleHooks(previousOrParentNode.tNode.flags, currentView); } /** @@ -1002,12 +1013,12 @@ export function elementProperty( index: number, propName: string, value: T | NO_CHANGE, sanitizer?: SanitizerFn): void { if (value === NO_CHANGE) return; const node = data[index] as LElementNode; - const tNode = node.tNode !; + const tNode = node.tNode; // if tNode.inputs is undefined, a listener has created outputs, but inputs haven't // yet been checked if (tNode && tNode.inputs === undefined) { // mark inputs as checked - tNode.inputs = generatePropertyAliases(node.tNode !.flags, BindingDirection.Input); + tNode.inputs = generatePropertyAliases(node.tNode.flags, BindingDirection.Input); } const inputData = tNode && tNode.inputs; @@ -1036,8 +1047,9 @@ export function elementProperty( * @param tViews Any TViews attached to this node * @returns the TNode object */ -function createTNode( - index: number, tagName: string | null, attrs: string[] | null, tViews: TView[] | null): TNode { +export function createTNode( + index: number | null, tagName: string | null, attrs: string[] | null, + tViews: TView[] | null): TNode { ngDevMode && ngDevMode.tNode++; return { index: index, @@ -1049,7 +1061,8 @@ function createTNode( inputs: undefined, outputs: undefined, tViews: tViews, - next: null + next: null, + dynamicContainerNode: null }; } @@ -1321,8 +1334,11 @@ function addComponentLogic(index: number, instance: T, def: ComponentDef): tView, null, null, def.onPush ? LViewFlags.Dirty : LViewFlags.CheckAlways, getCurrentSanitizer())); - (previousOrParentNode.data as any) = hostView; - (hostView.node as any) = previousOrParentNode; + // We need to set the host node/data here because when the component LNode was created, + // we didn't yet know it was a component (just an element). + (previousOrParentNode as{data: LView}).data = hostView; + (hostView as{node: LNode}).node = previousOrParentNode; + if (firstTemplatePass) tView.node = previousOrParentNode.tNode; initChangeDetectorIfExisting(previousOrParentNode.nodeInjector, instance, hostView); @@ -1351,19 +1367,19 @@ export function baseDirectiveCreate( directives[index] = directive; if (firstTemplatePass) { - const flags = previousOrParentNode.tNode !.flags; + const flags = previousOrParentNode.tNode.flags; if ((flags & TNodeFlags.DirectiveCountMask) === 0) { // When the first directive is created: // - save the index, // - set the number of directives to 1 - previousOrParentNode.tNode !.flags = + previousOrParentNode.tNode.flags = index << TNodeFlags.DirectiveStartingIndexShift | flags & TNodeFlags.isComponent | 1; } else { // Only need to bump the size when subsequent directives are created ngDevMode && assertNotEqual( flags & TNodeFlags.DirectiveCountMask, TNodeFlags.DirectiveCountMask, 'Reached the max number of directives'); - previousOrParentNode.tNode !.flags++; + previousOrParentNode.tNode.flags++; } } else { const diPublic = directiveDef !.diPublic; @@ -1481,7 +1497,7 @@ export function container( const node = createLNode( index, LNodeType.Container, undefined, tagName || null, attrs || null, lContainer); - if (firstTemplatePass && template == null) node.tNode !.tViews = []; + if (firstTemplatePass && template == null) node.tNode.tViews = []; // Containers are added to the current view tree instead of their embedded views // because views can be removed and re-inserted. @@ -1618,7 +1634,8 @@ export function embeddedViewStart(viewBlockId: number): RenderFlags { newView.queries = lContainer.queries.enterView(lContainer.nextIndex); } - enterView(newView, viewNode = createLNode(null, LNodeType.View, null, null, null, newView)); + enterView( + newView, viewNode = createLNode(viewBlockId, LNodeType.View, null, null, null, newView)); } return getRenderFlags(viewNode.data); } @@ -2027,7 +2044,7 @@ export function getRootView(component: any): LView { export function detectChanges(component: T): void { const hostNode = _getComponentHostLElementNode(component); ngDevMode && assertNotNull(hostNode.data, 'Component host node should be attached to an LView'); - const componentIndex = hostNode.tNode !.flags >> TNodeFlags.DirectiveStartingIndexShift; + const componentIndex = hostNode.tNode.flags >> TNodeFlags.DirectiveStartingIndexShift; const def = hostNode.view.tView.directives ![componentIndex] as ComponentDef; detectChangesInternal(hostNode.data as LView, hostNode, def, component); } diff --git a/packages/core/src/render3/interfaces/node.ts b/packages/core/src/render3/interfaces/node.ts index ad323d052f..1775469c51 100644 --- a/packages/core/src/render3/interfaces/node.ts +++ b/packages/core/src/render3/interfaces/node.ts @@ -118,11 +118,12 @@ export interface LNode { * Pointer to the corresponding TNode object, which stores static * data about this node. */ - tNode: TNode|null; + tNode: TNode; /** * A pointer to a LContainerNode created by directives requesting ViewContainerRef */ + // TODO(kara): Remove when removing LNodes dynamicLContainerNode: LContainerNode|null; } @@ -210,8 +211,10 @@ 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. */ - index: number; + index: number|null; /** * This number stores two values using its bits: @@ -306,6 +309,11 @@ export interface TNode { * to insert them or remove them from the DOM. */ next: TNode|null; + + /** + * A pointer to a LContainerNode created by directives requesting ViewContainerRef + */ + dynamicContainerNode: TNode|null; } /** Static data for an LElementNode */ diff --git a/packages/core/src/render3/interfaces/view.ts b/packages/core/src/render3/interfaces/view.ts index 1e412140b2..a405cdde93 100644 --- a/packages/core/src/render3/interfaces/view.ts +++ b/packages/core/src/render3/interfaces/view.ts @@ -46,6 +46,7 @@ export interface LView { * * If `LElementNode`, this is the LView of a component. */ + // TODO(kara): Remove when we have parent/child on TNodes readonly node: LViewNode|LElementNode; /** @@ -241,6 +242,17 @@ export interface LViewOrLContainer { * Stored on the template function as ngPrivateData. */ export interface TView { + /** + * Pointer to the `TNode` that represents the root of the view. + * + * If this is a `TNode` for an `LViewNode`, this is an embedded view of a container. + * We need this pointer to be able to efficiently find this node when inserting the view + * into an anchor. + * + * If this is a `TNode` for an `LElementNode`, this is the TView of a component. + */ + node: TNode; + /** Whether or not this template has been processed. */ firstTemplatePass: boolean; diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index c7c750d364..8a797bfb4e 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -72,7 +72,7 @@ export function getNextLNode(node: LNode): LNode|null { const lView = node.data as LView; return lView.next ? (lView.next as LView).node : null; } - return node.tNode !.next ? node.view.data[node.tNode !.next !.index] : null; + return node.tNode.next ? node.view.data[node.tNode.next !.index as number] : null; } /** diff --git a/packages/core/src/render3/query.ts b/packages/core/src/render3/query.ts index e81fcaf1f9..4502b47b9c 100644 --- a/packages/core/src/render3/query.ts +++ b/packages/core/src/render3/query.ts @@ -195,7 +195,7 @@ function getIdxOfMatchingSelector(tNode: TNode, selector: string): number|null { */ function getIdxOfMatchingDirective(node: LNode, type: Type): number|null { const defs = node.view.tView.directives !; - const flags = node.tNode !.flags; + const flags = node.tNode.flags; const count = flags & TNodeFlags.DirectiveCountMask; const start = flags >> TNodeFlags.DirectiveStartingIndexShift; const end = start + count; @@ -241,8 +241,7 @@ function add(query: LQuery| null, node: LNode) { } else { const selector = predicate.selector !; for (let i = 0; i < selector.length; i++) { - ngDevMode && assertNotNull(node.tNode, 'node.tNode'); - const directiveIdx = getIdxOfMatchingSelector(node.tNode !, selector[i]); + const directiveIdx = getIdxOfMatchingSelector(node.tNode, selector[i]); if (directiveIdx !== null) { // a node is matching a predicate - determine what to read // note that queries using name selector must specify read strategy diff --git a/packages/core/test/render3/instructions_spec.ts b/packages/core/test/render3/instructions_spec.ts index 18be0225ca..43a0de4031 100644 --- a/packages/core/test/render3/instructions_spec.ts +++ b/packages/core/test/render3/instructions_spec.ts @@ -48,7 +48,7 @@ describe('instructions', () => { expect(t.html).toEqual('
'); expect(ngDevMode).toHaveProperties({ firstTemplatePass: 1, - tNode: 1, + tNode: 2, // 1 for div, 1 for host element tView: 1, rendererCreateElement: 1, rendererSetAttribute: 2 @@ -69,7 +69,7 @@ describe('instructions', () => { expect(t.html).toEqual('
'); expect(ngDevMode).toHaveProperties({ firstTemplatePass: 1, - tNode: 1, + tNode: 2, // 1 for div, 1 for host element tView: 1, rendererCreateElement: 1, }); @@ -83,7 +83,7 @@ describe('instructions', () => { expect((t.hostNode.native as HTMLElement).querySelector('div') !.hidden).toEqual(false); expect(ngDevMode).toHaveProperties({ firstTemplatePass: 1, - tNode: 1, + tNode: 2, // 1 for div, 1 for host element tView: 1, rendererCreateElement: 1, rendererSetProperty: 1 diff --git a/packages/core/test/render3/integration_spec.ts b/packages/core/test/render3/integration_spec.ts index e0325ab458..7fea1a24f3 100644 --- a/packages/core/test/render3/integration_spec.ts +++ b/packages/core/test/render3/integration_spec.ts @@ -32,7 +32,7 @@ describe('render3 integration test', () => { } expect(ngDevMode).toHaveProperties({ firstTemplatePass: 1, - tNode: 2, + tNode: 3, // 1 for div, 1 for text, 1 for host element tView: 1, rendererCreateElement: 1, }); diff --git a/packages/core/test/render3/node_selector_matcher_spec.ts b/packages/core/test/render3/node_selector_matcher_spec.ts index 08a68d8cf3..e03deec396 100644 --- a/packages/core/test/render3/node_selector_matcher_spec.ts +++ b/packages/core/test/render3/node_selector_matcher_spec.ts @@ -19,7 +19,8 @@ function testLStaticData(tagName: string, attrs: string[] | null): TNode { inputs: undefined, outputs: undefined, tViews: null, - next: null + next: null, + dynamicContainerNode: null }; }