From 1f7d3b9a57c8184f5d8bdd1a4779d641a5b57a81 Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Wed, 16 Jan 2019 16:28:04 -0800 Subject: [PATCH] fix(ivy): TestBed should use annotation for the last match rather than the first (#28195) When we look for matching annotations in TestBed, we should always take the last matching annotation. Otherwise, we will return superclass data for subclasses, which would have unintended consequences like directives matching the wrong selectors. PR Close #28195 --- packages/core/src/render3/metadata.ts | 2 +- packages/core/test/test_bed_spec.ts | 22 +++++++++++++++++++++- packages/core/testing/src/resolvers.ts | 4 +++- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/packages/core/src/render3/metadata.ts b/packages/core/src/render3/metadata.ts index bfa8e65a64..053d742845 100644 --- a/packages/core/src/render3/metadata.ts +++ b/packages/core/src/render3/metadata.ts @@ -28,7 +28,7 @@ export function setClassMetadata( propDecorators: {[field: string]: any} | null): void { const clazz = type as TypeWithMetadata; if (decorators !== null) { - if (clazz.decorators !== undefined) { + if (clazz.hasOwnProperty('decorators') && clazz.decorators !== undefined) { clazz.decorators.push(...decorators); } else { clazz.decorators = decorators; diff --git a/packages/core/test/test_bed_spec.ts b/packages/core/test/test_bed_spec.ts index 0492ecd2c0..2b055bc58d 100644 --- a/packages/core/test/test_bed_spec.ts +++ b/packages/core/test/test_bed_spec.ts @@ -48,8 +48,21 @@ export class SimpleCmp { export class WithRefsCmp { } +@Component({selector: 'inherited-cmp', template: 'inherited'}) +export class InheritedCmp extends SimpleCmp { +} + +@Component({ + selector: 'simple-app', + template: ` + - + ` +}) +export class SimpleApp { +} + @NgModule({ - declarations: [HelloWorld, SimpleCmp, WithRefsCmp], + declarations: [HelloWorld, SimpleCmp, WithRefsCmp, InheritedCmp, SimpleApp], imports: [GreetingModule], providers: [ {provide: NAME, useValue: 'World!'}, @@ -174,6 +187,13 @@ describe('TestBed', () => { expect(hello.nativeElement).toHaveText('Hello injected World !'); }); + it('should resolve components that are extended by other components', () => { + // SimpleApp uses SimpleCmp in its template, which is extended by InheritedCmp + const simpleApp = TestBed.createComponent(SimpleApp); + simpleApp.detectChanges(); + expect(simpleApp.nativeElement).toHaveText('simple - inherited'); + }); + onlyInIvy('patched ng defs should be removed after resetting TestingModule') .it('make sure we restore ng defs to their initial states', () => { @Pipe({name: 'somePipe', pure: true}) diff --git a/packages/core/testing/src/resolvers.ts b/packages/core/testing/src/resolvers.ts index 2a15dd7bc5..bacbe78273 100644 --- a/packages/core/testing/src/resolvers.ts +++ b/packages/core/testing/src/resolvers.ts @@ -37,7 +37,9 @@ abstract class OverrideResolver implements Resolver { } getAnnotation(type: Type): T|null { - return reflection.annotations(type).find(a => a instanceof this.type) || null; + // We should always return the last match from filter(), or we may return superclass data by + // mistake. + return reflection.annotations(type).filter(a => a instanceof this.type).pop() || null; } resolve(type: Type): T|null {