cargdev/angular
1
0
Fork 0
Code Issues 5 Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
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 963ed711d8
commit 22533fb5b4
2 changed files with 114 additions and 43 deletions
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;
}
}