fix(ivy): providers should not be inherited (#27013)

PR Close #27013

packages/core/src/render3/features/inherit_definition_feature.ts

@@ -146,7 +146,7 @@ export function InheritDefinitionFeature(definition: DirectiveDef<any>| Componen
       const features = superDef.features;
       if (features) {
         for (const feature of features) {
-          if (feature && feature !== InheritDefinitionFeature) {
+          if (feature && feature.ngInherit) {
             (feature as DirectiveDefFeature)(definition);
           }
         }

packages/core/src/render3/features/ng_onchanges_feature.ts

@@ -8,7 +8,7 @@
 
 import {SimpleChange} from '../../change_detection/change_detection_util';
 import {OnChanges, SimpleChanges} from '../../metadata/lifecycle_hooks';
-import {DirectiveDef} from '../interfaces/definition';
+import {DirectiveDef, DirectiveDefFeature} from '../interfaces/definition';
 
 const PRIVATE_PREFIX = '__ngOnChanges_';
 
@@ -106,6 +106,10 @@ export function NgOnChangesFeature<T>(definition: DirectiveDef<T>): void {
   definition.doCheck = onChangesWrapper(definition.doCheck);
 }
 
+// This option ensures that the ngOnChanges lifecycle hook will be inherited
+// from superclasses (in InheritDefinitionFeature).
+(NgOnChangesFeature as DirectiveDefFeature).ngInherit = true;
+
 function onChangesWrapper(delegateHook: (() => void) | null) {
   return function(this: OnChangesExpando) {
     const simpleChanges = this[PRIVATE_PREFIX];

packages/core/src/render3/interfaces/definition.ts

@@ -307,8 +307,16 @@ export interface PipeDef<T> {
 
 export type PipeDefWithMeta<T, Name extends string> = PipeDef<T>;
 
-export type DirectiveDefFeature = <T>(directiveDef: DirectiveDef<T>) => void;
-export type ComponentDefFeature = <T>(componentDef: ComponentDef<T>) => void;
+export interface DirectiveDefFeature {
+  <T>(directiveDef: DirectiveDef<T>): void;
+  ngInherit?: true;
+}
+
+export interface ComponentDefFeature {
+  <T>(componentDef: ComponentDef<T>): void;
+  ngInherit?: true;
+}
+
 
 /**
  * Type used for directiveDefs on component definition.

packages/core/test/bundling/animation_world/bundle.golden_symbols.json

@@ -137,6 +137,9 @@
   {
     "name": "NgModuleRef"
   },
+  {
+    "name": "NgOnChangesFeature"
+  },
   {
     "name": "NodeInjector$1"
   },
@@ -161,6 +164,9 @@
   {
     "name": "PARENT_INJECTOR"
   },
+  {
+    "name": "PRIVATE_PREFIX"
+  },
   {
     "name": "QUERIES"
   },
@@ -188,6 +194,9 @@
   {
     "name": "SWITCH_VIEW_CONTAINER_REF_FACTORY"
   },
+  {
+    "name": "SimpleChange"
+  },
   {
     "name": "SimpleKeyframePlayer"
   },
@@ -944,6 +953,9 @@
   {
     "name": "noop"
   },
+  {
+    "name": "onChangesWrapper"
+  },
   {
     "name": "pointers"
   },

packages/core/test/bundling/hello_world/bundle.golden_symbols.json

@@ -83,6 +83,9 @@
   {
     "name": "NO_PARENT_INJECTOR"
   },
+  {
+    "name": "NgOnChangesFeature"
+  },
   {
     "name": "NodeInjectorFactory"
   },
@@ -95,6 +98,9 @@
   {
     "name": "PARENT_INJECTOR"
   },
+  {
+    "name": "PRIVATE_PREFIX"
+  },
   {
     "name": "QUERIES"
   },
@@ -107,6 +113,9 @@
   {
     "name": "SANITIZER"
   },
+  {
+    "name": "SimpleChange"
+  },
   {
     "name": "TVIEW"
   },
@@ -383,6 +392,9 @@
   {
     "name": "noop"
   },
+  {
+    "name": "onChangesWrapper"
+  },
   {
     "name": "postProcessBaseDirective"
   },

packages/core/test/bundling/hello_world_r2/bundle.golden_symbols.json

@@ -281,6 +281,9 @@
   {
     "name": "NgModuleRef$1"
   },
+  {
+    "name": "NgOnChangesFeature"
+  },
   {
     "name": "NgZone"
   },
@@ -329,6 +332,9 @@
   {
     "name": "PLATFORM_INITIALIZER"
   },
+  {
+    "name": "PRIVATE_PREFIX"
+  },
   {
     "name": "PlatformLocation"
   },
@@ -383,6 +389,9 @@
   {
     "name": "Self"
   },
+  {
+    "name": "SimpleChange"
+  },
   {
     "name": "SkipSelf"
   },
@@ -1142,6 +1151,9 @@
   {
     "name": "observable"
   },
+  {
+    "name": "onChangesWrapper"
+  },
   {
     "name": "onEnter"
   },

packages/core/test/bundling/injection/bundle.golden_symbols.json

@@ -32,6 +32,9 @@
   {
     "name": "NULL_INJECTOR$2"
   },
+  {
+    "name": "NgOnChangesFeature"
+  },
   {
     "name": "NullInjector"
   },
@@ -44,6 +47,9 @@
   {
     "name": "PARAMETERS"
   },
+  {
+    "name": "PRIVATE_PREFIX"
+  },
   {
     "name": "R3Injector"
   },
@@ -53,6 +59,9 @@
   {
     "name": "Self"
   },
+  {
+    "name": "SimpleChange"
+  },
   {
     "name": "SkipSelf"
   },
@@ -152,6 +161,9 @@
   {
     "name": "makeRecord"
   },
+  {
+    "name": "onChangesWrapper"
+  },
   {
     "name": "providerToFactory"
   },

packages/core/test/bundling/todo/bundle.golden_symbols.json

@@ -131,6 +131,9 @@
   {
     "name": "NgModuleRef"
   },
+  {
+    "name": "NgOnChangesFeature"
+  },
   {
     "name": "NodeInjector$1"
   },
@@ -155,6 +158,9 @@
   {
     "name": "PARENT_INJECTOR"
   },
+  {
+    "name": "PRIVATE_PREFIX"
+  },
   {
     "name": "QUERIES"
   },
@@ -182,6 +188,9 @@
   {
     "name": "SWITCH_VIEW_CONTAINER_REF_FACTORY"
   },
+  {
+    "name": "SimpleChange"
+  },
   {
     "name": "SkipSelf"
   },
@@ -962,6 +971,9 @@
   {
     "name": "noop"
   },
+  {
+    "name": "onChangesWrapper"
+  },
   {
     "name": "pointers"
   },

packages/core/test/render3/Inherit_definition_feature_spec.ts

@@ -6,10 +6,9 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {EventEmitter, Output} from '../../src/core';
-import {EMPTY} from '../../src/render3/definition';
-import {InheritDefinitionFeature} from '../../src/render3/features/inherit_definition_feature';
-import {ComponentDef, DirectiveDef, RenderFlags, defineBase, defineComponent, defineDirective} from '../../src/render3/index';
+import {Inject, InjectionToken} from '../../src/core';
+import {ComponentDef, DirectiveDef, InheritDefinitionFeature, NgOnChangesFeature, ProvidersFeature, RenderFlags, defineBase, defineComponent, defineDirective, directiveInject, element} from '../../src/render3/index';
+import {ComponentFixture, createComponent} from './render_util';
 
 describe('InheritDefinitionFeature', () => {
   it('should inherit lifecycle hooks', () => {
@@ -457,50 +456,90 @@ describe('InheritDefinitionFeature', () => {
     }).toThrowError('Directives cannot inherit Components');
   });
 
-  it('should run inherited features', () => {
-    const log: any[] = [];
+  it('should inherit ngOnChanges', () => {
+    const log: string[] = [];
+    let subDir !: SubDirective;
 
+    class SuperDirective {
+      someInput = '';
+
+      ngOnChanges() { log.push('on changes!'); }
+
+      static ngDirectiveDef = defineDirective({
+        type: SuperDirective,
+        selectors: [['', 'superDir', '']],
+        factory: () => new SuperDirective(),
+        features: [NgOnChangesFeature],
+        inputs: {someInput: 'someInput'}
+      });
+    }
+
+    class SubDirective extends SuperDirective {
+      static ngDirectiveDef = defineDirective({
+        type: SubDirective,
+        selectors: [['', 'subDir', '']],
+        factory: () => subDir = new SubDirective(),
+        features: [InheritDefinitionFeature],
+      });
+    }
+
+    const App = createComponent('app', (rf: RenderFlags, ctx: any) => {
+      if (rf & RenderFlags.Create) {
+        element(0, 'div', ['subDir', '']);
+      }
+    }, 1, 0, [SubDirective]);
+
+    const fixture = new ComponentFixture(App);
+    expect(log).toEqual(['on changes!']);
+  });
+
+  it('should NOT inherit providers', () => {
+    let otherDir !: OtherDirective;
+
+    const SOME_DIRS = new InjectionToken('someDirs');
+
+    // providers: [{ provide: SOME_DIRS, useClass: SuperDirective, multi: true }]
     class SuperDirective {
       static ngDirectiveDef = defineDirective({
         type: SuperDirective,
         selectors: [['', 'superDir', '']],
         factory: () => new SuperDirective(),
-        features: [
-          (arg: any) => { log.push('super1', arg); },
-          (arg: any) => { log.push('super2', arg); },
-        ]
+        features: [ProvidersFeature([{provide: SOME_DIRS, useClass: SuperDirective, multi: true}])],
       });
     }
 
+    // providers: [{ provide: SOME_DIRS, useClass: SubDirective, multi: true }]
     class SubDirective extends SuperDirective {
-      @Output()
-      baz = new EventEmitter();
-
-      @Output()
-      qux = new EventEmitter();
-
       static ngDirectiveDef = defineDirective({
         type: SubDirective,
         selectors: [['', 'subDir', '']],
         factory: () => new SubDirective(),
-        features: [InheritDefinitionFeature, (arg: any) => { log.push('sub1', arg); }]
+        features: [
+          ProvidersFeature([{provide: SOME_DIRS, useClass: SubDirective, multi: true}]),
+          InheritDefinitionFeature
+        ],
       });
     }
 
-    const superDef = SuperDirective.ngDirectiveDef as DirectiveDef<any>;
-    const subDef = SubDirective.ngDirectiveDef as DirectiveDef<any>;
+    class OtherDirective {
+      constructor(@Inject(SOME_DIRS) public dirs: any) {}
 
-    expect(log).toEqual([
-      'super1',
-      superDef,
-      'super2',
-      superDef,
-      'super1',
-      subDef,
-      'super2',
-      subDef,
-      'sub1',
-      subDef,
-    ]);
+      static ngDirectiveDef = defineDirective({
+        type: OtherDirective,
+        selectors: [['', 'otherDir', '']],
+        factory: () => otherDir = new OtherDirective(directiveInject(SOME_DIRS)),
+      });
+    }
+
+    /** <div otherDir subDir></div> */
+    const App = createComponent('app', (rf: RenderFlags, ctx: any) => {
+      if (rf & RenderFlags.Create) {
+        element(0, 'div', ['otherDir', '', 'subDir', '']);
+      }
+    }, 1, 0, [OtherDirective, SubDirective, SuperDirective]);
+
+    const fixture = new ComponentFixture(App);
+    expect(otherDir.dirs.length).toEqual(1);
+    expect(otherDir.dirs[0] instanceof SubDirective).toBe(true);
   });
 });