Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ivy): listeners inherited twice if sub class has own propMetadata #29353

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 12 additions & 2 deletions packages/core/src/render3/metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,17 @@ export function setClassMetadata(
type: Type<any>, decorators: any[] | null, ctorParameters: (() => any[]) | null,
propDecorators: {[field: string]: any} | null): void {
const clazz = type as TypeWithMetadata;

// We determine whether a class has its own metadata by taking the metadata from the parent
// constructor and checking whether it's the same as the subclass metadata below. We can't use
// `hasOwnProperty` here because it doesn't work correctly in IE10 for static fields that are
// defined by TS. See https://github.com/angular/angular/pull/28439#issuecomment-459349218.
const parentPrototype = clazz.prototype ? Object.getPrototypeOf(clazz.prototype) : null;
const parentConstructor: TypeWithMetadata|null = parentPrototype && parentPrototype.constructor;

if (decorators !== null) {
if (clazz.hasOwnProperty('decorators') && clazz.decorators !== undefined) {
if (clazz.decorators !== undefined &&
(!parentConstructor || parentConstructor.decorators !== clazz.decorators)) {
clazz.decorators.push(...decorators);
} else {
clazz.decorators = decorators;
Expand All @@ -45,7 +54,8 @@ export function setClassMetadata(
// decorator types. Decorators on individual fields are not merged, as it's also incredibly
// unlikely that a field will be decorated both with an Angular decorator and a non-Angular
// decorator that's also been downleveled.
if (clazz.propDecorators !== undefined) {
if (clazz.propDecorators !== undefined &&
(!parentConstructor || parentConstructor.propDecorators !== clazz.propDecorators)) {
clazz.propDecorators = {...clazz.propDecorators, ...propDecorators};
} else {
clazz.propDecorators = propDecorators;
Expand Down
26 changes: 25 additions & 1 deletion packages/core/test/render3/jit/directive_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Directive, HostListener} from '@angular/core';
import {Directive, HostListener, Input} from '@angular/core';
import {setClassMetadata} from '@angular/core/src/render3/metadata';

import {convertToR3QueryMetadata, directiveMetadata, extendsDirectlyFromObject} from '../../../src/render3/jit/directive';
Expand Down Expand Up @@ -130,5 +130,29 @@ describe('jit directive helper functions', () => {
expect(directiveMetadata(SuperDirective, {}).propMetadata.handleClick).toBeFalsy();
expect(directiveMetadata(SubDirective, {}).propMetadata.handleClick).toBeFalsy();
});

it('should not inherit propMetadata from super class when sub class has its own propMetadata',
() => {
class SuperDirective {}
setClassMetadata(SuperDirective, [{type: Directive}], null, {
someInput: [{type: Input}],
handleClick: [{type: HostListener, args: ['click', ['$event']]}]
});

class SubDirective extends SuperDirective {}
setClassMetadata(
SubDirective, [{type: Directive}], null, {someOtherInput: [{type: Input}]});

const superPropMetadata = directiveMetadata(SuperDirective, {}).propMetadata;
const subPropMetadata = directiveMetadata(SubDirective, {}).propMetadata;

expect(superPropMetadata.handleClick).toBeTruthy();
expect(superPropMetadata.someInput).toBeTruthy();

expect(subPropMetadata.handleClick).toBeFalsy();
expect(subPropMetadata.someInput).toBeFalsy();
expect(subPropMetadata.someOtherInput).toBeTruthy();
});

});
});