fix(ivy): ngOnChanges should be inherited from super class (#28563)

PR Close #28563

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

@@ -12,6 +12,7 @@ import {fillProperties} from '../../util/property';
 import {EMPTY_ARRAY, EMPTY_OBJ} from '../empty';
 import {ComponentDef, DirectiveDef, DirectiveDefFeature, RenderFlags} from '../interfaces/definition';
 
+import {NgOnChangesFeature} from './ng_onchanges_feature';
 
 
 /**
@@ -168,6 +169,10 @@ export function InheritDefinitionFeature(definition: DirectiveDef<any>| Componen
         definition.doCheck = definition.doCheck || superPrototype.ngDoCheck;
         definition.onDestroy = definition.onDestroy || superPrototype.ngOnDestroy;
         definition.onInit = definition.onInit || superPrototype.ngOnInit;
+
+        if (superPrototype.ngOnChanges) {
+          NgOnChangesFeature()(definition);
+        }
       }
     }
 

packages/core/src/render3/jit/directive.ts

@@ -151,9 +151,7 @@ function directiveMetadata(type: Type<any>, metadata: Directive): R3DirectiveMet
     inputs: metadata.inputs || EMPTY_ARRAY,
     outputs: metadata.outputs || EMPTY_ARRAY,
     queries: extractQueriesMetadata(type, propMetadata, isContentQuery),
-    lifecycle: {
+    lifecycle: {usesOnChanges: type.prototype.hasOwnProperty('ngOnChanges')},
-      usesOnChanges: type.prototype.ngOnChanges !== undefined,
-    },
     typeSourceSpan: null !,
     usesInheritance: !extendsDirectlyFromObject(type),
     exportAs: extractExportAs(metadata.exportAs),

packages/core/test/acceptance/inherit_definition_feature_spec.ts

@@ -0,0 +1,60 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {Component, Directive, Input, OnChanges, Type} from '@angular/core';
+import {TestBed} from '@angular/core/testing';
+
+describe('ngOnChanges', () => {
+  it('should be inherited when super is a directive', () => {
+    const log: string[] = [];
+
+    @Directive({selector: '[superDir]'})
+    class SuperDirective implements OnChanges {
+      @Input() someInput = '';
+
+      ngOnChanges() { log.push('on changes!'); }
+    }
+
+    @Directive({selector: '[subDir]'})
+    class SubDirective extends SuperDirective {
+    }
+
+    TestBed.configureTestingModule({declarations: [AppComp, SubDirective]});
+    TestBed.overrideComponent(
+        AppComp, {set: new Component({template: '<div subDir [someInput]="1"></div>'})});
+    const fixture = TestBed.createComponent(AppComp);
+    fixture.detectChanges();
+
+    expect(log).toEqual(['on changes!']);
+  });
+
+  it('should be inherited when super is a simple class', () => {
+    const log: string[] = [];
+
+    class SuperClass {
+      ngOnChanges() { log.push('on changes!'); }
+    }
+
+    @Directive({selector: '[subDir]'})
+    class SubDirective extends SuperClass {
+      @Input() someInput = '';
+    }
+
+    TestBed.configureTestingModule({declarations: [AppComp, SubDirective]});
+    TestBed.overrideComponent(
+        AppComp, {set: new Component({template: '<div subDir [someInput]="1"></div>'})});
+    const fixture = TestBed.createComponent(AppComp);
+    fixture.detectChanges();
+
+    expect(log).toEqual(['on changes!']);
+  });
+});
+
+@Component({selector: 'app-comp', template: ``})
+class AppComp {
+}

packages/core/test/render3/inherit_definition_feature_spec.ts

@@ -662,46 +662,6 @@ describe('InheritDefinitionFeature', () => {
     }).toThrowError('Directives cannot inherit Components');
   });
 
-  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', '']);
-      }
-      if (rf & RenderFlags.Update) {
-        elementProperty(0, 'someInput', bind(1));
-      }
-    }, 1, 1, [SubDirective]);
-
-    const fixture = new ComponentFixture(App);
-    expect(log).toEqual(['on changes!']);
-  });
-
   it('should NOT inherit providers', () => {
     let otherDir !: OtherDirective;
 

packages/examples/upgrade/static/ts/full/e2e_test/static_full_spec.ts

@@ -6,7 +6,6 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {fixmeIvy} from '@angular/private/testing';
 import {browser, by, element} from 'protractor';
 import {verifyNoBrowserErrors} from '../../../../../test-utils';
 
@@ -29,8 +28,7 @@ describe('upgrade/static (full)', () => {
     expect(heroComponents.count()).toEqual(3);
   });
 
-  fixmeIvy('unknown; <ng1Hero> component does not seem to render name & description')
+  it('should add a new hero when the "Add Hero" button is pressed', () => {
-      .it('should add a new hero when the "Add Hero" button is pressed', () => {
     const addHeroButton = element.all(by.css('button')).last();
     expect(addHeroButton.getText()).toEqual('Add Hero');
     addHeroButton.click();
@@ -38,8 +36,7 @@ describe('upgrade/static (full)', () => {
     expect(heroComponents.last().element(by.css('h2')).getText()).toEqual('Kamala Khan');
   });
 
-  fixmeIvy('unknown; <ng1Hero> component does not seem to render name & description')
+  it('should remove a hero when the "Remove" button is pressed', () => {
-      .it('should remove a hero when the "Remove" button is pressed', () => {
     let firstHero = element.all(by.css('ng1-hero')).get(0);
     expect(firstHero.element(by.css('h2')).getText()).toEqual('Superman');
 

packages/examples/upgrade/static/ts/lite/e2e_test/static_lite_spec.ts

@@ -6,7 +6,6 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {fixmeIvy} from '@angular/private/testing';
 import {ElementArrayFinder, ElementFinder, browser, by, element} from 'protractor';
 import {verifyNoBrowserErrors} from '../../../../../test-utils';
 
@@ -59,8 +58,7 @@ describe('upgrade/static (lite)', () => {
 
   it('should initially not render the heroes', () => expectHeroes(false));
 
-  fixmeIvy('unknown; <ng1Hero> component does not seem to render name & description')
+  it('should toggle the heroes when clicking the "show/hide" button', () => {
-      .it('should toggle the heroes when clicking the "show/hide" button', () => {
     showHideBtn.click();
     expectHeroes(true);
 
@@ -68,8 +66,7 @@ describe('upgrade/static (lite)', () => {
     expectHeroes(false);
   });
 
-  fixmeIvy('unknown; <ng1Hero> component does not seem to render name & description')
+  it('should add a new hero when clicking the "add" button', () => {
-      .it('should add a new hero when clicking the "add" button', () => {
     showHideBtn.click();
     ng2HeroesAddBtn.click();
 
@@ -77,8 +74,7 @@ describe('upgrade/static (lite)', () => {
     expect(ng1Heroes.last()).toHaveName('Kamala Khan');
   });
 
-  fixmeIvy('unknown; <ng1Hero> component does not seem to render name & description')
+  it('should remove a hero when clicking its "remove" button', () => {
-      .it('should remove a hero when clicking its "remove" button', () => {
     showHideBtn.click();
 
     const firstHero = ng1Heroes.first();