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.

Sign up for GitHub

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

Jump to bottom

feat(core): Introduce TestBed.inject to replace TestBed.get #32200

Closed
wants to merge 1 commit into from
Closed

feat(core): Introduce TestBed.inject to replace TestBed.get #32200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion aio/src/app/app.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ describe('AppComponent', () => {
});

it('should not be loaded/registered until necessary', () => {
const loader: TestElementsLoader = fixture.debugElement.injector.get(ElementsLoader);
const loader = fixture.debugElement.injector.get(ElementsLoader) as unknown as TestElementsLoader;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this change is needed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the "breaking changes" comment, I added AbstractType<T> to the "typed version" of Injector#get. Previously abstract classes would have returned any, the untyped version has been deprecated since Angular 4.x

This caused the returned value here to be type ElementsLoader which I thought I could cast directly to TestElementsLoader but they are apparently not fully compatible since the compiler complained about it

expect(loader.loadCustomElement).not.toHaveBeenCalled();

setHasFloatingToc(true);
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/application_ref.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,8 @@ export class ApplicationRef {
this.componentTypes.push(componentFactory.componentType);

// Create a factory associated with the current module if it's not bound to some other
const ngModule = isBoundToModule(componentFactory) ? null : this._injector.get(NgModuleRef);
const ngModule =
isBoundToModule(componentFactory) ? undefined : this._injector.get(NgModuleRef);
const selectorOrNode = rootSelectorOrNode || componentFactory.selector;
const compRef = componentFactory.create(Injector.NULL, [], selectorOrNode, ngModule);

Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/di/injector.ts
View file
Open in desktop
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 {Type} from '../interface/type';
import {AbstractType, Type} from '../interface/type';
import {stringify} from '../util/stringify';

import {resolveForwardRef} from './forward_ref';
Expand Down Expand Up @@ -55,7 +55,8 @@ export abstract class Injector {
* @returns The instance from the injector if defined, otherwise the `notFoundValue`.
* @throws When the `notFoundValue` is `undefined` or `Injector.THROW_IF_NOT_FOUND`.
*/
abstract get<T>(token: Type<T>|InjectionToken<T>, notFoundValue?: T, flags?: InjectFlags): T;
abstract get<T>(
token: Type<T>|InjectionToken<T>|AbstractType<T>, notFoundValue?: T, flags?: InjectFlags): T;
/**
* @deprecated from v4.0.0 use Type<T> or InjectionToken<T>
* @suppress {duplicate}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/render3/view_container_ref_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ describe('ViewContainerRef', () => {
const changeDetector = ref.injector.get(ChangeDetectorRef);
changeDetector.detectChanges();
expect(dynamicComp.doCheckCount).toEqual(1);
expect(changeDetector.context).toEqual(dynamicComp);
expect((changeDetector as any).context).toEqual(dynamicComp);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change tells me that this is a painful breaking change that will likely affect lots of code.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this is Injector.get, not TestBed, the change here is that we're now taking abstract classes in consideration to return the right type instead of returning any which is the deprecated behavior since Angular 4.x

I actually don't expect many breakages from Injector.get

Copy link
Author

@Goodwine Goodwine Aug 23, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, this only broke 5-6 targets on google3, affecting about 10-15 files which I already fixed.

});

it('should not throw when destroying a reattached component', () => {
Expand Down
64 changes: 44 additions & 20 deletions packages/core/testing/src/r3_test_bed.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
// this statement only.
// clang-format off
import {
AbstractType,
Component,
Directive,
InjectFlags,
Expand Down Expand Up @@ -49,7 +50,7 @@ const UNDEFINED: Symbol = Symbol('UNDEFINED');
* Note: Use `TestBed` in tests. It will be set to either `TestBedViewEngine` or `TestBedRender3`
* according to the compiler used.
*/
export class TestBedRender3 implements Injector, TestBed {
export class TestBedRender3 implements TestBed {
/**
* Initialize the environment for testing with a compiler factory, a PlatformRef, and an
* angular module. These are common to every test in the suite.
Expand Down Expand Up @@ -150,15 +151,26 @@ export class TestBedRender3 implements Injector, TestBed {
return TestBedRender3 as any as TestBedStatic;
}

static inject<T>(
token: Type<T>|InjectionToken<T>|AbstractType<T>, notFoundValue?: T, flags?: InjectFlags): T;
static inject<T>(
token: Type<T>|InjectionToken<T>|AbstractType<T>, notFoundValue: null, flags?: InjectFlags): T
|null;
static inject<T>(
token: Type<T>|InjectionToken<T>|AbstractType<T>, notFoundValue?: T|null,
flags?: InjectFlags): T|null {
return _getTestBedRender3().inject(token, notFoundValue, flags);
}

/** TODO(goodwine): Mark as deprecated from v9.0.0 use TestBed.inject */
static get<T>(token: Type<T>|InjectionToken<T>, notFoundValue?: T, flags?: InjectFlags): any;
/**
* @deprecated from v8.0.0 use Type<T> or InjectionToken<T>
*/
/** TODO(goodwine): Mark as deprecated from v9.0.0 use TestBed.inject */
static get(token: any, notFoundValue?: any): any;
/** TODO(goodwine): Mark as deprecated from v9.0.0 use TestBed.inject */
static get(
token: any, notFoundValue: any = Injector.THROW_IF_NOT_FOUND,
flags: InjectFlags = InjectFlags.Default): any {
return _getTestBedRender3().get(token, notFoundValue);
return _getTestBedRender3().inject(token, notFoundValue, flags);
}

static createComponent<T>(component: Type<T>): ComponentFixture<T> {
Expand Down Expand Up @@ -244,22 +256,33 @@ export class TestBedRender3 implements Injector, TestBed {

compileComponents(): Promise<any> { return this.compiler.compileComponents(); }

inject<T>(
token: Type<T>|InjectionToken<T>|AbstractType<T>, notFoundValue?: T, flags?: InjectFlags): T;
inject<T>(
token: Type<T>|InjectionToken<T>|AbstractType<T>, notFoundValue: null, flags?: InjectFlags): T
|null;
inject<T>(
token: Type<T>|InjectionToken<T>|AbstractType<T>, notFoundValue?: T|null,
flags?: InjectFlags): T|null {
if (token as unknown === TestBedRender3) {
return this as any;
}
const result = this.testModuleRef.injector.get(token, UNDEFINED, flags);
return result === UNDEFINED ? this.compiler.injector.get(token, notFoundValue, flags) : result;
}

/** TODO(goodwine): Mark as deprecated from v9.0.0 use TestBed.inject */
get<T>(token: Type<T>|InjectionToken<T>, notFoundValue?: T, flags?: InjectFlags): any;
/**
* @deprecated from v8.0.0 use Type<T> or InjectionToken<T>
*/
/** TODO(goodwine): Mark as deprecated from v9.0.0 use TestBed.inject */
get(token: any, notFoundValue?: any): any;
/** TODO(goodwine): Mark as deprecated from v9.0.0 use TestBed.inject */
get(token: any, notFoundValue: any = Injector.THROW_IF_NOT_FOUND,
flags: InjectFlags = InjectFlags.Default): any {
if (token === TestBedRender3) {
return this;
}
const result = this.testModuleRef.injector.get(token, UNDEFINED, flags);
return result === UNDEFINED ? this.compiler.injector.get(token, notFoundValue, flags) : result;
return this.inject(token, notFoundValue, flags);
}

execute(tokens: any[], fn: Function, context?: any): any {
const params = tokens.map(t => this.get(t));
const params = tokens.map(t => this.inject(t));
return fn.apply(context, params);
}

Expand Down Expand Up @@ -299,7 +322,7 @@ export class TestBedRender3 implements Injector, TestBed {
}

createComponent<T>(type: Type<T>): ComponentFixture<T> {
const testComponentRenderer: TestComponentRenderer = this.get(TestComponentRenderer);
const testComponentRenderer = this.inject(TestComponentRenderer);
const rootElId = `root-ng-internal-isolated-${_nextRootElementId++}`;
testComponentRenderer.insertRootElement(rootElId);

Expand All @@ -310,11 +333,12 @@ export class TestBedRender3 implements Injector, TestBed {
`It looks like '${stringify(type)}' has not been IVY compiled - it has no 'ngComponentDef' field`);
}

// TODO: Don't cast as `any`, proper type is boolean[]
const noNgZone = this.get(ComponentFixtureNoNgZone as any, false);
// TODO: Don't cast as `any`, proper type is boolean[]
const autoDetect: boolean = this.get(ComponentFixtureAutoDetect as any, false);
const ngZone: NgZone|null = noNgZone ? null : this.get(NgZone as Type<NgZone|null>, null);
// TODO: Don't cast as `InjectionToken<boolean>`, proper type is boolean[]
const noNgZone = this.inject(ComponentFixtureNoNgZone as InjectionToken<boolean>, false);
// TODO: Don't cast as `InjectionToken<boolean>`, proper type is boolean[]
const autoDetect: boolean =
this.inject(ComponentFixtureAutoDetect as InjectionToken<boolean>, false);
const ngZone: NgZone|null = noNgZone ? null : this.inject(NgZone, null);
const componentFactory = new ComponentFactory(componentDef);
const initComponent = () => {
const componentRef =
Expand Down
87 changes: 55 additions & 32 deletions packages/core/testing/src/test_bed.ts
View file
Open in desktop
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 {ApplicationInitStatus, CompilerOptions, Component, Directive, InjectFlags, InjectionToken, Injector, NgModule, NgModuleFactory, NgModuleRef, NgZone, Optional, Pipe, PlatformRef, Provider, SchemaMetadata, SkipSelf, StaticProvider, Type, ɵAPP_ROOT as APP_ROOT, ɵDepFlags as DepFlags, ɵNodeFlags as NodeFlags, ɵclearOverrides as clearOverrides, ɵgetInjectableDef as getInjectableDef, ɵivyEnabled as ivyEnabled, ɵoverrideComponentView as overrideComponentView, ɵoverrideProvider as overrideProvider, ɵstringify as stringify, ɵɵInjectableDef} from '@angular/core';
import {AbstractType, ApplicationInitStatus, CompilerOptions, Component, Directive, InjectFlags, InjectionToken, Injector, NgModule, NgModuleFactory, NgModuleRef, NgZone, Optional, Pipe, PlatformRef, Provider, SchemaMetadata, SkipSelf, StaticProvider, Type, ɵAPP_ROOT as APP_ROOT, ɵDepFlags as DepFlags, ɵNodeFlags as NodeFlags, ɵclearOverrides as clearOverrides, ɵgetInjectableDef as getInjectableDef, ɵivyEnabled as ivyEnabled, ɵoverrideComponentView as overrideComponentView, ɵoverrideProvider as overrideProvider, ɵstringify as stringify, ɵɵInjectableDef} from '@angular/core';

import {AsyncTestCompleter} from './async_test_completer';
import {ComponentFixture} from './component_fixture';
Expand Down Expand Up @@ -56,16 +56,15 @@ export interface TestBed {

compileComponents(): Promise<any>;

get<T>(token: Type<T>|InjectionToken<T>, notFoundValue?: T, flags?: InjectFlags): any;
inject<T>(
token: Type<T>|InjectionToken<T>|AbstractType<T>, notFoundValue?: T, flags?: InjectFlags): T;
inject<T>(
token: Type<T>|InjectionToken<T>|AbstractType<T>, notFoundValue: null, flags?: InjectFlags): T
|null;

// TODO: switch back to official deprecation marker once TSLint issue is resolved
// https://github.com/palantir/tslint/issues/4522
/**
* deprecated from v8.0.0 use Type<T> or InjectionToken<T>
* This does not use the deprecated jsdoc tag on purpose
* because it renders all overloads as deprecated in TSLint
* due to https://github.com/palantir/tslint/issues/4522.
*/
/** TODO(goodwine): Mark as deprecated from v9.0.0 use TestBed.inject */
get<T>(token: Type<T>|InjectionToken<T>, notFoundValue?: T, flags?: InjectFlags): any;
/** TODO(goodwine): Mark as deprecated from v9.0.0 use TestBed.inject */
get(token: any, notFoundValue?: any): any;

execute(tokens: any[], fn: Function, context?: any): any;
Expand Down Expand Up @@ -104,7 +103,7 @@ export interface TestBed {
* Note: Use `TestBed` in tests. It will be set to either `TestBedViewEngine` or `TestBedRender3`
* according to the compiler used.
*/
export class TestBedViewEngine implements Injector, TestBed {
export class TestBedViewEngine implements TestBed {
/**
* Initialize the environment for testing with a compiler factory, a PlatformRef, and an
* angular module. These are common to every test in the suite.
Expand Down Expand Up @@ -216,16 +215,29 @@ export class TestBedViewEngine implements Injector, TestBed {
return TestBedViewEngine as any as TestBedStatic;
}

static inject<T>(
token: Type<T>|InjectionToken<T>|AbstractType<T>, notFoundValue?: T, flags?: InjectFlags): T;
static inject<T>(
token: Type<T>|InjectionToken<T>|AbstractType<T>, notFoundValue: null, flags?: InjectFlags): T
|null;
static inject<T>(
token: Type<T>|InjectionToken<T>|AbstractType<T>, notFoundValue?: T|null,
flags?: InjectFlags): T|null {
return _getTestBedViewEngine().inject(token, notFoundValue, flags);
}

/** TODO(goodwine): Mark as deprecated from v9.0.0 use TestBed.inject */
static get<T>(token: Type<T>|InjectionToken<T>, notFoundValue?: T, flags?: InjectFlags): any;
/**
* @deprecated from v8.0.0 use Type<T> or InjectionToken<T>
* TODO(goodwine): Mark as deprecated from v9.0.0 use TestBed.inject
* @suppress {duplicate}
*/
static get(token: any, notFoundValue?: any): any;
/** TODO(goodwine): Mark as deprecated from v9.0.0 use TestBed.inject */
static get(
token: any, notFoundValue: any = Injector.THROW_IF_NOT_FOUND,
flags: InjectFlags = InjectFlags.Default): any {
return _getTestBedViewEngine().get(token, notFoundValue, flags);
return _getTestBedViewEngine().inject(token, notFoundValue, flags);
}

static createComponent<T>(component: Type<T>): ComponentFixture<T> {
Expand Down Expand Up @@ -431,8 +443,7 @@ export class TestBedViewEngine implements Injector, TestBed {
class DynamicTestModule {
}

const compilerFactory: TestingCompilerFactory =
this.platform.injector.get(TestingCompilerFactory);
const compilerFactory = this.platform.injector.get(TestingCompilerFactory);
this._compiler = compilerFactory.createTestingCompiler(this._compilerOptions);
for (const summary of [this._testEnvAotSummaries, ...this._aotSummaries]) {
this._compiler.loadAotSummaries(summary);
Expand All @@ -454,26 +465,37 @@ export class TestBedViewEngine implements Injector, TestBed {
}
}

get<T>(token: Type<T>|InjectionToken<T>, notFoundValue?: T, flags?: InjectFlags): any;
/**
* @deprecated from v8.0.0 use Type<T> or InjectionToken<T>
*/
get(token: any, notFoundValue?: any): any;
get(token: any, notFoundValue: any = Injector.THROW_IF_NOT_FOUND,
flags: InjectFlags = InjectFlags.Default): any {
inject<T>(
token: Type<T>|InjectionToken<T>|AbstractType<T>, notFoundValue?: T, flags?: InjectFlags): T;
inject<T>(
token: Type<T>|InjectionToken<T>|AbstractType<T>, notFoundValue: null, flags?: InjectFlags): T
|null;
inject<T>(
token: Type<T>|InjectionToken<T>|AbstractType<T>, notFoundValue?: T|null,
flags?: InjectFlags): T|null {
this._initIfNeeded();
if (token === TestBed) {
return this;
if (token as unknown === TestBed) {
return this as any;
}
// Tests can inject things from the ng module and from the compiler,
// but the ng module can't inject things from the compiler and vice versa.
const result = this._moduleRef.injector.get(token, UNDEFINED, flags);
return result === UNDEFINED ? this._compiler.injector.get(token, notFoundValue, flags) : result;
}

/** TODO(goodwine): Mark as deprecated from v9.0.0 use TestBed.inject */
get<T>(token: Type<T>|InjectionToken<T>, notFoundValue?: T, flags?: InjectFlags): any;
/** TODO(goodwine): Mark as deprecated from v9.0.0 use TestBed.inject */
get(token: any, notFoundValue?: any): any;
/** TODO(goodwine): Mark as deprecated from v9.0.0 use TestBed.inject */
get(token: any, notFoundValue: any = Injector.THROW_IF_NOT_FOUND,
flags: InjectFlags = InjectFlags.Default): any {
return this.inject(token, notFoundValue, flags);
}

execute(tokens: any[], fn: Function, context?: any): any {
this._initIfNeeded();
const params = tokens.map(t => this.get(t));
const params = tokens.map(t => this.inject(t));
return fn.apply(context, params);
}

Expand Down Expand Up @@ -575,12 +597,13 @@ export class TestBedViewEngine implements Injector, TestBed {
`Cannot create the component ${stringify(component)} as it was not imported into the testing module!`);
}

// TODO: Don't cast as `any`, proper type is boolean[]
const noNgZone = this.get(ComponentFixtureNoNgZone as any, false);
// TODO: Don't cast as `any`, proper type is boolean[]
const autoDetect: boolean = this.get(ComponentFixtureAutoDetect as any, false);
const ngZone: NgZone|null = noNgZone ? null : this.get(NgZone as Type<NgZone|null>, null);
const testComponentRenderer: TestComponentRenderer = this.get(TestComponentRenderer);
// TODO: Don't cast as `InjectionToken<boolean>`, declared type is boolean[]
const noNgZone = this.inject(ComponentFixtureNoNgZone as InjectionToken<boolean>, false);
// TODO: Don't cast as `InjectionToken<boolean>`, declared type is boolean[]
const autoDetect: boolean =
this.inject(ComponentFixtureAutoDetect as InjectionToken<boolean>, false);
const ngZone: NgZone|null = noNgZone ? null : this.inject(NgZone, null);
const testComponentRenderer: TestComponentRenderer = this.inject(TestComponentRenderer);
const rootElId = `root${_nextRootElementId++}`;
testComponentRenderer.insertRootElement(rootElId);

Expand Down Expand Up @@ -658,7 +681,7 @@ export function inject(tokens: any[], fn: Function): () => any {
// Return an async test method that returns a Promise if AsyncTestCompleter is one of
// the injected tokens.
return testBed.compileComponents().then(() => {
const completer: AsyncTestCompleter = testBed.get(AsyncTestCompleter);
const completer = testBed.inject(AsyncTestCompleter);
testBed.execute(tokens, fn, this);
return completer.promise;
});
Expand Down
18 changes: 9 additions & 9 deletions packages/core/testing/src/test_bed_common.ts
View file
Open in desktop
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 {Component, Directive, InjectFlags, InjectionToken, NgModule, Pipe, PlatformRef, SchemaMetadata, Type} from '@angular/core';
import {AbstractType, Component, Directive, InjectFlags, InjectionToken, NgModule, Pipe, PlatformRef, SchemaMetadata, Type} from '@angular/core';

import {ComponentFixture} from './component_fixture';
import {MetadataOverride} from './metadata_override';
Expand Down Expand Up @@ -114,15 +114,15 @@ export interface TestBedStatic {
deps?: any[],
}): TestBedStatic;

inject<T>(
token: Type<T>|InjectionToken<T>|AbstractType<T>, notFoundValue?: T, flags?: InjectFlags): T;
inject<T>(
token: Type<T>|InjectionToken<T>|AbstractType<T>, notFoundValue: null, flags?: InjectFlags): T
|null;

/** TODO(goodwine): Mark as deprecated from v9.0.0 use TestBed.inject */
get<T>(token: Type<T>|InjectionToken<T>, notFoundValue?: T, flags?: InjectFlags): any;
// TODO: switch back to official deprecation marker once TSLint issue is resolved
// https://github.com/palantir/tslint/issues/4522
/**
* deprecated from v8.0.0 use Type<T> or InjectionToken<T>
* This does not use the deprecated jsdoc tag on purpose
* because it renders all overloads as deprecated in TSLint
* due to https://github.com/palantir/tslint/issues/4522.
*/
/** TODO(goodwine): Mark as deprecated from v9.0.0 use TestBed.inject */
get(token: any, notFoundValue?: any): any;

createComponent<T>(component: Type<T>): ComponentFixture<T>;
Expand Down
10 changes: 5 additions & 5 deletions packages/platform-server/test/integration_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,7 @@ class HiddenModule {
const mock = ref.injector.get(HttpTestingController) as HttpTestingController;
const http = ref.injector.get(HttpClient);
ref.injector.get<NgZone>(NgZone).run(() => {
http.get('http://localhost/testing').subscribe((body: string) => {
http.get<string>('http://localhost/testing').subscribe((body: string) => {
NgZone.assertInAngularZone();
expect(body).toEqual('success!');
});
Expand All @@ -791,8 +791,8 @@ class HiddenModule {
platform.bootstrapModule(HttpClientExampleModule).then(ref => {
const mock = ref.injector.get(HttpTestingController) as HttpTestingController;
const http = ref.injector.get(HttpClient);
ref.injector.get<NgZone>(NgZone).run(() => {
http.get('http://localhost/testing').subscribe((body: string) => {
ref.injector.get(NgZone).run(() => {
http.get<string>('http://localhost/testing').subscribe((body: string) => {
expect(body).toEqual('success!');
});
expect(ref.injector.get<NgZone>(NgZone).hasPendingMacrotasks).toBeTruthy();
Expand All @@ -808,8 +808,8 @@ class HiddenModule {
platform.bootstrapModule(HttpInterceptorExampleModule).then(ref => {
const mock = ref.injector.get(HttpTestingController) as HttpTestingController;
const http = ref.injector.get(HttpClient);
ref.injector.get<NgZone>(NgZone).run(() => {
http.get('http://localhost/testing').subscribe((body: string) => {
ref.injector.get(NgZone).run(() => {
http.get<string>('http://localhost/testing').subscribe((body: string) => {
NgZone.assertInAngularZone();
expect(body).toEqual('success!');
});
Expand Down