diff --git a/packages/core/src/render3/jit/module.ts b/packages/core/src/render3/jit/module.ts index 4773018e48..f706d178c8 100644 --- a/packages/core/src/render3/jit/module.ts +++ b/packages/core/src/render3/jit/module.ts @@ -345,9 +345,12 @@ function setScopeOnDeclaredComponents(moduleType: Type, ngModule: NgModule) */ export function patchComponentDefWithScope( componentDef: ComponentDef, transitiveScopes: NgModuleTransitiveScopes) { - componentDef.directiveDefs = () => Array.from(transitiveScopes.compilation.directives) - .map(dir => getDirectiveDef(dir) || getComponentDef(dir) !) - .filter(def => !!def); + componentDef.directiveDefs = () => + Array.from(transitiveScopes.compilation.directives) + .map( + dir => dir.hasOwnProperty(NG_COMPONENT_DEF) ? getComponentDef(dir) ! : + getDirectiveDef(dir) !) + .filter(def => !!def); componentDef.pipeDefs = () => Array.from(transitiveScopes.compilation.pipes).map(pipe => getPipeDef(pipe) !); } diff --git a/packages/core/test/linker/inheritance_integration_spec.ts b/packages/core/test/linker/inheritance_integration_spec.ts new file mode 100644 index 0000000000..24f7129079 --- /dev/null +++ b/packages/core/test/linker/inheritance_integration_spec.ts @@ -0,0 +1,93 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {Component, Directive, HostBinding} from '@angular/core'; +import {TestBed} from '@angular/core/testing'; +import {describe, expect, it} from '@angular/core/testing/src/testing_internal'; +import {modifiedInIvy, onlyInIvy} from '@angular/private/testing'; + +@Directive({selector: '[directiveA]'}) +class DirectiveA { +} + +@Directive({selector: '[directiveB]'}) +class DirectiveB { + @HostBinding('title') + title = 'DirectiveB Title'; +} + +@Component({selector: 'component-a', template: 'ComponentA Template'}) +class ComponentA { +} + +@Component( + {selector: 'component-extends-directive', template: 'ComponentExtendsDirective Template'}) +class ComponentExtendsDirective extends DirectiveA { +} + +class ComponentWithNoAnnotation extends ComponentA {} + +@Directive({selector: '[directiveExtendsComponent]'}) +class DirectiveExtendsComponent extends ComponentA { + @HostBinding('title') + title = 'DirectiveExtendsComponent Title'; +} + +class DirectiveWithNoAnnotation extends DirectiveB {} + +@Component({selector: 'my-app', template: '...'}) +class App { +} + +describe('Inheritance logic', () => { + it('should handle Components that extend Directives', () => { + TestBed.configureTestingModule({declarations: [ComponentExtendsDirective, App]}); + const template = ''; + TestBed.overrideComponent(App, {set: {template}}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + expect(fixture.nativeElement.firstChild.innerHTML).toBe('ComponentExtendsDirective Template'); + }); + + it('should handle classes with no annotations that extend Components', () => { + TestBed.configureTestingModule({declarations: [ComponentWithNoAnnotation, App]}); + const template = ''; + TestBed.overrideComponent(App, {set: {template}}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + expect(fixture.nativeElement.firstChild.innerHTML).toBe('ComponentA Template'); + }); + + it('should handle classes with no annotations that extend Directives', () => { + TestBed.configureTestingModule({declarations: [DirectiveWithNoAnnotation, App]}); + const template = '
'; + TestBed.overrideComponent(App, {set: {template}}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + expect(fixture.nativeElement.firstChild.title).toBe('DirectiveB Title'); + }); + + modifiedInIvy('View Engine allows Directives to extend Components') + .it('should handle Directives that extend Components', () => { + TestBed.configureTestingModule({declarations: [DirectiveExtendsComponent, App]}); + const template = '
Some content
'; + TestBed.overrideComponent(App, {set: {template}}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + expect(fixture.nativeElement.firstChild.title).toBe('DirectiveExtendsComponent Title'); + }); + + onlyInIvy('Ivy does not allow Directives to extend Components') + .it('should throw in case a Directive tries to extend a Component', () => { + TestBed.configureTestingModule({declarations: [DirectiveExtendsComponent, App]}); + const template = '
Some content
'; + TestBed.overrideComponent(App, {set: {template}}); + expect(() => TestBed.createComponent(App)) + .toThrowError('Directives cannot inherit Components'); + }); +}); diff --git a/packages/core/test/test_bed_spec.ts b/packages/core/test/test_bed_spec.ts index d1a0d2f3ae..07e2445fe4 100644 --- a/packages/core/test/test_bed_spec.ts +++ b/packages/core/test/test_bed_spec.ts @@ -257,42 +257,67 @@ describe('TestBed', () => { }); 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}) - class SomePipe { - transform(value: string): string { return `transformed ${value}`; } - } + .describe('resetting ng defs', () => { + it('should restore ng defs to their initial states', () => { + @Pipe({name: 'somePipe', pure: true}) + class SomePipe { + transform(value: string): string { return `transformed ${value}`; } + } - @Directive({selector: 'someDirective'}) - class SomeDirective { - someProp = 'hello'; - } + @Directive({selector: 'someDirective'}) + class SomeDirective { + someProp = 'hello'; + } - @Component({selector: 'comp', template: 'someText'}) - class SomeComponent { - } + @Component({selector: 'comp', template: 'someText'}) + class SomeComponent { + } - @NgModule({declarations: [SomeComponent]}) - class SomeModule { - } + @NgModule({declarations: [SomeComponent]}) + class SomeModule { + } - TestBed.configureTestingModule({imports: [SomeModule]}); + TestBed.configureTestingModule({imports: [SomeModule]}); - // adding Pipe and Directive via metadata override - TestBed.overrideModule( - SomeModule, {set: {declarations: [SomeComponent, SomePipe, SomeDirective]}}); - TestBed.overrideComponent( - SomeComponent, {set: {template: `{{'hello' | somePipe}}`}}); - TestBed.createComponent(SomeComponent); + // adding Pipe and Directive via metadata override + TestBed.overrideModule( + SomeModule, {set: {declarations: [SomeComponent, SomePipe, SomeDirective]}}); + TestBed.overrideComponent( + SomeComponent, + {set: {template: `{{'hello' | somePipe}}`}}); + TestBed.createComponent(SomeComponent); - const defBeforeReset = (SomeComponent as any).ngComponentDef; - expect(defBeforeReset.pipeDefs().length).toEqual(1); - expect(defBeforeReset.directiveDefs().length).toEqual(2); // directive + component + const defBeforeReset = (SomeComponent as any).ngComponentDef; + expect(defBeforeReset.pipeDefs().length).toEqual(1); + expect(defBeforeReset.directiveDefs().length).toEqual(2); // directive + component - TestBed.resetTestingModule(); + TestBed.resetTestingModule(); - const defAfterReset = (SomeComponent as any).ngComponentDef; - expect(defAfterReset.pipeDefs().length).toEqual(0); - expect(defAfterReset.directiveDefs().length).toEqual(1); // component + const defAfterReset = (SomeComponent as any).ngComponentDef; + expect(defAfterReset.pipeDefs().length).toEqual(0); + expect(defAfterReset.directiveDefs().length).toEqual(1); // component + }); + + it('should cleanup ng defs for classes with no ng annotations (in case of inheritance)', + () => { + @Component({selector: 'someDirective', template: '...'}) + class SomeComponent { + } + + class ComponentWithNoAnnotations extends SomeComponent {} + + TestBed.configureTestingModule({declarations: [ComponentWithNoAnnotations]}); + TestBed.createComponent(ComponentWithNoAnnotations); + + expect(ComponentWithNoAnnotations.hasOwnProperty('ngComponentDef')).toBeTruthy(); + expect(SomeComponent.hasOwnProperty('ngComponentDef')).toBeTruthy(); + + TestBed.resetTestingModule(); + + expect(ComponentWithNoAnnotations.hasOwnProperty('ngComponentDef')).toBeFalsy(); + + // ngComponentDef should be preserved on super component + expect(SomeComponent.hasOwnProperty('ngComponentDef')).toBeTruthy(); + }); }); }); diff --git a/packages/core/testing/src/r3_test_bed.ts b/packages/core/testing/src/r3_test_bed.ts index 442e7b8ff1..3dcf1f12a0 100644 --- a/packages/core/testing/src/r3_test_bed.ts +++ b/packages/core/testing/src/r3_test_bed.ts @@ -336,7 +336,18 @@ export class TestBedRender3 implements Injector, TestBed { // restore initial component/directive/pipe defs this._initiaNgDefs.forEach((value: [string, PropertyDescriptor], type: Type) => { - Object.defineProperty(type, value[0], value[1]); + const [prop, descriptor] = value; + if (!descriptor) { + // Delete operations are generally undesirable since they have performance implications on + // objects they were applied to. In this particular case, situations where this code is + // invoked should be quite rare to cause any noticable impact, since it's applied only to + // some test cases (for example when class with no annotations extends some @Component) when + // we need to clear 'ngComponentDef' field on a given class to restore its original state + // (before applying overrides and running tests). + delete (type as any)[prop]; + } else { + Object.defineProperty(type, prop, descriptor); + } }); this._initiaNgDefs.clear(); clearResolutionOfComponentResourcesQueue(); diff --git a/packages/core/testing/src/resolvers.ts b/packages/core/testing/src/resolvers.ts index bacbe78273..027929078e 100644 --- a/packages/core/testing/src/resolvers.ts +++ b/packages/core/testing/src/resolvers.ts @@ -37,9 +37,21 @@ abstract class OverrideResolver implements Resolver { } getAnnotation(type: Type): T|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; + const annotations = reflection.annotations(type); + // Try to find the nearest known Type annotation and make sure that this annotation is an + // instance of the type we are looking for, so we can use it for resolution. Note: there might + // be multiple known annotations found due to the fact that Components can extend Directives (so + // both Directive and Component annotations would be present), so we always check if the known + // annotation has the right type. + for (let i = annotations.length - 1; i >= 0; i--) { + const annotation = annotations[i]; + const isKnownType = annotation instanceof Directive || annotation instanceof Component || + annotation instanceof Pipe || annotation instanceof NgModule; + if (isKnownType) { + return annotation instanceof this.type ? annotation : null; + } + } + return null; } resolve(type: Type): T|null {