honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ivy): skip field inheritance if InheritDefinitionFeature is present on parent def (#34244) 

The main logic of the `InheritDefinitionFeature` is to go through the prototype chain of a given Component and merge all Angular-specific information onto that Component def. The problem happens in case there is a Component in a hierarchy that also contains the `InheritDefinitionFeature` (i.e. it extends some other Component), so it inherits all Angular-specific information from its super class. As a result, the root Component may end up having duplicate information inherited from different Components in hierarchy.

Let's consider the following structure: `GrandChild` extends `Child` that extends `Base` and the `Base` class has a `HostListener`. In this scenario `GrandChild` and `Child` will have `InheritDefinitionFeature` included into the `features` list. The processing will happend in the following order:

- `Child` inherits `HostListener` from the `Base` class
- `GrandChild` inherits `HostListener` from the `Child` class
- since `Child` has a parent, `GrandChild` also inherits from the `Base` class

The result is that the `GrandChild` def has duplicated host listener, which is not correct.

This commit introduces additional logic that checks whether we came across a def that has `InheritDefinitionFeature` feature (which means that this def already inherited information from its super classes). If that's the case, we skip further fields-related inheritance logic, but keep going though the prototype chain to look for super classes that contain other features (like NgOnChanges), that we need to invoke for a given Component def.

PR Close #34244
This commit is contained in:
Andrew Kushnir 2019-12-04 23:21:59 -08:00 committed by Alex Rickabaugh
parent 5b864ede13
commit effb92dfae
2 changed files with 114 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

90
packages/core/src/render3/features/inherit_definition_feature.ts
View File

@ -25,6 +25,7 @@ export function getSuperType(type: Type<any>): Type<any>&
*/
export function ɵɵInheritDefinitionFeature(definition: DirectiveDef<any>| ComponentDef<any>): void {
let superType = getSuperType(definition.type);
let shouldInheritFields = true;
while (superType) {
let superDef: DirectiveDef<any>|ComponentDef<any>|undefined = undefined;
@ -40,38 +41,40 @@ export function ɵɵInheritDefinitionFeature(definition: DirectiveDef<any>| 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<any>| 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<any>| ComponentDef<any>, superViewQuery: ViewQueriesFunction<any>) {
const prevViewQuery = definition.viewQuery;
if (prevViewQuery) {
definition.viewQuery = (rf, ctx) => {
superViewQuery(rf, ctx);
@ -119,7 +131,6 @@ function inheritContentQueries(
definition: DirectiveDef<any>| ComponentDef<any>,
superContentQueries: ContentQueriesFunction<any>) {
const prevContentQueries = definition.contentQueries;
if (prevContentQueries) {
definition.contentQueries = (rf, ctx, directiveIndex) => {
superContentQueries(rf, ctx, directiveIndex);
@ -134,17 +145,12 @@ function inheritHostBindings(
definition: DirectiveDef<any>| ComponentDef<any>,
superHostBindings: HostBindingsFunction<any>) {
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;
}
}

67
packages/core/test/acceptance/inherit_definition_feature_spec.ts
View File

@ -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: `
<app-base></app-base>
<app-child></app-child>
<app-grand-child></app-grand-child>
`,
})
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...',
() => {