fix(core): Allow modification of lifecycle hooks any time before bootstrap (#35464)

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

PR Close #35464
This commit is contained in:
Misko Hevery 2020-02-14 18:02:11 -08:00 committed by Andrew Kushnir
parent 469aba0140
commit 737506e79c
13 changed files with 264 additions and 147 deletions

View File

@ -894,7 +894,7 @@ export declare function ɵɵnextContext<T = any>(level?: number): T;
export declare type ɵɵNgModuleDefWithMeta<T, Declarations, Imports, Exports> = ɵNgModuleDef<T>;
export declare function ɵɵNgOnChangesFeature<T>(definition: ɵDirectiveDef<T>): void;
export declare function ɵɵNgOnChangesFeature<T>(): 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<any> | 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<unknown> | null;
export declare function ɵɵtemplateRefExtractor(tNode: TNode, currentView: ɵangular_packages_core_core_bp): TemplateRef<unknown> | null;
export declare function ɵɵtext(index: number, value?: string): void;

View File

@ -12,7 +12,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 2987,
"main-es2015": 450883,
"main-es2015": 450301,
"polyfills-es2015": 52630
}
}

View File

@ -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
}

View File

@ -314,14 +314,6 @@ export function ɵɵdefineComponent<T>(componentDefinition: {
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
@ -329,8 +321,8 @@ export function ɵɵdefineComponent<T>(componentDefinition: {
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.
// 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,
@ -351,11 +343,12 @@ export function ɵɵdefineComponent<T>(componentDefinition: {
.map(extractDirectiveDef) :
null;
def.pipeDefs = pipeTypes ?
() => (typeof pipeTypes === 'function' ? pipeTypes() : pipeTypes).map(extractPipeDef) :
() =>
(typeof pipeTypes === 'function' ? pipeTypes() : pipeTypes).map(extractPipeDef) :
null;
return def as never;
});
}) as never;
}
/**

View File

@ -78,17 +78,6 @@ export function ɵɵInheritDefinitionFeature(definition: DirectiveDef<any>|Compo
const defData = (definition as ComponentDef<any>).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

View File

@ -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<T>(): DirectiveDefFeature {
return NgOnChangesFeatureImpl;
}
export function ɵɵNgOnChangesFeature<T>(definition: DirectiveDef<T>): void {
export function NgOnChangesFeatureImpl<T>(definition: DirectiveDef<T>) {
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,10 +50,19 @@ export function ɵɵNgOnChangesFeature<T>(definition: DirectiveDef<T>): void {
// tslint:disable-next-line:no-toplevel-property-access
(ɵɵNgOnChangesFeature as DirectiveDefFeature).ngInherit = true;
function wrapOnChanges() {
return function wrapOnChangesHook_inPreviousChangesStorage(this: OnChanges) {
/**
* 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 && simpleChangesStore.current;
const current = simpleChangesStore?.current;
if (current) {
const previous = simpleChangesStore!.previous;
@ -74,9 +78,9 @@ function wrapOnChanges() {
simpleChangesStore!.current = null;
this.ngOnChanges(current);
}
};
}
function ngOnChangesSetInput<T>(
this: DirectiveDef<T>, 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;

View File

@ -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<any>, 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<any>;
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);
}
}
}

View File

@ -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';
@ -1166,9 +1167,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).
@ -1176,7 +1179,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;

View File

@ -250,16 +250,6 @@ export interface DirectiveDef<T> {
*/
readonly factory: FactoryFn<T>|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
*/

View File

@ -1124,6 +1124,100 @@ describe('onChanges', () => {
});
});
describe('meta-programing', () => {
it('should allow adding lifecycle hook methods any time before first instance creation', () => {
const events: any[] = [];
@Component({template: `<child name="value"></child>`})
class App {
}
@Component({selector: 'child', template: `empty`})
class Child {
@Input() name: string = '';
}
const ChildPrototype = Child.prototype as any;
ChildPrototype.ngOnInit = () => events.push('onInit');
ChildPrototype.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');
};
ChildPrototype.ngDoCheck = () => events.push('ngDoCheck');
ChildPrototype.ngAfterContentInit = () => events.push('ngAfterContentInit');
ChildPrototype.ngAfterContentChecked = () => events.push('ngAfterContentChecked');
ChildPrototype.ngAfterViewInit = () => events.push('ngAfterViewInit');
ChildPrototype.ngAfterViewChecked = () => events.push('ngAfterViewChecked');
ChildPrototype.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 allow adding lifecycle hook methods with inheritance any time before first instance creation',
() => {
const events: any[] = [];
@Component({template: `<child name="value"></child>`})
class App {
}
class BaseChild {}
@Component({selector: 'child', template: `empty`})
class Child extends BaseChild {
@Input() name: string = '';
}
// These are defined on the base class
const BasePrototype = BaseChild.prototype as any;
BasePrototype.ngOnInit = () => events.push('onInit');
BasePrototype.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');
};
// These will be overwritten later
BasePrototype.ngDoCheck = () => events.push('Expected to be overbidden');
BasePrototype.ngAfterContentInit = () => events.push('Expected to be overbidden');
// These are define on the concrete class
const ChildPrototype = Child.prototype as any;
ChildPrototype.ngDoCheck = () => events.push('ngDoCheck');
ChildPrototype.ngAfterContentInit = () => events.push('ngAfterContentInit');
ChildPrototype.ngAfterContentChecked = () => events.push('ngAfterContentChecked');
ChildPrototype.ngAfterViewInit = () => events.push('ngAfterViewInit');
ChildPrototype.ngAfterViewChecked = () => events.push('ngAfterViewChecked');
ChildPrototype.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[] = [];

View File

@ -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"
},

View File

@ -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"
},

View File

@ -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"
},