Skip to content

Commit

Permalink
fix(core): Allow modification of lifecycle hooks any time before boot…
Browse files Browse the repository at this point in the history
…strap

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 angular#30497
  • Loading branch information
mhevery committed Jun 22, 2020
1 parent 3a418ab commit e3af65e
Show file tree
Hide file tree
Showing 13 changed files with 212 additions and 149 deletions.
4 changes: 2 additions & 2 deletions goldens/public-api/core/core.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down
6 changes: 3 additions & 3 deletions goldens/size-tracking/aio-payloads.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
"master": {
"uncompressed": {
"runtime-es2015": 2987,
"main-es2015": 451406,
"polyfills-es2015": 52630
"main-es2015": 450966,
"polyfills-es2015": 52655
}
}
},
Expand All @@ -26,4 +26,4 @@
}
}
}
}
}
10 changes: 5 additions & 5 deletions goldens/size-tracking/integration-payloads.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@
"master": {
"uncompressed": {
"runtime-es2015": 1485,
"main-es2015": 16959,
"polyfills-es2015": 36938
"main-es2015": 17362,
"polyfills-es2015": 36657
}
}
},
"cli-hello-world-ivy-compat": {
"master": {
"uncompressed": {
"runtime-es2015": 1485,
"main-es2015": 147314,
"main-es2015": 147573,
"polyfills-es2015": 36571
}
}
Expand Down Expand Up @@ -49,7 +49,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 2289,
"main-es2015": 221268,
"main-es2015": 221846,
"polyfills-es2015": 36938,
"5-es2015": 779
}
Expand All @@ -66,4 +66,4 @@
}
}
}
}
}
125 changes: 59 additions & 66 deletions packages/core/src/render3/definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,72 +290,65 @@ export function ɵɵdefineComponent<T>(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<ComponentDef<any>, keyof ComponentDef<any>> = {
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<ComponentDef<any>, keyof ComponentDef<any>> = {
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;
}

/**
Expand Down
11 changes: 0 additions & 11 deletions packages/core/src/render3/features/inherit_definition_feature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
62 changes: 35 additions & 27 deletions packages/core/src/render3/features/ng_onchanges_feature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -55,28 +50,37 @@ 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) {
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<T>(
this: DirectiveDef<T>, instance: T, value: any, publicName: string, privateName: string): void {
const simpleChangesStore = getSimpleChangesStore(instance) ||
Expand All @@ -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;
Expand Down

0 comments on commit e3af65e

Please sign in to comment.