From 6f13bbcf273017ceac3a8a6bf186ec97fae1c01f Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Thu, 12 Apr 2018 14:52:00 -0700 Subject: [PATCH] refactor(ivy): misc --- packages/core/src/render3/di.ts | 21 ++++---- packages/core/src/render3/hooks.ts | 12 ++--- packages/core/src/render3/instructions.ts | 52 ++++++++++++-------- packages/core/src/render3/interfaces/node.ts | 22 +++------ packages/core/src/render3/interfaces/view.ts | 4 +- packages/core/src/render3/query.ts | 14 +++--- packages/core/test/render3/di_spec.ts | 2 +- 7 files changed, 65 insertions(+), 62 deletions(-) diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index 964eab955e1d2..99d83eb6e897e 100644 --- a/packages/core/src/render3/di.ts +++ b/packages/core/src/render3/di.ts @@ -318,7 +318,8 @@ function getOrCreateHostChangeDetector(currentNode: LViewNode | LElementNode): existingRef : createViewRef( hostNode.data as LView, - hostNode.view.directives ![hostNode.tNode !.flags >> TNodeFlags.INDX_SHIFT]); + hostNode.view + .directives ![hostNode.tNode !.flags >> TNodeFlags.DirectiveStartingIndexShift]); } /** @@ -382,23 +383,19 @@ 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; - - // The size of the node's directive's list is stored in certain bits of the node's flags, - // so exact it with a mask and shift it back such that the bits reflect the real value. const flags = node.tNode !.flags; - const size = (flags & TNodeFlags.SIZE_MASK) >> TNodeFlags.SIZE_SHIFT; - - if (size !== 0) { - // The start index of the directives list is also part of the node's flags, but there is - // nothing to the "left" of it so it doesn't need a mask. - const start = flags >> TNodeFlags.INDX_SHIFT; + const count = flags & TNodeFlags.DirectiveCountMask; + if (count !== 0) { + const start = flags >> TNodeFlags.DirectiveStartingIndexShift; + const end = start + count; const defs = node.view.tView.directives !; - for (let i = start, ii = start + size; i < ii; i++) { + + for (let i = start; i < end; i++) { // Get the definition for the directive at this index and, if it is injectable (diPublic), // and matches the given token, return the directive instance. const directiveDef = defs[i] as DirectiveDef; - if (directiveDef.diPublic && directiveDef.type == token) { + if (directiveDef.type === token && directiveDef.diPublic) { return getDirectiveInstance(node.view.directives ![i]); } } diff --git a/packages/core/src/render3/hooks.ts b/packages/core/src/render3/hooks.ts index 07f78b0019a27..0094566580e6a 100644 --- a/packages/core/src/render3/hooks.ts +++ b/packages/core/src/render3/hooks.ts @@ -44,15 +44,15 @@ export function queueInitHooks( export function queueLifecycleHooks(flags: number, currentView: LView): void { const tView = currentView.tView; if (tView.firstTemplatePass === true) { - const start = flags >> TNodeFlags.INDX_SHIFT; - const size = (flags & TNodeFlags.SIZE_MASK) >> TNodeFlags.SIZE_SHIFT; - const end = start + size; + const start = flags >> TNodeFlags.DirectiveStartingIndexShift; + const count = flags & TNodeFlags.DirectiveCountMask; + const end = start + count; // It's necessary to loop through the directives at elementEnd() (rather than processing in // directiveCreate) so we can preserve the current hook order. Content, view, and destroy // hooks for projected components and directives must be called *before* their hosts. for (let i = start; i < end; i++) { - const def = (tView.directives ![i] as DirectiveDef); + const def: DirectiveDef = tView.directives ![i]; queueContentHooks(def, tView, i); queueViewHooks(def, tView, i); queueDestroyHooks(def, tView, i); @@ -97,9 +97,9 @@ function queueDestroyHooks(def: DirectiveDef, tView: TView, i: number): voi * @param currentView The current view */ export function executeInitHooks(currentView: LView, tView: TView, creationMode: boolean): void { - if (currentView.lifecycleStage === LifecycleStage.INIT) { + if (currentView.lifecycleStage === LifecycleStage.Init) { executeHooks(currentView.directives !, tView.initHooks, tView.checkHooks, creationMode); - currentView.lifecycleStage = LifecycleStage.AFTER_INIT; + currentView.lifecycleStage = LifecycleStage.AfterInit; } } diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index ded6568583729..09f3328836846 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -232,7 +232,7 @@ export function leaveView(newView: LView): void { } // Views should be clean and in update mode after being checked, so these bits are cleared currentView.flags &= ~(LViewFlags.CreationMode | LViewFlags.Dirty); - currentView.lifecycleStage = LifecycleStage.INIT; + currentView.lifecycleStage = LifecycleStage.Init; currentView.bindingIndex = -1; enterView(newView, null); } @@ -299,7 +299,7 @@ export function createLView( template: template, context: context, dynamicViewCount: 0, - lifecycleStage: LifecycleStage.INIT, + lifecycleStage: LifecycleStage.Init, queries: null, }; @@ -599,8 +599,8 @@ function findDirectiveMatches(tNode: TNode): CurrentMatchesList|null { const def = registry[i]; if (isNodeMatchingSelectorList(tNode, def.selectors !)) { if ((def as ComponentDef).template) { - if (tNode.flags & TNodeFlags.Component) throwMultipleComponentError(tNode); - tNode.flags = TNodeFlags.Component; + if (tNode.flags & TNodeFlags.isComponent) throwMultipleComponentError(tNode); + tNode.flags = TNodeFlags.isComponent; } if (def.diPublic) def.diPublic(def); (matches || (matches = [])).push(def, null); @@ -650,7 +650,7 @@ export function initChangeDetectorIfExisting( } export function isComponent(tNode: TNode): boolean { - return (tNode.flags & TNodeFlags.Component) === TNodeFlags.Component; + return (tNode.flags & TNodeFlags.isComponent) === TNodeFlags.isComponent; } /** @@ -658,14 +658,15 @@ export function isComponent(tNode: TNode): boolean { */ function instantiateDirectivesDirectly() { const tNode = previousOrParentNode.tNode !; - const size = (tNode.flags & TNodeFlags.SIZE_MASK) >> TNodeFlags.SIZE_SHIFT; + const count = tNode.flags & TNodeFlags.DirectiveCountMask; - if (size > 0) { - const startIndex = tNode.flags >> TNodeFlags.INDX_SHIFT; + if (count > 0) { + const start = tNode.flags >> TNodeFlags.DirectiveStartingIndexShift; + const end = start + count; const tDirectives = currentView.tView.directives !; - for (let i = startIndex; i < startIndex + size; i++) { - const def = tDirectives[i] as DirectiveDef; + for (let i = start; i < end; i++) { + const def: DirectiveDef = tDirectives[i]; directiveCreate(i, def.factory(), def); } } @@ -818,7 +819,7 @@ export function hostElement( if (firstTemplatePass) { node.tNode = createTNode(tag as string, null, null); - node.tNode.flags = TNodeFlags.Component; + node.tNode.flags = TNodeFlags.isComponent; if (def.diPublic) def.diPublic(def); currentView.tView.directives = [def]; } @@ -1005,15 +1006,16 @@ function setInputsForProperty(inputs: PropertyAliasValue, value: any): void { */ function generatePropertyAliases( tNodeFlags: TNodeFlags, direction: BindingDirection): PropertyAliases|null { - const size = (tNodeFlags & TNodeFlags.SIZE_MASK) >> TNodeFlags.SIZE_SHIFT; + const count = tNodeFlags & TNodeFlags.DirectiveCountMask; let propStore: PropertyAliases|null = null; - if (size > 0) { - const start = tNodeFlags >> TNodeFlags.INDX_SHIFT; + if (count > 0) { + const start = tNodeFlags >> TNodeFlags.DirectiveStartingIndexShift; + const end = start + count; const isInput = direction === BindingDirection.Input; const defs = currentView.tView.directives !; - for (let i = start, ii = start + size; i < ii; i++) { + for (let i = start; i < end; i++) { const directiveDef = defs[i] as DirectiveDef; const propertyAliasMap: {[publicName: string]: string} = isInput ? directiveDef.inputs : directiveDef.outputs; @@ -1211,7 +1213,7 @@ export function directiveCreate( const instance = baseDirectiveCreate(index, directive, directiveDef); ngDevMode && assertNotNull(previousOrParentNode.tNode, 'previousOrParentNode.tNode'); - const tNode: TNode|null = previousOrParentNode.tNode !; + const tNode = previousOrParentNode.tNode; const isComponent = (directiveDef as ComponentDef).template; if (isComponent) { @@ -1275,9 +1277,19 @@ export function baseDirectiveCreate( if (firstTemplatePass) { const flags = previousOrParentNode.tNode !.flags; - previousOrParentNode.tNode !.flags = (flags & TNodeFlags.SIZE_MASK) === 0 ? - (index << TNodeFlags.INDX_SHIFT) | TNodeFlags.SIZE_SKIP | flags & TNodeFlags.Component : - flags + TNodeFlags.SIZE_SKIP; + if ((flags & TNodeFlags.DirectiveCountMask) === 0) { + // When the first directive is created: + // - save the index, + // - set the number of directives to 1 + 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++; + } } else { const diPublic = directiveDef !.diPublic; if (diPublic) diPublic(directiveDef !); @@ -1932,7 +1944,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.INDX_SHIFT; + 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 6e12887f61b1e..691e49209513f 100644 --- a/packages/core/src/render3/interfaces/node.ts +++ b/packages/core/src/render3/interfaces/node.ts @@ -28,25 +28,17 @@ export const enum LNodeType { } /** - * TNodeFlags corresponds to the TNode.flags property. It contains information - * on how to map a particular set of bits to the node's first directive index - * (with INDX_SHIFT) or the node's directive count (with SIZE_MASK) + * Corresponds to the TNode.flags property. */ export const enum TNodeFlags { - /** Whether or not this node is a component */ - Component = 0b001, + /** The number of directives on this node is encoded on the least significant bits */ + DirectiveCountMask = 0b00000000000000000000111111111111, - /** How far to shift the flags to get the first directive index on this node */ - INDX_SHIFT = 13, + /** Then this bit is set when the node is a component */ + isComponent = 0b1000000000000, - /** How far to shift the flags to get the number of directives on this node */ - SIZE_SHIFT = 1, - - /** The amount to add to flags to increment size when each directive is added */ - SIZE_SKIP = 2, - - /** Mask to get the number of directives on this node */ - SIZE_MASK = 0b00000000000000000001111111111110 + /** The index of the first directive on this node is encoded on the most significant bits */ + DirectiveStartingIndexShift = 13, } /** diff --git a/packages/core/src/render3/interfaces/view.ts b/packages/core/src/render3/interfaces/view.ts index 68bfce83c937c..db3d2b0147552 100644 --- a/packages/core/src/render3/interfaces/view.ts +++ b/packages/core/src/render3/interfaces/view.ts @@ -408,10 +408,10 @@ export type HookData = (number | (() => void))[]; export const enum LifecycleStage { /* Init hooks need to be run, if any. */ - INIT = 1, + Init = 1, /* Content hooks need to be run, if any. Init hooks have already run. */ - AFTER_INIT = 2, + AfterInit = 2, } /** diff --git a/packages/core/src/render3/query.ts b/packages/core/src/render3/query.ts index 413c585cd04b4..e81fcaf1f991c 100644 --- a/packages/core/src/render3/query.ts +++ b/packages/core/src/render3/query.ts @@ -193,13 +193,15 @@ function getIdxOfMatchingSelector(tNode: TNode, selector: string): number|null { * @param type Type of a directive to look for. * @returns Index of a found directive or null when none found. */ -function geIdxOfMatchingDirective(node: LNode, type: Type): number|null { +function getIdxOfMatchingDirective(node: LNode, type: Type): number|null { const defs = node.view.tView.directives !; const flags = node.tNode !.flags; - const size = (flags & TNodeFlags.SIZE_MASK) >> TNodeFlags.SIZE_SHIFT; - for (let i = flags >> TNodeFlags.INDX_SHIFT, ii = i + size; i < ii; i++) { + const count = flags & TNodeFlags.DirectiveCountMask; + const start = flags >> TNodeFlags.DirectiveStartingIndexShift; + const end = start + count; + for (let i = start; i < end; i++) { const def = defs[i] as DirectiveDef; - if (def.diPublic && def.type === type) { + if (def.type === type && def.diPublic) { return i; } } @@ -212,7 +214,7 @@ function readFromNodeInjector( if (read instanceof ReadFromInjectorFn) { return read.read(nodeInjector, node, directiveIdx); } else { - const matchingIdx = geIdxOfMatchingDirective(node, read as Type); + const matchingIdx = getIdxOfMatchingDirective(node, read as Type); if (matchingIdx !== null) { return node.view.directives ![matchingIdx]; } @@ -226,7 +228,7 @@ function add(query: LQuery| null, node: LNode) { const predicate = query.predicate; const type = predicate.type; if (type) { - const directiveIdx = geIdxOfMatchingDirective(node, type); + const directiveIdx = getIdxOfMatchingDirective(node, type); if (directiveIdx !== null) { // a node is matching a predicate - determine what to read // if read token and / or strategy is not specified, use type as read token diff --git a/packages/core/test/render3/di_spec.ts b/packages/core/test/render3/di_spec.ts index 1128e67ece2d9..d7f6e0d49ee72 100644 --- a/packages/core/test/render3/di_spec.ts +++ b/packages/core/test/render3/di_spec.ts @@ -74,7 +74,7 @@ describe('di', () => { static ngDirectiveDef = defineDirective({ type: DirA, selectors: [['', 'dirA', '']], - factory: () => new DirA, + factory: () => new DirA(), features: [PublicFeature] }); }