From 3e03dbe5769391d490c91a0880f92195c0c0e730 Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Tue, 23 Jan 2018 18:39:09 -0800 Subject: [PATCH] refactor(ivy): flatten hooks and collapse LView hook properties (#21650) PR Close #21650 --- integration/_payload-limits.json | 4 +- .../hello_world__render3__rollup/src/index.ts | 1 + packages/core/src/render3/definition.ts | 38 +++----- packages/core/src/render3/hooks.ts | 93 +++++++++---------- packages/core/src/render3/instructions.ts | 21 ++--- .../core/src/render3/interfaces/definition.ts | 25 ++--- packages/core/src/render3/interfaces/view.ts | 39 +++++--- .../test/render3/compiler_canonical_spec.ts | 3 + packages/core/test/render3/content_spec.ts | 2 +- packages/core/test/render3/define_spec.ts | 4 +- packages/core/test/render3/lifecycle_spec.ts | 12 +-- 11 files changed, 113 insertions(+), 129 deletions(-) diff --git a/integration/_payload-limits.json b/integration/_payload-limits.json index 0ab5e1b809..6b879a1a8b 100644 --- a/integration/_payload-limits.json +++ b/integration/_payload-limits.json @@ -18,14 +18,14 @@ "hello_world__render3__closure": { "master": { "uncompressed": { - "bundle": 7065 + "bundle": 7674 } } }, "hello_world__render3__rollup": { "master": { "uncompressed": { - "bundle": 56550 + "bundle": 58662 } } } diff --git a/integration/hello_world__render3__rollup/src/index.ts b/integration/hello_world__render3__rollup/src/index.ts index 42ac9758f3..be662f10d5 100644 --- a/integration/hello_world__render3__rollup/src/index.ts +++ b/integration/hello_world__render3__rollup/src/index.ts @@ -14,6 +14,7 @@ export class HelloWorld { /** @nocollapse */ static ngComponentDef: ComponentDef = defineComponent({ + type: HelloWorld, tag: 'hello-world', template: function (ctx: HelloWorld, cm: boolean) { if (cm) { diff --git a/packages/core/src/render3/definition.ts b/packages/core/src/render3/definition.ts index 65458202a2..528fb5078c 100644 --- a/packages/core/src/render3/definition.ts +++ b/packages/core/src/render3/definition.ts @@ -13,8 +13,7 @@ import {Type} from '../type'; import {resolveRendererType2} from '../view/util'; import {diPublic} from './di'; -import {componentRefresh} from './instructions'; -import {ComponentDef, ComponentDefArgs, DirectiveDef, DirectiveDefArgs, LifecycleHooksMap} from './interfaces/definition'; +import {ComponentDef, ComponentDefArgs, DirectiveDef, DirectiveDefArgs} from './interfaces/definition'; @@ -34,8 +33,9 @@ import {ComponentDef, ComponentDefArgs, DirectiveDef, DirectiveDefArgs, Lifecycl * ``` */ export function defineComponent(componentDefinition: ComponentDefArgs): ComponentDef { + const type = componentDefinition.type; const def = >{ - type: componentDefinition.type, + type: type, diPublic: null, n: componentDefinition.factory, tag: (componentDefinition as ComponentDefArgs).tag || null !, @@ -46,14 +46,16 @@ export function defineComponent(componentDefinition: ComponentDefArgs): Co methods: invertObject(componentDefinition.methods), rendererType: resolveRendererType2(componentDefinition.rendererType) || null, exportAs: componentDefinition.exportAs, - lifecycleHooks: null ! + onInit: type.prototype.ngOnInit || null, + doCheck: type.prototype.ngDoCheck || null, + afterContentInit: type.prototype.ngAfterContentInit || null, + afterContentChecked: type.prototype.ngAfterContentChecked || null, + afterViewInit: type.prototype.ngAfterViewInit || null, + afterViewChecked: type.prototype.ngAfterViewChecked || null, + onDestroy: type.prototype.ngOnDestroy || null }; const feature = componentDefinition.features; feature && feature.forEach((fn) => fn(def)); - - // These must be set after feature functions are run, so ngOnChanges can be - // properly set up. - def.lifecycleHooks = getLifecyleHooksMap(componentDefinition.type); return def; } @@ -97,7 +99,7 @@ export function NgOnChangesFeature(type: Type): (definition: DirectiveDef< } }); } - proto.ngDoCheck = (function(delegateDoCheck) { + definition.doCheck = (function(delegateDoCheck) { return function(this: OnChangesExpando) { let simpleChanges = this[PRIVATE_PREFIX]; if (simpleChanges != null) { @@ -110,24 +112,6 @@ export function NgOnChangesFeature(type: Type): (definition: DirectiveDef< }; } -/** - * Takes the component type and returns a formatted map of lifecycle hooks for that - * component. - * - * @param type The component type - */ -function getLifecyleHooksMap(type: Type): LifecycleHooksMap { - return { - onInit: type.prototype.ngOnInit || null, - doCheck: type.prototype.ngDoCheck || null, - afterContentInit: type.prototype.ngAfterContentInit || null, - afterContentChecked: type.prototype.ngAfterContentChecked || null, - afterViewInit: type.prototype.ngAfterViewInit || null, - afterViewChecked: type.prototype.ngAfterViewChecked || null, - onDestroy: type.prototype.ngOnDestroy || null, - }; -} - export function PublicFeature(definition: DirectiveDef) { definition.diPublic = diPublic; diff --git a/packages/core/src/render3/hooks.ts b/packages/core/src/render3/hooks.ts index 7a9215c518..be76049088 100644 --- a/packages/core/src/render3/hooks.ts +++ b/packages/core/src/render3/hooks.ts @@ -6,9 +6,9 @@ * found in the LICENSE file at https://angular.io/license */ -import {DirectiveDef, LifecycleHooksMap} from './interfaces/definition'; +import {DirectiveDef} from './interfaces/definition'; import {LNodeFlags} from './interfaces/node'; -import {HookData, LView, TView} from './interfaces/view'; +import {HookData, LView, LifecycleStage, TView} from './interfaces/view'; @@ -16,7 +16,6 @@ import {HookData, LView, TView} from './interfaces/view'; export const enum LifecycleHook { ON_INIT = 0b00, ON_CHECK = 0b01, - ON_CHANGES = 0b10, /* Mask used to get the type of the lifecycle hook from flags in hook queue */ TYPE_MASK = 0b00000000000000000000000000000001, @@ -37,14 +36,16 @@ export const enum LifecycleHook { * @param hooks The static hooks map on the directive def * @param tView The current TView */ -export function queueInitHooks(index: number, hooks: LifecycleHooksMap, tView: TView): void { - const firstTemplatePass = tView.firstTemplatePass; - if (firstTemplatePass === true && hooks.onInit != null) { - (tView.initHooks || (tView.initHooks = [])).push(getInitFlags(index), hooks.onInit); - } +export function queueInitHooks( + index: number, onInit: (() => void) | null, doCheck: (() => void) | null, tView: TView): void { + if (tView.firstTemplatePass === true) { + if (onInit != null) { + (tView.initHooks || (tView.initHooks = [])).push(getInitFlags(index), onInit); + } - if (firstTemplatePass === true && hooks.doCheck != null) { - (tView.initHooks || (tView.initHooks = [])).push(getCheckFlags(index), hooks.doCheck); + if (doCheck != null) { + (tView.initHooks || (tView.initHooks = [])).push(getCheckFlags(index), doCheck); + } } } @@ -62,41 +63,41 @@ export function queueLifecycleHooks(flags: number, currentView: LView): void { // 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, end = start + size; i < end; i++) { - const hooks = (tView.data[i] as DirectiveDef).lifecycleHooks; - queueContentHooks(hooks, tView, i); - queueViewHooks(hooks, tView, i); - queueDestroyHooks(hooks, tView, i); + const def = (tView.data[i] as DirectiveDef); + queueContentHooks(def, tView, i); + queueViewHooks(def, tView, i); + queueDestroyHooks(def, tView, i); } } } /** Queues afterContentInit and afterContentChecked hooks on TView */ -function queueContentHooks(hooks: LifecycleHooksMap, tView: TView, i: number): void { - if (hooks.afterContentInit != null) { - (tView.contentHooks || (tView.contentHooks = [])).push(getInitFlags(i), hooks.afterContentInit); +function queueContentHooks(def: DirectiveDef, tView: TView, i: number): void { + if (def.afterContentInit != null) { + (tView.contentHooks || (tView.contentHooks = [])).push(getInitFlags(i), def.afterContentInit); } - if (hooks.afterContentChecked != null) { + if (def.afterContentChecked != null) { (tView.contentHooks || (tView.contentHooks = [ - ])).push(getCheckFlags(i), hooks.afterContentChecked); + ])).push(getCheckFlags(i), def.afterContentChecked); } } /** Queues afterViewInit and afterViewChecked hooks on TView */ -function queueViewHooks(hooks: LifecycleHooksMap, tView: TView, i: number): void { - if (hooks.afterViewInit != null) { - (tView.viewHooks || (tView.viewHooks = [])).push(getInitFlags(i), hooks.afterViewInit); +function queueViewHooks(def: DirectiveDef, tView: TView, i: number): void { + if (def.afterViewInit != null) { + (tView.viewHooks || (tView.viewHooks = [])).push(getInitFlags(i), def.afterViewInit); } - if (hooks.afterViewChecked != null) { - (tView.viewHooks || (tView.viewHooks = [])).push(getCheckFlags(i), hooks.afterViewChecked); + if (def.afterViewChecked != null) { + (tView.viewHooks || (tView.viewHooks = [])).push(getCheckFlags(i), def.afterViewChecked); } } /** Queues onDestroy hooks on TView */ -function queueDestroyHooks(hooks: LifecycleHooksMap, tView: TView, i: number): void { - if (hooks.onDestroy != null) { - (tView.destroyHooks || (tView.destroyHooks = [])).push(i, hooks.onDestroy); +function queueDestroyHooks(def: DirectiveDef, tView: TView, i: number): void { + if (def.onDestroy != null) { + (tView.destroyHooks || (tView.destroyHooks = [])).push(i, def.onDestroy); } } @@ -118,9 +119,24 @@ function getCheckFlags(index: number): number { export function executeInitHooks(currentView: LView): void { const initHooks = currentView.tView.initHooks; - if (currentView.initHooksCalled === false && initHooks != null) { + if (currentView.lifecycleStage === LifecycleStage.INIT && initHooks != null) { executeLifecycleHooks(currentView, initHooks); - currentView.initHooksCalled = true; + currentView.lifecycleStage = LifecycleStage.CONTENT_INIT; + } +} + +/** + * Calls all afterContentInit and afterContentChecked hooks for the view, then splices + * out afterContentInit hooks to prep for the next run in update mode. + * + * @param currentView The current view + */ +export function executeContentHooks(currentView: LView): void { + const contentHooks = currentView.tView.contentHooks; + + if (currentView.lifecycleStage < LifecycleStage.VIEW_INIT && contentHooks != null) { + executeLifecycleHooks(currentView, contentHooks); + currentView.lifecycleStage = LifecycleStage.VIEW_INIT; } } @@ -137,21 +153,6 @@ export function executeViewHooks(currentView: LView): void { } } -/** - * Calls all afterContentInit and afterContentChecked hooks for the view, then splices - * out afterContentInit hooks to prep for the next run in update mode. - * - * @param currentView The current view - */ -export function executeContentHooks(currentView: LView): void { - const contentHooks = currentView.tView.contentHooks; - - if (currentView.contentHooksCalled === false && contentHooks != null) { - executeLifecycleHooks(currentView, contentHooks); - currentView.contentHooksCalled = true; - } -} - /** * Calls lifecycle hooks with their contexts, skipping init hooks if it's not * creation mode. @@ -165,11 +166,9 @@ function executeLifecycleHooks(currentView: LView, arr: HookData): void { for (let i = 0; i < arr.length; i += 2) { const flags = arr[i] as number; - const hook = arr[i | 1] as() => void; const initOnly = (flags & LifecycleHook.TYPE_MASK) === LifecycleHook.ON_INIT; - const instance = data[flags >> LifecycleHook.INDX_SHIFT]; if (initOnly === false || creationMode) { - hook.call(instance); + (arr[i | 1] as() => void).call(data[flags >> LifecycleHook.INDX_SHIFT]); } } } diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 32e45391c3..d4aee504a4 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -18,7 +18,7 @@ import {LContainer, TContainer} from './interfaces/container'; import {LInjector} from './interfaces/injector'; import {CssSelector, LProjection} from './interfaces/projection'; import {LQuery, QueryReadType} from './interfaces/query'; -import {LView, TData, TView} from './interfaces/view'; +import {LView, LifecycleStage, TData, TView} from './interfaces/view'; import {LContainerNode, LElementNode, LNode, LNodeFlags, LProjectionNode, LTextNode, LViewNode, TNode, TContainerNode, InitialInputData, InitialInputs, PropertyAliases, PropertyAliasValue,} from './interfaces/node'; import {assertNodeType, assertNodeOfPossibleTypes} from './node_assert'; @@ -157,8 +157,7 @@ export function enterView(newView: LView, host: LElementNode | LViewNode | null) export function leaveView(newView: LView): void { executeViewHooks(currentView); currentView.creationMode = false; - currentView.initHooksCalled = false; - currentView.contentHooksCalled = false; + currentView.lifecycleStage = LifecycleStage.INIT; currentView.tView.firstTemplatePass = false; enterView(newView, null); } @@ -182,8 +181,7 @@ export function createLView( template: template, context: context, dynamicViewCount: 0, - initHooksCalled: false, - contentHooksCalled: false + lifecycleStage: LifecycleStage.INIT }; return newView; @@ -831,23 +829,22 @@ export function text(index: number, value?: any): void { * @param value Stringified value to write. */ export function textBinding(index: number, value: T | NO_CHANGE): void { - // TODO(misko): I don't think index < nodes.length check is needed here. - let existingNode = index < data.length && data[index] as LTextNode; - if (existingNode && existingNode.native) { + ngDevMode && assertDataInRange(index); + let existingNode = data[index] as LTextNode; + ngDevMode && assertNotNull(existingNode, 'existing node'); + if (existingNode.native) { // If DOM node exists and value changed, update textContent value !== NO_CHANGE && ((renderer as ProceduralRenderer3).setValue ? (renderer as ProceduralRenderer3).setValue(existingNode.native, stringify(value)) : existingNode.native.textContent = stringify(value)); - } else if (existingNode) { + } else { // Node was created but DOM node creation was delayed. Create and append now. existingNode.native = ((renderer as ProceduralRenderer3).createText ? (renderer as ProceduralRenderer3).createText(stringify(value)) : (renderer as ObjectOrientedRenderer3).createTextNode !(stringify(value))); insertChild(existingNode, currentView); - } else { - text(index, value); } } @@ -909,7 +906,7 @@ export function directiveCreate( // Init hooks are queued now so ngOnInit is called in host components before // any projected components. - queueInitHooks(index, directiveDef.lifecycleHooks, currentView.tView); + queueInitHooks(index, directiveDef.onInit, directiveDef.doCheck, currentView.tView); return instance; } diff --git a/packages/core/src/render3/interfaces/definition.ts b/packages/core/src/render3/interfaces/definition.ts index 9d4f666bcf..91b6b43a56 100644 --- a/packages/core/src/render3/interfaces/definition.ts +++ b/packages/core/src/render3/interfaces/definition.ts @@ -71,14 +71,16 @@ export interface DirectiveDef { * Refreshes host bindings on the associated directive. Also calls lifecycle hooks * like ngOnInit and ngDoCheck, if they are defined on the directive. */ - // Note: This call must be separate from r() because hooks like ngOnInit need to - // be called breadth-first across a view before processing onInits in children - // (for backwards compatibility). Child template processing thus needs to be - // delayed until all inputs and host bindings in a view have been checked. h(directiveIndex: number, elementIndex: number): void; - /* A map of the lifecycle hooks defined on this directive (key: name, value: fn) */ - lifecycleHooks: LifecycleHooksMap; + /* The following are lifecycle hooks for this component */ + onInit: (() => void)|null; + doCheck: (() => void)|null; + afterContentInit: (() => void)|null; + afterContentChecked: (() => void)|null; + afterViewInit: (() => void)|null; + afterViewChecked: (() => void)|null; + onDestroy: (() => void)|null; } export interface ComponentDef extends DirectiveDef { @@ -104,17 +106,6 @@ export interface ComponentDef extends DirectiveDef { readonly rendererType: RendererType2|null; } -/* A map of the lifecycle hooks defined on a directive (key: name, value: fn) */ -export interface LifecycleHooksMap { - onInit: () => void | null; - doCheck: () => void | null; - afterContentInit: () => void | null; - afterContentChecked: () => void | null; - afterViewInit: () => void | null; - afterViewChecked: () => void | null; - onDestroy: () => void | null; -} - export interface DirectiveDefArgs { type: Type; factory: () => T; diff --git a/packages/core/src/render3/interfaces/view.ts b/packages/core/src/render3/interfaces/view.ts index 14a24b18af..e0bd995f83 100644 --- a/packages/core/src/render3/interfaces/view.ts +++ b/packages/core/src/render3/interfaces/view.ts @@ -88,24 +88,21 @@ export interface LView { cleanup: any[]|null; /** - * Whether or not the ngOnInit and ngDoCheck hooks have been called in this change - * detection run. + * This number tracks the next lifecycle hook that needs to be run. * - * These two hooks are executed by the first Comp.r() instruction that runs OR the - * first cR() instruction that runs (so inits are run for the top level view before - * any embedded views). For this reason, the call must be tracked. - */ - initHooksCalled: boolean; - - /** - * Whether or not the afterContentInit and afterContentChecked hooks have been called - * in this change detection run. + * If lifecycleStage === LifecycleStage.ON_INIT, the init hooks haven't yet been run + * and should be executed by the first r() instruction that runs OR the first + * cR() instruction that runs (so inits are run for the top level view before any + * embedded views). * - * Content hooks are executed by the first Comp.r() instruction that runs (to avoid - * adding to the code size), so it needs to be able to check whether or not they should - * be called. + * If lifecycleStage === LifecycleStage.CONTENT_INIT, the init hooks have been run, but + * the content hooks have not yet been run. They should be executed on the first + * r() instruction that runs. + * + * If lifecycleStage === LifecycleStage.VIEW_INIT, both the init hooks and content hooks + * have already been run. */ - contentHooksCalled: boolean; + lifecycleStage: LifecycleStage; /** * The first LView or LContainer beneath this LView in the hierarchy. @@ -239,6 +236,18 @@ export interface TView { */ export type HookData = (number | (() => void))[]; +/** Possible values of LView.lifecycleStage, used to determine which hooks to run. */ +export const enum LifecycleStage { + /* Init hooks need to be run, if any. */ + INIT = 1, + + /* Content hooks need to be run, if any. Init hooks have already run. */ + CONTENT_INIT = 2, + + /* View hooks need to be run, if any. Any init hooks/content hooks have ran. */ + VIEW_INIT = 3 +} + /** * Static data that corresponds to the instance-specific data array on an LView. * diff --git a/packages/core/test/render3/compiler_canonical_spec.ts b/packages/core/test/render3/compiler_canonical_spec.ts index 65d77d5f20..264fcbc8dc 100644 --- a/packages/core/test/render3/compiler_canonical_spec.ts +++ b/packages/core/test/render3/compiler_canonical_spec.ts @@ -175,6 +175,7 @@ describe('compiler specification', () => { @Component({selector: 'simple', template: `
`}) class SimpleComponent { static ngComponentDef = r3.defineComponent({ + type: SimpleComponent, tag: 'simple', factory: () => new SimpleComponent(), template: function(ctx: SimpleComponent, cm: boolean) { @@ -196,6 +197,7 @@ describe('compiler specification', () => { }) class ComplexComponent { static ngComponentDef = r3.defineComponent({ + type: ComplexComponent, tag: 'complex', factory: () => new ComplexComponent(), template: function(ctx: ComplexComponent, cm: boolean) { @@ -221,6 +223,7 @@ describe('compiler specification', () => { }) class MyApp { static ngComponentDef = r3.defineComponent({ + type: MyApp, tag: 'my-app', factory: () => new MyApp(), template: function(ctx: MyApp, cm: boolean) { diff --git a/packages/core/test/render3/content_spec.ts b/packages/core/test/render3/content_spec.ts index 322eaa37c5..7fa6a7edbf 100644 --- a/packages/core/test/render3/content_spec.ts +++ b/packages/core/test/render3/content_spec.ts @@ -104,7 +104,7 @@ describe('content projection', () => { /**
*/ const Child = createComponent('child', (ctx: any, cm: boolean) => { if (cm) { - m(0, pD()); + pD(0); E(1, 'div'); { P(2, 0); } e(); diff --git a/packages/core/test/render3/define_spec.ts b/packages/core/test/render3/define_spec.ts index d27483bed4..f8f48006ac 100644 --- a/packages/core/test/render3/define_spec.ts +++ b/packages/core/test/render3/define_spec.ts @@ -42,11 +42,11 @@ describe('define', () => { expect(myDir.log).toEqual(['second']); expect(myDir.valB).toEqual('works'); myDir.log.length = 0; - myDir.ngDoCheck(); + MyDirective.ngDirectiveDef.doCheck !.call(myDir); expect(myDir.log).toEqual([ 'ngOnChanges', 'valA', 'initValue', 'first', 'valB', undefined, 'second', 'ngDoCheck' ]); }); }); }); -}); \ No newline at end of file +}); diff --git a/packages/core/test/render3/lifecycle_spec.ts b/packages/core/test/render3/lifecycle_spec.ts index 927b7d7038..ce5085fdd2 100644 --- a/packages/core/test/render3/lifecycle_spec.ts +++ b/packages/core/test/render3/lifecycle_spec.ts @@ -31,7 +31,7 @@ describe('lifecycles', () => { let Comp = createOnInitComponent('comp', (ctx: any, cm: boolean) => { if (cm) { - m(0, pD()); + pD(0); E(1, 'div'); { P(2, 0); } e(); @@ -435,14 +435,14 @@ describe('lifecycles', () => { let Comp = createAfterContentInitComp('comp', function(ctx: any, cm: boolean) { if (cm) { - m(0, pD()); + pD(0); P(1, 0); } }); let Parent = createAfterContentInitComp('parent', function(ctx: any, cm: boolean) { if (cm) { - m(0, pD()); + pD(0); E(1, Comp); { P(3, 0); } e(); @@ -454,7 +454,7 @@ describe('lifecycles', () => { let ProjectedComp = createAfterContentInitComp('projected', (ctx: any, cm: boolean) => { if (cm) { - m(0, pD()); + pD(0); P(1, 0); } }); @@ -813,7 +813,7 @@ describe('lifecycles', () => { let Comp = createAfterViewInitComponent('comp', (ctx: any, cm: boolean) => { if (cm) { - m(0, pD()); + pD(0); E(1, 'div'); { P(2, 0); } e(); @@ -1244,7 +1244,7 @@ describe('lifecycles', () => { let Comp = createOnDestroyComponent('comp', (ctx: any, cm: boolean) => { if (cm) { - m(0, pD()); + pD(0); P(1, 0); } });