diff --git a/packages/core/src/render3/features/inherit_definition_feature.ts b/packages/core/src/render3/features/inherit_definition_feature.ts index faab82aab6..13c21b41e4 100644 --- a/packages/core/src/render3/features/inherit_definition_feature.ts +++ b/packages/core/src/render3/features/inherit_definition_feature.ts @@ -25,6 +25,7 @@ export function getSuperType(type: Type): Type& */ export function ɵɵInheritDefinitionFeature(definition: DirectiveDef| ComponentDef): void { let superType = getSuperType(definition.type); + let shouldInheritFields = true; while (superType) { let superDef: DirectiveDef|ComponentDef|undefined = undefined; @@ -40,38 +41,40 @@ export function ɵɵInheritDefinitionFeature(definition: DirectiveDef| Comp } if (superDef) { - // Some fields in the definition may be empty, if there were no values to put in them that - // would've justified object creation. Unwrap them if necessary. - const writeableDef = definition as any; - writeableDef.inputs = maybeUnwrapEmpty(definition.inputs); - writeableDef.declaredInputs = maybeUnwrapEmpty(definition.declaredInputs); - writeableDef.outputs = maybeUnwrapEmpty(definition.outputs); + if (shouldInheritFields) { + // Some fields in the definition may be empty, if there were no values to put in them that + // would've justified object creation. Unwrap them if necessary. + const writeableDef = definition as any; + writeableDef.inputs = maybeUnwrapEmpty(definition.inputs); + writeableDef.declaredInputs = maybeUnwrapEmpty(definition.declaredInputs); + writeableDef.outputs = maybeUnwrapEmpty(definition.outputs); - // Merge hostBindings - const superHostBindings = superDef.hostBindings; - superHostBindings && inheritHostBindings(definition, superHostBindings); + // Merge hostBindings + const superHostBindings = superDef.hostBindings; + superHostBindings && inheritHostBindings(definition, superHostBindings); - // Merge queries - const superViewQuery = superDef.viewQuery; - const superContentQueries = superDef.contentQueries; - superViewQuery && inheritViewQuery(definition, superViewQuery); - superContentQueries && inheritContentQueries(definition, superContentQueries); + // Merge queries + const superViewQuery = superDef.viewQuery; + const superContentQueries = superDef.contentQueries; + superViewQuery && inheritViewQuery(definition, superViewQuery); + superContentQueries && inheritContentQueries(definition, superContentQueries); - // Merge inputs and outputs - fillProperties(definition.inputs, superDef.inputs); - fillProperties(definition.declaredInputs, superDef.declaredInputs); - fillProperties(definition.outputs, superDef.outputs); + // Merge inputs and outputs + fillProperties(definition.inputs, superDef.inputs); + fillProperties(definition.declaredInputs, superDef.declaredInputs); + fillProperties(definition.outputs, superDef.outputs); - // Inherit hooks - // Assume super class inheritance feature has already run. - definition.afterContentChecked = - definition.afterContentChecked || superDef.afterContentChecked; - definition.afterContentInit = definition.afterContentInit || superDef.afterContentInit; - definition.afterViewChecked = definition.afterViewChecked || superDef.afterViewChecked; - definition.afterViewInit = definition.afterViewInit || superDef.afterViewInit; - definition.doCheck = definition.doCheck || superDef.doCheck; - definition.onDestroy = definition.onDestroy || superDef.onDestroy; - definition.onInit = definition.onInit || superDef.onInit; + // Inherit hooks + // Assume super class inheritance feature has already run. + definition.afterContentChecked = + definition.afterContentChecked || superDef.afterContentChecked; + definition.afterContentInit = definition.afterContentInit || superDef.afterContentInit; + definition.afterViewChecked = definition.afterViewChecked || superDef.afterViewChecked; + definition.afterViewInit = definition.afterViewInit || superDef.afterViewInit; + definition.doCheck = definition.doCheck || superDef.doCheck; + definition.onDestroy = definition.onDestroy || superDef.onDestroy; + definition.onInit = definition.onInit || superDef.onInit; + } // Run parent features const features = superDef.features; @@ -81,6 +84,16 @@ export function ɵɵInheritDefinitionFeature(definition: DirectiveDef| Comp if (feature && feature.ngInherit) { (feature as DirectiveDefFeature)(definition); } + // If `InheritDefinitionFeature` is a part of the current `superDef`, it means that this + // def already has all the necessary information inherited from its super class(es), so we + // can stop merging fields from super classes. However we need to iterate through the + // prototype chain to look for classes that might contain other "features" (like + // NgOnChanges), which we should invoke for the original `definition`. We set the + // `shouldInheritFields` flag to indicate that, essentially skipping fields inheritance + // logic and only invoking functions from the "features" list. + if (feature === ɵɵInheritDefinitionFeature) { + shouldInheritFields = false; + } } } } @@ -104,7 +117,6 @@ function maybeUnwrapEmpty(value: any): any { function inheritViewQuery( definition: DirectiveDef| ComponentDef, superViewQuery: ViewQueriesFunction) { const prevViewQuery = definition.viewQuery; - if (prevViewQuery) { definition.viewQuery = (rf, ctx) => { superViewQuery(rf, ctx); @@ -119,7 +131,6 @@ function inheritContentQueries( definition: DirectiveDef| ComponentDef, superContentQueries: ContentQueriesFunction) { const prevContentQueries = definition.contentQueries; - if (prevContentQueries) { definition.contentQueries = (rf, ctx, directiveIndex) => { superContentQueries(rf, ctx, directiveIndex); @@ -134,17 +145,12 @@ function inheritHostBindings( definition: DirectiveDef| ComponentDef, superHostBindings: HostBindingsFunction) { const prevHostBindings = definition.hostBindings; - // If the subclass does not have a host bindings function, we set the subclass host binding - // function to be the superclass's (in this feature). We should check if they're the same here - // to ensure we don't inherit it twice. - if (superHostBindings !== prevHostBindings) { - if (prevHostBindings) { - definition.hostBindings = (rf: RenderFlags, ctx: any, elementIndex: number) => { - superHostBindings(rf, ctx, elementIndex); - prevHostBindings(rf, ctx, elementIndex); - }; - } else { - definition.hostBindings = superHostBindings; - } + if (prevHostBindings) { + definition.hostBindings = (rf: RenderFlags, ctx: any, elementIndex: number) => { + superHostBindings(rf, ctx, elementIndex); + prevHostBindings(rf, ctx, elementIndex); + }; + } else { + definition.hostBindings = superHostBindings; } } diff --git a/packages/core/test/acceptance/inherit_definition_feature_spec.ts b/packages/core/test/acceptance/inherit_definition_feature_spec.ts index e963049643..85d953dc74 100644 --- a/packages/core/test/acceptance/inherit_definition_feature_spec.ts +++ b/packages/core/test/acceptance/inherit_definition_feature_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Component, ContentChildren, Directive, EventEmitter, HostBinding, Input, OnChanges, Output, QueryList, ViewChildren} from '@angular/core'; +import {Component, ContentChildren, Directive, EventEmitter, HostBinding, HostListener, Input, OnChanges, Output, QueryList, ViewChildren} from '@angular/core'; import {TestBed} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; import {onlyInIvy} from '@angular/private/testing'; @@ -4225,6 +4225,71 @@ describe('inheritance', () => { expect(foundQueryList !.length).toBe(5); }); + it('should inherit host listeners from base class once', () => { + const events: string[] = []; + + @Component({ + selector: 'app-base', + template: 'base', + }) + class BaseComponent { + @HostListener('click') + clicked() { events.push('BaseComponent.clicked'); } + } + + @Component({ + selector: 'app-child', + template: 'child', + }) + class ChildComponent extends BaseComponent { + // additional host listeners are defined here to have `hostBindings` function generated on + // component def, which would trigger `hostBindings` functions merge operation in + // InheritDefinitionFeature logic (merging Child and Base host binding functions) + @HostListener('focus') + focused() {} + + clicked() { events.push('ChildComponent.clicked'); } + } + + @Component({ + selector: 'app-grand-child', + template: 'grand-child', + }) + class GrandChildComponent extends ChildComponent { + // additional host listeners are defined here to have `hostBindings` function generated on + // component def, which would trigger `hostBindings` functions merge operation in + // InheritDefinitionFeature logic (merging GrandChild and Child host binding functions) + @HostListener('blur') + blurred() {} + + clicked() { events.push('GrandChildComponent.clicked'); } + } + + @Component({ + selector: 'root-app', + template: ` + + + + `, + }) + class RootApp { + } + + const components = [BaseComponent, ChildComponent, GrandChildComponent]; + TestBed.configureTestingModule({ + declarations: [RootApp, ...components], + }); + const fixture = TestBed.createComponent(RootApp); + fixture.detectChanges(); + + components.forEach(component => { + fixture.debugElement.query(By.directive(component)).nativeElement.click(); + }); + expect(events).toEqual( + ['BaseComponent.clicked', 'ChildComponent.clicked', 'GrandChildComponent.clicked']); + }); + xdescribe( 'what happens when...', () => {