From ca2462cff726ce334fa1d14eb1e7a35737b9e69d Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Tue, 16 Apr 2019 16:58:44 -0700 Subject: [PATCH] fix(ivy): support providing components and dirs in tests (#29945) Previous to this commit, providing a component or directive in a test module without @Injectable() would throw because the injectable factory would not be found. Providing components in tests in addition to declaring or importing them is not necessary, but it should not throw an error. This commit ensures factory data is saved when component defs and directive defs are created, which allows them to be processed by the module injector. Note that bootstrapping is still required for this setup to work because directiveInject() does not support cases where the view has not been created. This case will be handled in a future commit. PR Close #29945 --- integration/_payload-limits.json | 2 +- packages/core/src/render3/definition.ts | 11 +++ packages/core/src/render3/jit/directive.ts | 12 +++ .../core/test/acceptance/providers_spec.ts | 95 ++++++++++++++++++- .../cyclic_import/bundle.golden_symbols.json | 6 ++ .../hello_world/bundle.golden_symbols.json | 6 ++ .../inherit_definition_feature_spec.ts | 50 ---------- 7 files changed, 129 insertions(+), 53 deletions(-) diff --git a/integration/_payload-limits.json b/integration/_payload-limits.json index 2d9ba4c9b0..c35f27442b 100644 --- a/integration/_payload-limits.json +++ b/integration/_payload-limits.json @@ -12,7 +12,7 @@ "master": { "uncompressed": { "runtime": 1440, - "main": 14287, + "main": 14487, "polyfills": 43567 } } diff --git a/packages/core/src/render3/definition.ts b/packages/core/src/render3/definition.ts index 5c1fe79af0..8bc33fd507 100644 --- a/packages/core/src/render3/definition.ts +++ b/packages/core/src/render3/definition.ts @@ -9,6 +9,7 @@ import '../util/ng_dev_mode'; import {ChangeDetectionStrategy} from '../change_detection/constants'; +import {NG_INJECTABLE_DEF, ɵɵdefineInjectable} from '../di/interface/defs'; import {Mutable, Type} from '../interface/type'; import {NgModuleDef} from '../metadata/ng_module'; import {SchemaMetadata} from '../metadata/schema'; @@ -300,7 +301,17 @@ export function ɵɵdefineComponent(componentDefinition: { def.pipeDefs = pipeTypes ? () => (typeof pipeTypes === 'function' ? pipeTypes() : pipeTypes).map(extractPipeDef) : null; + + // Add ngInjectableDef so components are reachable through the module injector by default + // (unless it has already been set by the @Injectable decorator). This is mostly to + // support injecting components in tests. In real application code, components should + // be retrieved through the node injector, so this isn't a problem. + if (!type.hasOwnProperty(NG_INJECTABLE_DEF)) { + (type as any)[NG_INJECTABLE_DEF] = + ɵɵdefineInjectable({factory: componentDefinition.factory as() => T}); + } }) as never; + return def as never; } diff --git a/packages/core/src/render3/jit/directive.ts b/packages/core/src/render3/jit/directive.ts index e59f8017ef..6306a5bc94 100644 --- a/packages/core/src/render3/jit/directive.ts +++ b/packages/core/src/render3/jit/directive.ts @@ -9,6 +9,7 @@ import {R3DirectiveMetadataFacade, getCompilerFacade} from '../../compiler/compiler_facade'; import {R3ComponentMetadataFacade, R3QueryMetadataFacade} from '../../compiler/compiler_facade_interface'; import {resolveForwardRef} from '../../di/forward_ref'; +import {compileInjectable} from '../../di/jit/injectable'; import {getReflect, reflectDependencies} from '../../di/jit/util'; import {Type} from '../../interface/type'; import {Query} from '../../metadata/di'; @@ -93,6 +94,12 @@ export function compileComponent(type: Type, metadata: Component): void { // Make the property configurable in dev mode to allow overriding in tests configurable: !!ngDevMode, }); + + + // Add ngInjectableDef so components are reachable through the module injector by default + // This is mostly to support injecting components in tests. In real application code, + // components should be retrieved through the node injector, so this isn't a problem. + compileInjectable(type); } function hasSelectorScope(component: Type): component is Type& @@ -125,6 +132,11 @@ export function compileDirective(type: Type, directive: Directive): void { // Make the property configurable in dev mode to allow overriding in tests configurable: !!ngDevMode, }); + + // Add ngInjectableDef so directives are reachable through the module injector by default + // This is mostly to support injecting directives in tests. In real application code, + // directives should be retrieved through the node injector, so this isn't a problem. + compileInjectable(type); } export function extendsDirectlyFromObject(type: Type): boolean { diff --git a/packages/core/test/acceptance/providers_spec.ts b/packages/core/test/acceptance/providers_spec.ts index e5c6d0bd4b..9f91576bb8 100644 --- a/packages/core/test/acceptance/providers_spec.ts +++ b/packages/core/test/acceptance/providers_spec.ts @@ -6,12 +6,54 @@ * found in the LICENSE file at https://angular.io/license */ -import {Component, Injectable} from '@angular/core'; +import {Component, Directive, Inject, Injectable, InjectionToken} from '@angular/core'; import {TestBed} from '@angular/core/testing'; +import {By} from '@angular/platform-browser'; import {onlyInIvy} from '@angular/private/testing'; - describe('providers', () => { + + describe('inheritance', () => { + + it('should NOT inherit providers', () => { + const SOME_DIRS = new InjectionToken('someDirs'); + + @Directive({ + selector: '[super-dir]', + providers: [{provide: SOME_DIRS, useClass: SuperDirective, multi: true}] + }) + class SuperDirective { + } + + @Directive({ + selector: '[sub-dir]', + providers: [{provide: SOME_DIRS, useClass: SubDirective, multi: true}] + }) + class SubDirective extends SuperDirective { + } + + @Directive({selector: '[other-dir]'}) + class OtherDirective { + constructor(@Inject(SOME_DIRS) public dirs: any) {} + } + + @Component({selector: 'app-comp', template: `
`}) + class App { + } + + TestBed.configureTestingModule( + {declarations: [SuperDirective, SubDirective, OtherDirective, App]}); + + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + + const otherDir = fixture.debugElement.query(By.css('div')).injector.get(OtherDirective); + expect(otherDir.dirs.length).toEqual(1); + expect(otherDir.dirs[0] instanceof SubDirective).toBe(true); + }); + + }); + describe('lifecycles', () => { it('should inherit ngOnDestroy hooks on providers', () => { const logs: string[] = []; @@ -181,4 +223,53 @@ describe('providers', () => { }); }); + + describe('components and directives', () => { + + class MyService { + value = 'some value'; + } + + @Component({selector: 'my-comp', template: ``}) + class MyComp { + constructor(public svc: MyService) {} + } + + @Directive({selector: '[some-dir]'}) + class MyDir { + constructor(public svc: MyService) {} + } + + it('should support providing components in tests without @Injectable', () => { + @Component({selector: 'test-comp', template: ''}) + class TestComp { + } + + TestBed.configureTestingModule({ + declarations: [TestComp, MyComp], + // providing MyComp is unnecessary but it shouldn't throw + providers: [MyComp, MyService], + }); + + const fixture = TestBed.createComponent(TestComp); + const myCompInstance = fixture.debugElement.query(By.css('my-comp')).injector.get(MyComp); + expect(myCompInstance.svc.value).toEqual('some value'); + }); + + it('should support providing directives in tests without @Injectable', () => { + @Component({selector: 'test-comp', template: '
'}) + class TestComp { + } + + TestBed.configureTestingModule({ + declarations: [TestComp, MyDir], + // providing MyDir is unnecessary but it shouldn't throw + providers: [MyDir, MyService], + }); + + const fixture = TestBed.createComponent(TestComp); + const myCompInstance = fixture.debugElement.query(By.css('div')).injector.get(MyDir); + expect(myCompInstance.svc.value).toEqual('some value'); + }); + }); }); diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json index acb85251e0..77cd3d5d65 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -83,6 +83,9 @@ { "name": "NG_ELEMENT_ID" }, + { + "name": "NG_INJECTABLE_DEF" + }, { "name": "NG_PIPE_DEF" }, @@ -680,6 +683,9 @@ { "name": "ɵɵdefineComponent" }, + { + "name": "ɵɵdefineInjectable" + }, { "name": "ɵɵdefineInjector" }, diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json index 99e9f7e146..0f92586a4d 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -71,6 +71,9 @@ { "name": "NG_ELEMENT_ID" }, + { + "name": "NG_INJECTABLE_DEF" + }, { "name": "NG_PIPE_DEF" }, @@ -491,6 +494,9 @@ { "name": "ɵɵdefineComponent" }, + { + "name": "ɵɵdefineInjectable" + }, { "name": "ɵɵnamespaceHTML" }, diff --git a/packages/core/test/render3/inherit_definition_feature_spec.ts b/packages/core/test/render3/inherit_definition_feature_spec.ts index 910878d2ff..b9155bea2d 100644 --- a/packages/core/test/render3/inherit_definition_feature_spec.ts +++ b/packages/core/test/render3/inherit_definition_feature_spec.ts @@ -636,54 +636,4 @@ describe('InheritDefinitionFeature', () => { }).toThrowError('Directives cannot inherit Components'); }); - it('should NOT inherit providers', () => { - let otherDir !: OtherDirective; - - const SOME_DIRS = new InjectionToken('someDirs'); - - // providers: [{ provide: SOME_DIRS, useClass: SuperDirective, multi: true }] - class SuperDirective { - static ngDirectiveDef = ɵɵdefineDirective({ - type: SuperDirective, - selectors: [['', 'superDir', '']], - factory: () => new SuperDirective(), - features: - [ɵɵProvidersFeature([{provide: SOME_DIRS, useClass: SuperDirective, multi: true}])], - }); - } - - // providers: [{ provide: SOME_DIRS, useClass: SubDirective, multi: true }] - class SubDirective extends SuperDirective { - static ngDirectiveDef = ɵɵdefineDirective({ - type: SubDirective, - selectors: [['', 'subDir', '']], - factory: () => new SubDirective(), - features: [ - ɵɵProvidersFeature([{provide: SOME_DIRS, useClass: SubDirective, multi: true}]), - ɵɵInheritDefinitionFeature - ], - }); - } - - class OtherDirective { - constructor(@Inject(SOME_DIRS) public dirs: any) {} - - static ngDirectiveDef = ɵɵdefineDirective({ - type: OtherDirective, - selectors: [['', 'otherDir', '']], - factory: () => otherDir = new OtherDirective(ɵɵdirectiveInject(SOME_DIRS)), - }); - } - - /**
*/ - const App = createComponent('app', (rf: RenderFlags, ctx: any) => { - if (rf & RenderFlags.Create) { - ɵɵelement(0, 'div', ['otherDir', '', 'subDir', '']); - } - }, 1, 0, [OtherDirective, SubDirective, SuperDirective]); - - const fixture = new ComponentFixture(App); - expect(otherDir.dirs.length).toEqual(1); - expect(otherDir.dirs[0] instanceof SubDirective).toBe(true); - }); });