Skip to content

Commit

Permalink
fix(core): remove side effects from ɵɵNgOnChangesFeature() (#35769) (
Browse files Browse the repository at this point in the history
…#35846)

`ɵɵNgOnChangesFeature()` would set `ngInherit`, which is a side effect and also not necessary. This was pulled out to module scope so the function itself can be pure. Since it only curries another function, the call is entirely unnecessary. Updated the compiler to only generate a reference to this function, rather than a call to it, and removed the extra curry indirection.

PR Close #35769

(cherry picked from commit 9cf85d2)

PR Close #35846
  • Loading branch information
dgp1130 authored and matsko committed Mar 6, 2020
1 parent 68ca32f commit d24ce21
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2542,7 +2542,7 @@ describe('compiler compliance', () => {
type: LifecycleComp,
selectors: [["lifecycle-comp"]],
inputs: {nameMin: ["name", "nameMin"]},
features: [$r3$.ɵɵNgOnChangesFeature()],
features: [$r3$.ɵɵNgOnChangesFeature],
decls: 0,
vars: 0,
template: function LifecycleComp_Template(rf, ctx) {},
Expand Down Expand Up @@ -2662,7 +2662,7 @@ describe('compiler compliance', () => {
ForOfDirective.ɵdir = $r3$.ɵɵdefineDirective({
type: ForOfDirective,
selectors: [["", "forOf", ""]],
features: [$r3$.ɵɵNgOnChangesFeature()],
features: [$r3$.ɵɵNgOnChangesFeature],
inputs: {forOf: "forOf"}
});
`;
Expand Down Expand Up @@ -2742,7 +2742,7 @@ describe('compiler compliance', () => {
ForOfDirective.ɵdir = $r3$.ɵɵdefineDirective({
type: ForOfDirective,
selectors: [["", "forOf", ""]],
features: [$r3$.ɵɵNgOnChangesFeature()],
features: [$r3$.ɵɵNgOnChangesFeature],
inputs: {forOf: "forOf"}
});
`;
Expand Down Expand Up @@ -3767,7 +3767,7 @@ describe('compiler compliance', () => {
// ...
BaseClass.ɵdir = $r3$.ɵɵdefineDirective({
type: BaseClass,
features: [$r3$.ɵɵNgOnChangesFeature()]
features: [$r3$.ɵɵNgOnChangesFeature]
});
// ...
`;
Expand Down
4 changes: 2 additions & 2 deletions packages/compiler/src/render3/view/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ function baseDirectiveFields(
*/
function addFeatures(
definitionMap: DefinitionMap, meta: R3DirectiveMetadata | R3ComponentMetadata) {
// e.g. `features: [NgOnChangesFeature()]`
// e.g. `features: [NgOnChangesFeature]`
const features: o.Expression[] = [];

const providers = meta.providers;
Expand All @@ -107,7 +107,7 @@ function addFeatures(
features.push(o.importExpr(R3.CopyDefinitionFeature));
}
if (meta.lifecycle.usesOnChanges) {
features.push(o.importExpr(R3.NgOnChangesFeature).callFn(EMPTY_ARRAY));
features.push(o.importExpr(R3.NgOnChangesFeature));
}
if (features.length) {
definitionMap.set('features', o.literalArr(features));
Expand Down
16 changes: 8 additions & 8 deletions packages/core/src/render3/features/ng_onchanges_feature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,26 +35,26 @@ type OnChangesExpando = OnChanges & {
* static ɵcmp = defineComponent({
* ...
* inputs: {name: 'publicName'},
* features: [NgOnChangesFeature()]
* features: [NgOnChangesFeature]
* });
* ```
*
* @codeGenApi
*/
export function ɵɵNgOnChangesFeature<T>(): DirectiveDefFeature {
// This option ensures that the ngOnChanges lifecycle hook will be inherited
// from superclasses (in InheritDefinitionFeature).
(NgOnChangesFeatureImpl as DirectiveDefFeature).ngInherit = true;
return NgOnChangesFeatureImpl;
}

function NgOnChangesFeatureImpl<T>(definition: DirectiveDef<T>): void {
export function ɵɵNgOnChangesFeature<T>(definition: DirectiveDef<T>): void {
if (definition.type.prototype.ngOnChanges) {
definition.setInput = ngOnChangesSetInput;
(definition as{onChanges: Function}).onChanges = wrapOnChanges();
}
}

// This option ensures that the ngOnChanges lifecycle hook will be inherited
// from superclasses (in InheritDefinitionFeature).
/** @nocollapse */
// 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);
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/render3/common_with_def.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ NgIf.ɵfac = () =>
NgTemplateOutlet.ɵdir = ɵɵdefineDirective({
type: NgTemplateOutletDef,
selectors: [['', 'ngTemplateOutlet', '']],
features: [ɵɵNgOnChangesFeature()],
features: [ɵɵNgOnChangesFeature],
inputs:
{ngTemplateOutlet: 'ngTemplateOutlet', ngTemplateOutletContext: 'ngTemplateOutletContext'}
});
Expand Down
2 changes: 1 addition & 1 deletion tools/public_api_guard/core/core.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -915,7 +915,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>(): DirectiveDefFeature;
export declare function ɵɵNgOnChangesFeature<T>(definition: DirectiveDef<T>): void;

export declare function ɵɵpipe(index: number, pipeName: string): any;

Expand Down

0 comments on commit d24ce21

Please sign in to comment.