fix(core): remove side effects from ɵɵNgOnChangesFeature() (angular…

…#35769) `ɵɵ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 angular#35769 (cherry picked from commit 9cf85d2)
dgp1130 · Mar 4, 2020 · 614a105 · 614a105
1 parent e75b1d7
commit 614a105
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 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
@@ -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
@@ -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
@@ -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);

diff --git 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
@@ -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;