From 9cf85d21775c259bc580d43a729283856db82757 Mon Sep 17 00:00:00 2001 From: Doug Parker Date: Fri, 21 Feb 2020 11:34:39 -0800 Subject: [PATCH] =?UTF-8?q?fix(core):=20remove=20side=20effects=20from=20`?= =?UTF-8?q?=C9=B5=C9=B5NgOnChangesFeature()`=20(#35769)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ɵɵ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 --- .../compliance/r3_compiler_compliance_spec.ts | 8 ++++---- packages/compiler/src/render3/view/compiler.ts | 4 ++-- .../src/render3/features/ng_onchanges_feature.ts | 16 ++++++++-------- packages/core/test/render3/common_with_def.ts | 2 +- tools/public_api_guard/core/core.d.ts | 2 +- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts index b9b96fcff5..1df894c1c0 100644 --- a/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts @@ -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) {}, @@ -2662,7 +2662,7 @@ describe('compiler compliance', () => { ForOfDirective.ɵdir = $r3$.ɵɵdefineDirective({ type: ForOfDirective, selectors: [["", "forOf", ""]], - features: [$r3$.ɵɵNgOnChangesFeature()], + features: [$r3$.ɵɵNgOnChangesFeature], inputs: {forOf: "forOf"} }); `; @@ -2742,7 +2742,7 @@ describe('compiler compliance', () => { ForOfDirective.ɵdir = $r3$.ɵɵdefineDirective({ type: ForOfDirective, selectors: [["", "forOf", ""]], - features: [$r3$.ɵɵNgOnChangesFeature()], + features: [$r3$.ɵɵNgOnChangesFeature], inputs: {forOf: "forOf"} }); `; @@ -3767,7 +3767,7 @@ describe('compiler compliance', () => { // ... BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ type: BaseClass, - features: [$r3$.ɵɵNgOnChangesFeature()] + features: [$r3$.ɵɵNgOnChangesFeature] }); // ... `; diff --git a/packages/compiler/src/render3/view/compiler.ts b/packages/compiler/src/render3/view/compiler.ts index e105af2d17..a481bdb674 100644 --- a/packages/compiler/src/render3/view/compiler.ts +++ b/packages/compiler/src/render3/view/compiler.ts @@ -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; @@ -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)); diff --git a/packages/core/src/render3/features/ng_onchanges_feature.ts b/packages/core/src/render3/features/ng_onchanges_feature.ts index efc69de2ff..7d749c624f 100644 --- a/packages/core/src/render3/features/ng_onchanges_feature.ts +++ b/packages/core/src/render3/features/ng_onchanges_feature.ts @@ -35,26 +35,26 @@ type OnChangesExpando = OnChanges & { * static ɵcmp = defineComponent({ * ... * inputs: {name: 'publicName'}, - * features: [NgOnChangesFeature()] + * features: [NgOnChangesFeature] * }); * ``` * * @codeGenApi */ -export function ɵɵNgOnChangesFeature(): 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(definition: DirectiveDef): void { +export function ɵɵNgOnChangesFeature(definition: DirectiveDef): 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); diff --git a/packages/core/test/render3/common_with_def.ts b/packages/core/test/render3/common_with_def.ts index 9a40985fcd..96904c1b8e 100644 --- a/packages/core/test/render3/common_with_def.ts +++ b/packages/core/test/render3/common_with_def.ts @@ -41,7 +41,7 @@ NgIf.ɵfac = () => NgTemplateOutlet.ɵdir = ɵɵdefineDirective({ type: NgTemplateOutletDef, selectors: [['', 'ngTemplateOutlet', '']], - features: [ɵɵNgOnChangesFeature()], + features: [ɵɵNgOnChangesFeature], inputs: {ngTemplateOutlet: 'ngTemplateOutlet', ngTemplateOutletContext: 'ngTemplateOutletContext'} }); diff --git a/tools/public_api_guard/core/core.d.ts b/tools/public_api_guard/core/core.d.ts index 2184ce1cf1..d927be91d1 100644 --- a/tools/public_api_guard/core/core.d.ts +++ b/tools/public_api_guard/core/core.d.ts @@ -915,7 +915,7 @@ export declare function ɵɵnextContext(level?: number): T; export declare type ɵɵNgModuleDefWithMeta = NgModuleDef; -export declare function ɵɵNgOnChangesFeature(): DirectiveDefFeature; +export declare function ɵɵNgOnChangesFeature(definition: DirectiveDef): void; export declare function ɵɵpipe(index: number, pipeName: string): any;