From 269c093bd86528cdc2e31119bec065b58306dd47 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Fri, 14 Feb 2020 18:02:11 -0800 Subject: [PATCH] fix(core): Allow modification of lifecycle hooks any time before bootstrap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently we read lifecycle hooks eagerly during `ɵɵdefineComponent`. The result is that it is not possible to do any sort of meta-programing such as mixins or adding lifecycle hooks using custom decorators since any such code executes after `ɵɵdefineComponent` has extracted the lifecycle hooks from the prototype. Additionally the behavior is inconsistent between AOT and JIT mode. In JIT mode overriding lifecycle hooks is possible because the whole `ɵɵdefineComponent` is placed in getter which is executed lazily. This is because JIT mode must compile a template which can be specified as `templateURL` and those we are waiting for its resolution. - `+` `ɵɵdefineComponent` becomes smaller as it no longer needs to copy lifecycle hooks from prototype to `ComponentDef` - `-` `ɵɵNgOnChangesFeature` feature is now always included with the codebase as it is no longer tree shakable. Previously we have read lifecycle hooks from prototype in the `ɵɵdefineComponent` so that lifecycle hook access would be monomorphic. This decision was made before we had `T*` data structures. By not reading the lifecycle hooks we are moving the megamorhic read form `ɵɵdefineComponent` to instructions. However, the reads happen on `firstTemplatePass` only and are subsequently cached in the `T*` data structures. The result is that the overall performance should be same (or slightly better as the intermediate `ComponentDef` has been removed.) - [ ] Remove `ɵɵNgOnChangesFeature` from compiler. (It will no longer be a feature.) - [ ] Discuss the future of `Features` as they hinder meta-programing. Fix #30497 --- .../aio-payloads_20200714135705.json | 34 +++++ .../aio-payloads_20200714135739.json | 29 ++++ .../aio-payloads_20200714135740.json | 29 ++++ .../aio-payloads_20200714142715.json | 29 ++++ .../integration-payloads_20200714135705.json | 73 ++++++++++ .../integration-payloads_20200714135744.json | 69 ++++++++++ .../integration-payloads_20200714142506.json | 69 ++++++++++ .../integration-payloads_20200714142544.json | 69 ++++++++++ goldens/public-api/core/core.d.ts | 4 +- goldens/size-tracking/aio-payloads.json | 4 +- .../size-tracking/integration-payloads.json | 12 +- packages/core/src/render3/definition.ts | 125 +++++++++--------- .../features/inherit_definition_feature.ts | 11 -- .../render3/features/ng_onchanges_feature.ts | 62 +++++---- packages/core/src/render3/hooks.ts | 59 +++++---- .../core/src/render3/instructions/shared.ts | 7 +- .../core/src/render3/interfaces/definition.ts | 10 -- .../core/test/acceptance/lifecycle_spec.ts | 40 ++++++ .../cyclic_import/bundle.golden_symbols.json | 9 ++ .../hello_world/bundle.golden_symbols.json | 9 ++ .../bundling/todo/bundle.golden_symbols.json | 9 ++ 21 files changed, 613 insertions(+), 149 deletions(-) create mode 100644 .history/goldens/size-tracking/aio-payloads_20200714135705.json create mode 100644 .history/goldens/size-tracking/aio-payloads_20200714135739.json create mode 100644 .history/goldens/size-tracking/aio-payloads_20200714135740.json create mode 100644 .history/goldens/size-tracking/aio-payloads_20200714142715.json create mode 100644 .history/goldens/size-tracking/integration-payloads_20200714135705.json create mode 100644 .history/goldens/size-tracking/integration-payloads_20200714135744.json create mode 100644 .history/goldens/size-tracking/integration-payloads_20200714142506.json create mode 100644 .history/goldens/size-tracking/integration-payloads_20200714142544.json diff --git a/.history/goldens/size-tracking/aio-payloads_20200714135705.json b/.history/goldens/size-tracking/aio-payloads_20200714135705.json new file mode 100644 index 00000000000000..ca8aa6658416c2 --- /dev/null +++ b/.history/goldens/size-tracking/aio-payloads_20200714135705.json @@ -0,0 +1,34 @@ +{ + "aio": { + "master": { + "uncompressed": { + "runtime-es2015": 2987, + "main-es2015": 450880, + "polyfills-es2015": 52685 + } + } + }, + "aio-local": { + "master": { + "uncompressed": { + "runtime-es2015": 2987, +<<<<<<< HEAD + "main-es2015": 450883, + "polyfills-es2015": 52630 +======= + "main-es2015": 450966, + "polyfills-es2015": 52655 +>>>>>>> e3af65ede0... fix(core): Allow modification of lifecycle hooks any time before bootstrap + } + } + }, + "aio-local-viewengine": { + "master": { + "uncompressed": { + "runtime-es2015": 3097, + "main-es2015": 429200, + "polyfills-es2015": 52195 + } + } + } +} \ No newline at end of file diff --git a/.history/goldens/size-tracking/aio-payloads_20200714135739.json b/.history/goldens/size-tracking/aio-payloads_20200714135739.json new file mode 100644 index 00000000000000..0c9d5bc580c4a4 --- /dev/null +++ b/.history/goldens/size-tracking/aio-payloads_20200714135739.json @@ -0,0 +1,29 @@ +{ + "aio": { + "master": { + "uncompressed": { + "runtime-es2015": 2987, + "main-es2015": 450880, + "polyfills-es2015": 52685 + } + } + }, + "aio-local": { + "master": { + "uncompressed": { + "runtime-es2015": 2987, + "main-es2015": 450883, + "polyfills-es2015": 52630 + } + } + }, + "aio-local-viewengine": { + "master": { + "uncompressed": { + "runtime-es2015": 3097, + "main-es2015": 429200, + "polyfills-es2015": 52195 + } + } + } +} \ No newline at end of file diff --git a/.history/goldens/size-tracking/aio-payloads_20200714135740.json b/.history/goldens/size-tracking/aio-payloads_20200714135740.json new file mode 100644 index 00000000000000..0c9d5bc580c4a4 --- /dev/null +++ b/.history/goldens/size-tracking/aio-payloads_20200714135740.json @@ -0,0 +1,29 @@ +{ + "aio": { + "master": { + "uncompressed": { + "runtime-es2015": 2987, + "main-es2015": 450880, + "polyfills-es2015": 52685 + } + } + }, + "aio-local": { + "master": { + "uncompressed": { + "runtime-es2015": 2987, + "main-es2015": 450883, + "polyfills-es2015": 52630 + } + } + }, + "aio-local-viewengine": { + "master": { + "uncompressed": { + "runtime-es2015": 3097, + "main-es2015": 429200, + "polyfills-es2015": 52195 + } + } + } +} \ No newline at end of file diff --git a/.history/goldens/size-tracking/aio-payloads_20200714142715.json b/.history/goldens/size-tracking/aio-payloads_20200714142715.json new file mode 100644 index 00000000000000..4dbcab337436e4 --- /dev/null +++ b/.history/goldens/size-tracking/aio-payloads_20200714142715.json @@ -0,0 +1,29 @@ +{ + "aio": { + "master": { + "uncompressed": { + "runtime-es2015": 2987, + "main-es2015": 450880, + "polyfills-es2015": 52685 + } + } + }, + "aio-local": { + "master": { + "uncompressed": { + "runtime-es2015": 2987, + "main-es2015": 450301, + "polyfills-es2015": 52630 + } + } + }, + "aio-local-viewengine": { + "master": { + "uncompressed": { + "runtime-es2015": 3097, + "main-es2015": 429200, + "polyfills-es2015": 52195 + } + } + } +} \ No newline at end of file diff --git a/.history/goldens/size-tracking/integration-payloads_20200714135705.json b/.history/goldens/size-tracking/integration-payloads_20200714135705.json new file mode 100644 index 00000000000000..e5672464f82465 --- /dev/null +++ b/.history/goldens/size-tracking/integration-payloads_20200714135705.json @@ -0,0 +1,73 @@ +{ + "cli-hello-world": { + "master": { + "uncompressed": { + "runtime-es2015": 1485, + "main-es2015": 141151, + "polyfills-es2015": 36571 + } + } + }, + "cli-hello-world-ivy-minimal": { + "master": { + "uncompressed": { + "runtime-es2015": 1485, + "main-es2015": 17362, + "polyfills-es2015": 36657 + } + } + }, + "cli-hello-world-ivy-compat": { + "master": { + "uncompressed": { + "runtime-es2015": 1485, + "main-es2015": 147573, + "polyfills-es2015": 36571 + } + } + }, + "cli-hello-world-ivy-i18n": { + "master": { + "uncompressed": { + "runtime-es2015": 1485, + "main-es2015": 135533, + "polyfills-es2015": 37248 + } + } + }, + "cli-hello-world-lazy": { + "master": { + "uncompressed": { + "runtime-es2015": 2289, + "main-es2015": 246044, + "polyfills-es2015": 36938, + "5-es2015": 751 + } + } + }, + "cli-hello-world-lazy-rollup": { + "master": { + "uncompressed": { + "runtime-es2015": 2289, +<<<<<<< HEAD + "main-es2015": 221897, +======= + "main-es2015": 221846, +>>>>>>> e3af65ede0... fix(core): Allow modification of lifecycle hooks any time before bootstrap + "polyfills-es2015": 36938, + "5-es2015": 779 + } + } + }, + "hello_world__closure": { + "master": { + "uncompressed": { + "bundle": "TODO(i): temporarily increase the payload size limit from 105779 - this is due to a closure issue related to ESM reexports that still needs to be investigated", + "bundle": "TODO(i): we should define ngDevMode to false in Closure, but --define only works in the global scope.", + "bundle": "TODO(i): (FW-2164) TS 3.9 new class shape seems to have broken Closure in big ways. The size went from 169991 to 252338", + "bundle": "TODO(i): after removal of tsickle from ngc-wrapped / ng_package, we had to switch to SIMPLE optimizations which increased the size from 252338 to 1198917, see PR#37221 and PR#37317 for more info", + "bundle": 1209659 + } + } + } +} \ No newline at end of file diff --git a/.history/goldens/size-tracking/integration-payloads_20200714135744.json b/.history/goldens/size-tracking/integration-payloads_20200714135744.json new file mode 100644 index 00000000000000..bffe464ebbf8ad --- /dev/null +++ b/.history/goldens/size-tracking/integration-payloads_20200714135744.json @@ -0,0 +1,69 @@ +{ + "cli-hello-world": { + "master": { + "uncompressed": { + "runtime-es2015": 1485, + "main-es2015": 141151, + "polyfills-es2015": 36571 + } + } + }, + "cli-hello-world-ivy-minimal": { + "master": { + "uncompressed": { + "runtime-es2015": 1485, + "main-es2015": 17362, + "polyfills-es2015": 36657 + } + } + }, + "cli-hello-world-ivy-compat": { + "master": { + "uncompressed": { + "runtime-es2015": 1485, + "main-es2015": 147573, + "polyfills-es2015": 36571 + } + } + }, + "cli-hello-world-ivy-i18n": { + "master": { + "uncompressed": { + "runtime-es2015": 1485, + "main-es2015": 135533, + "polyfills-es2015": 37248 + } + } + }, + "cli-hello-world-lazy": { + "master": { + "uncompressed": { + "runtime-es2015": 2289, + "main-es2015": 246044, + "polyfills-es2015": 36938, + "5-es2015": 751 + } + } + }, + "cli-hello-world-lazy-rollup": { + "master": { + "uncompressed": { + "runtime-es2015": 2289, + "main-es2015": 221897, + "polyfills-es2015": 36938, + "5-es2015": 779 + } + } + }, + "hello_world__closure": { + "master": { + "uncompressed": { + "bundle": "TODO(i): temporarily increase the payload size limit from 105779 - this is due to a closure issue related to ESM reexports that still needs to be investigated", + "bundle": "TODO(i): we should define ngDevMode to false in Closure, but --define only works in the global scope.", + "bundle": "TODO(i): (FW-2164) TS 3.9 new class shape seems to have broken Closure in big ways. The size went from 169991 to 252338", + "bundle": "TODO(i): after removal of tsickle from ngc-wrapped / ng_package, we had to switch to SIMPLE optimizations which increased the size from 252338 to 1198917, see PR#37221 and PR#37317 for more info", + "bundle": 1209659 + } + } + } +} \ No newline at end of file diff --git a/.history/goldens/size-tracking/integration-payloads_20200714142506.json b/.history/goldens/size-tracking/integration-payloads_20200714142506.json new file mode 100644 index 00000000000000..6535e3d4b61182 --- /dev/null +++ b/.history/goldens/size-tracking/integration-payloads_20200714142506.json @@ -0,0 +1,69 @@ +{ + "cli-hello-world": { + "master": { + "uncompressed": { + "runtime-es2015": 1485, + "main-es2015": 141151, + "polyfills-es2015": 36571 + } + } + }, + "cli-hello-world-ivy-minimal": { + "master": { + "uncompressed": { + "runtime-es2015": 1485, + "main-es2015": 17362, + "polyfills-es2015": 36657 + } + } + }, + "cli-hello-world-ivy-compat": { + "master": { + "uncompressed": { + "runtime-es2015": 1485, + "main-es2015": 147573, + "polyfills-es2015": 36571 + } + } + }, + "cli-hello-world-ivy-i18n": { + "master": { + "uncompressed": { + "runtime-es2015": 1485, + "main-es2015": 136168, + "polyfills-es2015": 37248 + } + } + }, + "cli-hello-world-lazy": { + "master": { + "uncompressed": { + "runtime-es2015": 2289, + "main-es2015": 245351, + "polyfills-es2015": 36938, + "5-es2015": 751 + } + } + }, + "cli-hello-world-lazy-rollup": { + "master": { + "uncompressed": { + "runtime-es2015": 2289, + "main-es2015": 221897, + "polyfills-es2015": 36938, + "5-es2015": 779 + } + } + }, + "hello_world__closure": { + "master": { + "uncompressed": { + "bundle": "TODO(i): temporarily increase the payload size limit from 105779 - this is due to a closure issue related to ESM reexports that still needs to be investigated", + "bundle": "TODO(i): we should define ngDevMode to false in Closure, but --define only works in the global scope.", + "bundle": "TODO(i): (FW-2164) TS 3.9 new class shape seems to have broken Closure in big ways. The size went from 169991 to 252338", + "bundle": "TODO(i): after removal of tsickle from ngc-wrapped / ng_package, we had to switch to SIMPLE optimizations which increased the size from 252338 to 1198917, see PR#37221 and PR#37317 for more info", + "bundle": 1209659 + } + } + } +} \ No newline at end of file diff --git a/.history/goldens/size-tracking/integration-payloads_20200714142544.json b/.history/goldens/size-tracking/integration-payloads_20200714142544.json new file mode 100644 index 00000000000000..6535e3d4b61182 --- /dev/null +++ b/.history/goldens/size-tracking/integration-payloads_20200714142544.json @@ -0,0 +1,69 @@ +{ + "cli-hello-world": { + "master": { + "uncompressed": { + "runtime-es2015": 1485, + "main-es2015": 141151, + "polyfills-es2015": 36571 + } + } + }, + "cli-hello-world-ivy-minimal": { + "master": { + "uncompressed": { + "runtime-es2015": 1485, + "main-es2015": 17362, + "polyfills-es2015": 36657 + } + } + }, + "cli-hello-world-ivy-compat": { + "master": { + "uncompressed": { + "runtime-es2015": 1485, + "main-es2015": 147573, + "polyfills-es2015": 36571 + } + } + }, + "cli-hello-world-ivy-i18n": { + "master": { + "uncompressed": { + "runtime-es2015": 1485, + "main-es2015": 136168, + "polyfills-es2015": 37248 + } + } + }, + "cli-hello-world-lazy": { + "master": { + "uncompressed": { + "runtime-es2015": 2289, + "main-es2015": 245351, + "polyfills-es2015": 36938, + "5-es2015": 751 + } + } + }, + "cli-hello-world-lazy-rollup": { + "master": { + "uncompressed": { + "runtime-es2015": 2289, + "main-es2015": 221897, + "polyfills-es2015": 36938, + "5-es2015": 779 + } + } + }, + "hello_world__closure": { + "master": { + "uncompressed": { + "bundle": "TODO(i): temporarily increase the payload size limit from 105779 - this is due to a closure issue related to ESM reexports that still needs to be investigated", + "bundle": "TODO(i): we should define ngDevMode to false in Closure, but --define only works in the global scope.", + "bundle": "TODO(i): (FW-2164) TS 3.9 new class shape seems to have broken Closure in big ways. The size went from 169991 to 252338", + "bundle": "TODO(i): after removal of tsickle from ngc-wrapped / ng_package, we had to switch to SIMPLE optimizations which increased the size from 252338 to 1198917, see PR#37221 and PR#37317 for more info", + "bundle": 1209659 + } + } + } +} \ No newline at end of file diff --git a/goldens/public-api/core/core.d.ts b/goldens/public-api/core/core.d.ts index 48bb530ff1b72f..dfb7c9cdf3aa53 100644 --- a/goldens/public-api/core/core.d.ts +++ b/goldens/public-api/core/core.d.ts @@ -894,7 +894,7 @@ export declare function ɵɵnextContext(level?: number): T; export declare type ɵɵNgModuleDefWithMeta = ɵNgModuleDef; -export declare function ɵɵNgOnChangesFeature(definition: ɵDirectiveDef): void; +export declare function ɵɵNgOnChangesFeature(): DirectiveDefFeature; export declare function ɵɵpipe(index: number, pipeName: string): any; @@ -1056,7 +1056,7 @@ export declare function ɵɵstylePropInterpolateV(prop: string, values: any[], v export declare function ɵɵtemplate(index: number, templateFn: ComponentTemplate | null, decls: number, vars: number, tagName?: string | null, attrsIndex?: number | null, localRefsIndex?: number | null, localRefExtractor?: LocalRefExtractor): void; -export declare function ɵɵtemplateRefExtractor(tNode: TNode, currentView: ɵangular_packages_core_core_bo): TemplateRef | null; +export declare function ɵɵtemplateRefExtractor(tNode: TNode, currentView: ɵangular_packages_core_core_bp): TemplateRef | null; export declare function ɵɵtext(index: number, value?: string): void; diff --git a/goldens/size-tracking/aio-payloads.json b/goldens/size-tracking/aio-payloads.json index b382fea53aadf1..4dbcab337436e4 100755 --- a/goldens/size-tracking/aio-payloads.json +++ b/goldens/size-tracking/aio-payloads.json @@ -12,7 +12,7 @@ "master": { "uncompressed": { "runtime-es2015": 2987, - "main-es2015": 450883, + "main-es2015": 450301, "polyfills-es2015": 52630 } } @@ -26,4 +26,4 @@ } } } -} +} \ No newline at end of file diff --git a/goldens/size-tracking/integration-payloads.json b/goldens/size-tracking/integration-payloads.json index 93c9f66230a601..6535e3d4b61182 100644 --- a/goldens/size-tracking/integration-payloads.json +++ b/goldens/size-tracking/integration-payloads.json @@ -12,8 +12,8 @@ "master": { "uncompressed": { "runtime-es2015": 1485, - "main-es2015": 16959, - "polyfills-es2015": 36938 + "main-es2015": 17362, + "polyfills-es2015": 36657 } } }, @@ -21,7 +21,7 @@ "master": { "uncompressed": { "runtime-es2015": 1485, - "main-es2015": 147314, + "main-es2015": 147573, "polyfills-es2015": 36571 } } @@ -30,7 +30,7 @@ "master": { "uncompressed": { "runtime-es2015": 1485, - "main-es2015": 135533, + "main-es2015": 136168, "polyfills-es2015": 37248 } } @@ -39,7 +39,7 @@ "master": { "uncompressed": { "runtime-es2015": 2289, - "main-es2015": 246044, + "main-es2015": 245351, "polyfills-es2015": 36938, "5-es2015": 751 } @@ -66,4 +66,4 @@ } } } -} +} \ No newline at end of file diff --git a/packages/core/src/render3/definition.ts b/packages/core/src/render3/definition.ts index 829e04a999e7f8..74fa4237b6e917 100644 --- a/packages/core/src/render3/definition.ts +++ b/packages/core/src/render3/definition.ts @@ -290,72 +290,65 @@ export function ɵɵdefineComponent(componentDefinition: { schemas?: SchemaMetadata[] | null; }): never { return noSideEffects(() => { - // Initialize ngDevMode. This must be the first statement in ɵɵdefineComponent. - // See the `initNgDevMode` docstring for more information. - (typeof ngDevMode === 'undefined' || ngDevMode) && initNgDevMode(); - - const type = componentDefinition.type; - const typePrototype = type.prototype; - const declaredInputs: {[key: string]: string} = {} as any; - const def: Mutable, keyof ComponentDef> = { - type: type, - providersResolver: null, - decls: componentDefinition.decls, - vars: componentDefinition.vars, - factory: null, - template: componentDefinition.template || null!, - consts: componentDefinition.consts || null, - ngContentSelectors: componentDefinition.ngContentSelectors, - hostBindings: componentDefinition.hostBindings || null, - hostVars: componentDefinition.hostVars || 0, - hostAttrs: componentDefinition.hostAttrs || null, - contentQueries: componentDefinition.contentQueries || null, - declaredInputs: declaredInputs, - inputs: null!, // assigned in noSideEffects - outputs: null!, // assigned in noSideEffects - exportAs: componentDefinition.exportAs || null, - onChanges: null, - onInit: typePrototype.ngOnInit || null, - doCheck: typePrototype.ngDoCheck || null, - afterContentInit: typePrototype.ngAfterContentInit || null, - afterContentChecked: typePrototype.ngAfterContentChecked || null, - afterViewInit: typePrototype.ngAfterViewInit || null, - afterViewChecked: typePrototype.ngAfterViewChecked || null, - onDestroy: typePrototype.ngOnDestroy || null, - onPush: componentDefinition.changeDetection === ChangeDetectionStrategy.OnPush, - directiveDefs: null!, // assigned in noSideEffects - pipeDefs: null!, // assigned in noSideEffects - selectors: componentDefinition.selectors || EMPTY_ARRAY, - viewQuery: componentDefinition.viewQuery || null, - features: componentDefinition.features as DirectiveDefFeature[] || null, - data: componentDefinition.data || {}, - // TODO(misko): convert ViewEncapsulation into const enum so that it can be used directly in - // the next line. Also `None` should be 0 not 2. - encapsulation: componentDefinition.encapsulation || ViewEncapsulation.Emulated, - id: 'c', - styles: componentDefinition.styles || EMPTY_ARRAY, - _: null as never, - setInput: null, - schemas: componentDefinition.schemas || null, - tView: null, - }; - const directiveTypes = componentDefinition.directives!; - const feature = componentDefinition.features; - const pipeTypes = componentDefinition.pipes!; - def.id += _renderCompCount++; - def.inputs = invertObject(componentDefinition.inputs, declaredInputs), - def.outputs = invertObject(componentDefinition.outputs), - feature && feature.forEach((fn) => fn(def)); - def.directiveDefs = directiveTypes ? - () => (typeof directiveTypes === 'function' ? directiveTypes() : directiveTypes) - .map(extractDirectiveDef) : - null; - def.pipeDefs = pipeTypes ? - () => (typeof pipeTypes === 'function' ? pipeTypes() : pipeTypes).map(extractPipeDef) : - null; - - return def as never; - }); + // Initialize ngDevMode. This must be the first statement in ɵɵdefineComponent. + // See the `initNgDevMode` docstring for more information. + (typeof ngDevMode === 'undefined' || ngDevMode) && initNgDevMode(); + + const type = componentDefinition.type; + const typePrototype = type.prototype; + const declaredInputs: {[key: string]: string} = {} as any; + const def: Mutable, keyof ComponentDef> = { + type: type, + providersResolver: null, + decls: componentDefinition.decls, + vars: componentDefinition.vars, + factory: null, + template: componentDefinition.template || null!, + consts: componentDefinition.consts || null, + ngContentSelectors: componentDefinition.ngContentSelectors, + hostBindings: componentDefinition.hostBindings || null, + hostVars: componentDefinition.hostVars || 0, + hostAttrs: componentDefinition.hostAttrs || null, + contentQueries: componentDefinition.contentQueries || null, + declaredInputs: declaredInputs, + inputs: null!, // assigned in noSideEffects + outputs: null!, // assigned in noSideEffects + exportAs: componentDefinition.exportAs || null, + onPush: componentDefinition.changeDetection === ChangeDetectionStrategy.OnPush, + directiveDefs: null!, // assigned in noSideEffects + pipeDefs: null!, // assigned in noSideEffects + selectors: componentDefinition.selectors || EMPTY_ARRAY, + viewQuery: componentDefinition.viewQuery || null, + features: componentDefinition.features as DirectiveDefFeature[] || null, + data: componentDefinition.data || {}, + // TODO(misko): convert ViewEncapsulation into const enum so that it can be used + // directly in the next line. Also `None` should be 0 not 2. + encapsulation: componentDefinition.encapsulation || ViewEncapsulation.Emulated, + id: 'c', + styles: componentDefinition.styles || EMPTY_ARRAY, + _: null as never, + setInput: null, + schemas: componentDefinition.schemas || null, + tView: null, + }; + const directiveTypes = componentDefinition.directives!; + const feature = componentDefinition.features; + const pipeTypes = componentDefinition.pipes!; + def.id += _renderCompCount++; + def.inputs = invertObject(componentDefinition.inputs, declaredInputs), + def.outputs = invertObject(componentDefinition.outputs), + feature && feature.forEach((fn) => fn(def)); + def.directiveDefs = directiveTypes ? + () => (typeof directiveTypes === 'function' ? directiveTypes() : directiveTypes) + .map(extractDirectiveDef) : + null; + def.pipeDefs = pipeTypes ? + () => + (typeof pipeTypes === 'function' ? pipeTypes() : pipeTypes).map(extractPipeDef) : + null; + + return def as never; + }) as never; } /** diff --git a/packages/core/src/render3/features/inherit_definition_feature.ts b/packages/core/src/render3/features/inherit_definition_feature.ts index 5bdc85dae68648..891a2b9a922ff6 100644 --- a/packages/core/src/render3/features/inherit_definition_feature.ts +++ b/packages/core/src/render3/features/inherit_definition_feature.ts @@ -78,17 +78,6 @@ export function ɵɵInheritDefinitionFeature(definition: DirectiveDef|Compo const defData = (definition as ComponentDef).data; defData.animation = (defData.animation || []).concat(superDef.data.animation); } - - // Inherit hooks - // Assume super class inheritance feature has already run. - writeableDef.afterContentChecked = - writeableDef.afterContentChecked || superDef.afterContentChecked; - writeableDef.afterContentInit = definition.afterContentInit || superDef.afterContentInit; - writeableDef.afterViewChecked = definition.afterViewChecked || superDef.afterViewChecked; - writeableDef.afterViewInit = definition.afterViewInit || superDef.afterViewInit; - writeableDef.doCheck = definition.doCheck || superDef.doCheck; - writeableDef.onDestroy = definition.onDestroy || superDef.onDestroy; - writeableDef.onInit = definition.onInit || superDef.onInit; } // Run parent features diff --git a/packages/core/src/render3/features/ng_onchanges_feature.ts b/packages/core/src/render3/features/ng_onchanges_feature.ts index fcb209b50248bc..a4b82c0df8cf98 100644 --- a/packages/core/src/render3/features/ng_onchanges_feature.ts +++ b/packages/core/src/render3/features/ng_onchanges_feature.ts @@ -11,14 +11,6 @@ import {SimpleChange, SimpleChanges} from '../../interface/simple_change'; import {EMPTY_OBJ} from '../empty'; import {DirectiveDef, DirectiveDefFeature} from '../interfaces/definition'; -const PRIVATE_PREFIX = '__ngOnChanges_'; - -type OnChangesExpando = OnChanges&{ - __ngOnChanges_: SimpleChanges|null|undefined; - // tslint:disable-next-line:no-any Can hold any value - [key: string]: any; -}; - /** * The NgOnChangesFeature decorates a component with support for the ngOnChanges * lifecycle hook, so it should be included in any component that implements @@ -41,12 +33,15 @@ type OnChangesExpando = OnChanges&{ * * @codeGenApi */ +export function ɵɵNgOnChangesFeature(): DirectiveDefFeature { + return NgOnChangesFeatureImpl; +} -export function ɵɵNgOnChangesFeature(definition: DirectiveDef): void { +export function NgOnChangesFeatureImpl(definition: DirectiveDef) { if (definition.type.prototype.ngOnChanges) { definition.setInput = ngOnChangesSetInput; - (definition as {onChanges: Function}).onChanges = wrapOnChanges(); } + return rememberChangeHistoryAndInvokeOnChangesHook; } // This option ensures that the ngOnChanges lifecycle hook will be inherited @@ -55,28 +50,37 @@ export function ɵɵNgOnChangesFeature(definition: DirectiveDef): void { // tslint:disable-next-line:no-toplevel-property-access (ɵɵNgOnChangesFeature as DirectiveDefFeature).ngInherit = true; -function wrapOnChanges() { - return function wrapOnChangesHook_inPreviousChangesStorage(this: OnChanges) { - const simpleChangesStore = getSimpleChangesStore(this); - const current = simpleChangesStore && simpleChangesStore.current; +/** + * This is a synthetic lifecycle hook which gets inserted into `TView.preOrderHooks` to simulate + * `ngOnChanges`. + * + * The hook reads the `NgSimpleChangesStore` data from the component instance and if changes are + * found it invokes `ngOnChanges` on the component instance. + * + * @param this Component instance. Because this function gets inserted into `TView.preOrderHooks`, + * it is guaranteed to be called with component instance. + */ +function rememberChangeHistoryAndInvokeOnChangesHook(this: OnChanges) { + const simpleChangesStore = getSimpleChangesStore(this); + const current = simpleChangesStore?.current; - if (current) { - const previous = simpleChangesStore!.previous; - if (previous === EMPTY_OBJ) { - simpleChangesStore!.previous = current; - } else { - // New changes are copied to the previous store, so that we don't lose history for inputs - // which were not changed this time - for (let key in current) { - previous[key] = current[key]; - } + if (current) { + const previous = simpleChangesStore!.previous; + if (previous === EMPTY_OBJ) { + simpleChangesStore!.previous = current; + } else { + // New changes are copied to the previous store, so that we don't lose history for inputs + // which were not changed this time + for (let key in current) { + previous[key] = current[key]; } - simpleChangesStore!.current = null; - this.ngOnChanges(current); } - }; + simpleChangesStore!.current = null; + this.ngOnChanges(current); + } } + function ngOnChangesSetInput( this: DirectiveDef, instance: T, value: any, publicName: string, privateName: string): void { const simpleChangesStore = getSimpleChangesStore(instance) || @@ -102,6 +106,10 @@ function setSimpleChangesStore(instance: any, store: NgSimpleChangesStore): NgSi return instance[SIMPLE_CHANGES_STORE] = store; } +/** + * Data structure which is monkey-patched on the component instance and used by `ngOnChanges` + * life-cycle hook to track previous input values. + */ interface NgSimpleChangesStore { previous: SimpleChanges; current: SimpleChanges|null; diff --git a/packages/core/src/render3/hooks.ts b/packages/core/src/render3/hooks.ts index b1ca90e5d42158..5bcf1bacc6b35f 100644 --- a/packages/core/src/render3/hooks.ts +++ b/packages/core/src/render3/hooks.ts @@ -6,9 +6,10 @@ * found in the LICENSE file at https://angular.io/license */ +import {AfterContentChecked, AfterContentInit, AfterViewChecked, AfterViewInit, DoCheck, OnChanges, OnDestroy, OnInit} from '../interface/lifecycle_hooks'; import {assertEqual, assertNotEqual} from '../util/assert'; - import {assertFirstCreatePass} from './assert'; +import {NgOnChangesFeatureImpl} from './features/ng_onchanges_feature'; import {DirectiveDef} from './interfaces/definition'; import {TNode} from './interfaces/node'; import {FLAGS, HookData, InitPhaseState, LView, LViewFlags, PREORDER_HOOK_FLAGS, PreOrderHookFlags, TView} from './interfaces/view'; @@ -31,20 +32,23 @@ import {getCheckNoChangesMode} from './state'; export function registerPreOrderHooks( directiveIndex: number, directiveDef: DirectiveDef, tView: TView): void { ngDevMode && assertFirstCreatePass(tView); - const {onChanges, onInit, doCheck} = directiveDef; + const {ngOnChanges, ngOnInit, ngDoCheck} = + directiveDef.type.prototype as OnChanges & OnInit & DoCheck; - if (onChanges) { - (tView.preOrderHooks || (tView.preOrderHooks = [])).push(directiveIndex, onChanges); - (tView.preOrderCheckHooks || (tView.preOrderCheckHooks = [])).push(directiveIndex, onChanges); + if (ngOnChanges as Function | undefined) { + const wrappedOnChanges = NgOnChangesFeatureImpl(directiveDef); + (tView.preOrderHooks || (tView.preOrderHooks = [])).push(directiveIndex, wrappedOnChanges); + (tView.preOrderCheckHooks || (tView.preOrderCheckHooks = [])) + .push(directiveIndex, wrappedOnChanges); } - if (onInit) { - (tView.preOrderHooks || (tView.preOrderHooks = [])).push(-directiveIndex, onInit); + if (ngOnInit) { + (tView.preOrderHooks || (tView.preOrderHooks = [])).push(0 - directiveIndex, ngOnInit); } - if (doCheck) { - (tView.preOrderHooks || (tView.preOrderHooks = [])).push(directiveIndex, doCheck); - (tView.preOrderCheckHooks || (tView.preOrderCheckHooks = [])).push(directiveIndex, doCheck); + if (ngDoCheck) { + (tView.preOrderHooks || (tView.preOrderHooks = [])).push(directiveIndex, ngDoCheck); + (tView.preOrderCheckHooks || (tView.preOrderCheckHooks = [])).push(directiveIndex, ngDoCheck); } } @@ -73,27 +77,36 @@ export function registerPostOrderHooks(tView: TView, tNode: TNode): void { // hooks for projected components and directives must be called *before* their hosts. for (let i = tNode.directiveStart, end = tNode.directiveEnd; i < end; i++) { const directiveDef = tView.data[i] as DirectiveDef; - if (directiveDef.afterContentInit) { - (tView.contentHooks || (tView.contentHooks = [])).push(-i, directiveDef.afterContentInit); + const lifecycleHooks: AfterContentInit&AfterContentChecked&AfterViewInit&AfterViewChecked& + OnDestroy = directiveDef.type.prototype; + const { + ngAfterContentInit, + ngAfterContentChecked, + ngAfterViewInit, + ngAfterViewChecked, + ngOnDestroy + } = lifecycleHooks; + + if (ngAfterContentInit) { + (tView.contentHooks || (tView.contentHooks = [])).push(-i, ngAfterContentInit); } - if (directiveDef.afterContentChecked) { - (tView.contentHooks || (tView.contentHooks = [])).push(i, directiveDef.afterContentChecked); - (tView.contentCheckHooks || (tView.contentCheckHooks = [])) - .push(i, directiveDef.afterContentChecked); + if (ngAfterContentChecked) { + (tView.contentHooks || (tView.contentHooks = [])).push(i, ngAfterContentChecked); + (tView.contentCheckHooks || (tView.contentCheckHooks = [])).push(i, ngAfterContentChecked); } - if (directiveDef.afterViewInit) { - (tView.viewHooks || (tView.viewHooks = [])).push(-i, directiveDef.afterViewInit); + if (ngAfterViewInit) { + (tView.viewHooks || (tView.viewHooks = [])).push(-i, ngAfterViewInit); } - if (directiveDef.afterViewChecked) { - (tView.viewHooks || (tView.viewHooks = [])).push(i, directiveDef.afterViewChecked); - (tView.viewCheckHooks || (tView.viewCheckHooks = [])).push(i, directiveDef.afterViewChecked); + if (ngAfterViewChecked) { + (tView.viewHooks || (tView.viewHooks = [])).push(i, ngAfterViewChecked); + (tView.viewCheckHooks || (tView.viewCheckHooks = [])).push(i, ngAfterViewChecked); } - if (directiveDef.onDestroy != null) { - (tView.destroyHooks || (tView.destroyHooks = [])).push(i, directiveDef.onDestroy); + if (ngOnDestroy != null) { + (tView.destroyHooks || (tView.destroyHooks = [])).push(i, ngOnDestroy); } } } diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index 0afae0d5e61355..3f2c9c97440590 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -7,6 +7,7 @@ */ import {Injector} from '../../di'; import {ErrorHandler} from '../../error_handler'; +import {DoCheck, OnChanges, OnInit} from '../../interface/lifecycle_hooks'; import {CUSTOM_ELEMENTS_SCHEMA, NO_ERRORS_SCHEMA, SchemaMetadata} from '../../metadata/schema'; import {ViewEncapsulation} from '../../metadata/view'; import {validateAgainstEventAttributes, validateAgainstEventProperties} from '../../sanitization/sanitization'; @@ -1168,9 +1169,11 @@ export function resolveDirectives( if (def.hostBindings !== null || def.hostAttrs !== null || def.hostVars !== 0) tNode.flags |= TNodeFlags.hasHostBindings; + const lifeCycleHooks: OnChanges&OnInit&DoCheck = def.type.prototype; // Only push a node index into the preOrderHooks array if this is the first // pre-order hook found on this node. - if (!preOrderHooksFound && (def.onChanges || def.onInit || def.doCheck)) { + if (!preOrderHooksFound && + (lifeCycleHooks.ngOnChanges || lifeCycleHooks.ngOnInit || lifeCycleHooks.ngDoCheck)) { // We will push the actual hook function into this array later during dir instantiation. // We cannot do it now because we must ensure hooks are registered in the same // order that directives are created (i.e. injection order). @@ -1178,7 +1181,7 @@ export function resolveDirectives( preOrderHooksFound = true; } - if (!preOrderCheckHooksFound && (def.onChanges || def.doCheck)) { + if (!preOrderCheckHooksFound && (lifeCycleHooks.ngOnChanges || lifeCycleHooks.ngDoCheck)) { (tView.preOrderCheckHooks || (tView.preOrderCheckHooks = [])) .push(tNode.index - HEADER_OFFSET); preOrderCheckHooksFound = true; diff --git a/packages/core/src/render3/interfaces/definition.ts b/packages/core/src/render3/interfaces/definition.ts index a2ac00150acf65..a48cb77732bd7d 100644 --- a/packages/core/src/render3/interfaces/definition.ts +++ b/packages/core/src/render3/interfaces/definition.ts @@ -250,16 +250,6 @@ export interface DirectiveDef { */ readonly factory: FactoryFn|null; - /* The following are lifecycle hooks for this component */ - readonly onChanges: (() => void)|null; - readonly onInit: (() => void)|null; - readonly doCheck: (() => void)|null; - readonly afterContentInit: (() => void)|null; - readonly afterContentChecked: (() => void)|null; - readonly afterViewInit: (() => void)|null; - readonly afterViewChecked: (() => void)|null; - readonly onDestroy: (() => void)|null; - /** * The features applied to this directive */ diff --git a/packages/core/test/acceptance/lifecycle_spec.ts b/packages/core/test/acceptance/lifecycle_spec.ts index 40dda04dfc1e1a..fc16069295c231 100644 --- a/packages/core/test/acceptance/lifecycle_spec.ts +++ b/packages/core/test/acceptance/lifecycle_spec.ts @@ -1124,6 +1124,46 @@ describe('onChanges', () => { }); }); +describe('meta-programing', () => { + it('should allow adding lifecycle hook methods any time before first instance creation', () => { + const events: any[] = []; + + @Component({template: ``}) + class App { + } + + @Component({selector: 'child', template: `empty`}) + class Child { + @Input() name: string = ''; + } + + (Child.prototype as any).ngOnInit = () => events.push('onInit'); + (Child.prototype as any).ngOnChanges = (e: SimpleChanges) => { + const name = e['name']; + expect(name.previousValue).toEqual(undefined); + expect(name.currentValue).toEqual('value'); + expect(name.firstChange).toEqual(true); + events.push('ngOnChanges'); + }, (Child.prototype as any).ngDoCheck = () => events.push('ngDoCheck'); + (Child.prototype as any).ngAfterContentInit = () => events.push('ngAfterContentInit'); + (Child.prototype as any).ngAfterContentChecked = () => events.push('ngAfterContentChecked'); + (Child.prototype as any).ngAfterViewInit = () => events.push('ngAfterViewInit'); + (Child.prototype as any).ngAfterViewChecked = () => events.push('ngAfterViewChecked'); + (Child.prototype as any).ngOnDestroy = () => events.push('ngOnDestroy'); + + TestBed.configureTestingModule({ + declarations: [App, Child], + }); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + fixture.destroy(); + expect(events).toEqual([ + 'ngOnChanges', 'onInit', 'ngDoCheck', 'ngAfterContentInit', 'ngAfterContentChecked', + 'ngAfterViewInit', 'ngAfterViewChecked', 'ngOnDestroy' + ]); + }); +}); + it('should call all hooks in correct order when several directives on same node', () => { let log: string[] = []; diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json index 1882884128bf97..d012b3be33246e 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -200,6 +200,9 @@ { "name": "getPreviousOrParentTNode" }, + { + "name": "getSimpleChangesStore" + }, { "name": "getTView" }, @@ -281,6 +284,9 @@ { "name": "nextNgElementId" }, + { + "name": "ngOnChangesSetInput" + }, { "name": "noSideEffects" }, @@ -293,6 +299,9 @@ { "name": "refreshView" }, + { + "name": "rememberChangeHistoryAndInvokeOnChangesHook" + }, { "name": "renderComponent" }, 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 9c2433bd5d6d7c..f7d1fe5780c6cf 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -137,6 +137,9 @@ { "name": "getPreviousOrParentTNode" }, + { + "name": "getSimpleChangesStore" + }, { "name": "includeViewProviders" }, @@ -176,6 +179,9 @@ { "name": "nextNgElementId" }, + { + "name": "ngOnChangesSetInput" + }, { "name": "refreshComponent" }, @@ -185,6 +191,9 @@ { "name": "refreshView" }, + { + "name": "rememberChangeHistoryAndInvokeOnChangesHook" + }, { "name": "renderComponent" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 8900f3f379aff8..0d6e14706ba3a8 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -392,6 +392,9 @@ { "name": "getSelectedIndex" }, + { + "name": "getSimpleChangesStore" + }, { "name": "getSymbolIterator" }, @@ -548,6 +551,9 @@ { "name": "nextNgElementId" }, + { + "name": "ngOnChangesSetInput" + }, { "name": "noSideEffects" }, @@ -569,6 +575,9 @@ { "name": "registerPostOrderHooks" }, + { + "name": "rememberChangeHistoryAndInvokeOnChangesHook" + }, { "name": "removeFromArray" },