From f755db78dcdf461dc42e709b3ab728ceba353d1d Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Tue, 13 Feb 2018 12:51:21 -0800 Subject: [PATCH] fix(core): require factory to be provided for shakeable InjectionToken (#22207) InjectionToken can be created with an ngInjectableDef, and previously this allowed the full expressiveness of @Injectable. However, this requires a runtime reflection system in order to generate factories from expressed provider declarations. Instead, this change requires scoped InjectionTokens to provide the factory directly (likely using inject() for the arguments), bypassing the need for a reflection system. Fixes #22205 PR Close #22207 --- integration/_payload-limits.json | 2 +- .../bazel/injectable_def/app/src/token.ts | 20 +++++++++++-------- .../compiler-cli/src/metadata/evaluator.ts | 3 --- packages/compiler-cli/test/ngc_spec.ts | 19 ++++++++++++++++++ packages/compiler/src/injectable_compiler.ts | 4 +++- packages/core/src/di/injection_token.ts | 10 +++------- tools/public_api_guard/core/core.d.ts | 3 ++- 7 files changed, 40 insertions(+), 21 deletions(-) diff --git a/integration/_payload-limits.json b/integration/_payload-limits.json index 726640df59..2caa8b7270 100644 --- a/integration/_payload-limits.json +++ b/integration/_payload-limits.json @@ -3,7 +3,7 @@ "master": { "uncompressed": { "inline": 1447, - "main": 159944, + "main": 155112, "polyfills": 59179 } } diff --git a/packages/compiler-cli/integrationtest/bazel/injectable_def/app/src/token.ts b/packages/compiler-cli/integrationtest/bazel/injectable_def/app/src/token.ts index ae07315cba..e557195ded 100644 --- a/packages/compiler-cli/integrationtest/bazel/injectable_def/app/src/token.ts +++ b/packages/compiler-cli/integrationtest/bazel/injectable_def/app/src/token.ts @@ -12,6 +12,16 @@ import {ServerModule} from '@angular/platform-server'; export interface IService { readonly data: string; } +@NgModule({}) +export class TokenModule { +} + +export const TOKEN = new InjectionToken('test', { + scope: TokenModule, + factory: () => new Service(), +}); + + @Component({ selector: 'token-app', template: '{{data}}', @@ -25,18 +35,12 @@ export class AppComponent { imports: [ BrowserModule.withServerTransition({appId: 'id-app'}), ServerModule, + TokenModule, ], declarations: [AppComponent], bootstrap: [AppComponent], - providers: [{provide: forwardRef(() => TOKEN), useClass: forwardRef(() => Service)}] }) export class TokenAppModule { } -export class Service { readonly data = 'fromToken'; } - -export const TOKEN = new InjectionToken('test', { - scope: TokenAppModule, - useClass: Service, - deps: [], -}); +export class Service { readonly data = 'fromToken'; } \ No newline at end of file diff --git a/packages/compiler-cli/src/metadata/evaluator.ts b/packages/compiler-cli/src/metadata/evaluator.ts index cf5c2ea269..3dfb6ab6be 100644 --- a/packages/compiler-cli/src/metadata/evaluator.ts +++ b/packages/compiler-cli/src/metadata/evaluator.ts @@ -381,9 +381,6 @@ export class Evaluator { case ts.SyntaxKind.NewExpression: const newExpression = node; const newArgs = arrayOrEmpty(newExpression.arguments).map(arg => this.evaluateNode(arg)); - if (!this.options.verboseInvalidExpression && newArgs.some(isMetadataError)) { - return recordEntry(newArgs.find(isMetadataError), node); - } const newTarget = this.evaluateNode(newExpression.expression); if (isMetadataError(newTarget)) { return recordEntry(newTarget, node); diff --git a/packages/compiler-cli/test/ngc_spec.ts b/packages/compiler-cli/test/ngc_spec.ts index de60f5630a..6c53bb4417 100644 --- a/packages/compiler-cli/test/ngc_spec.ts +++ b/packages/compiler-cli/test/ngc_spec.ts @@ -2094,5 +2094,24 @@ describe('ngc transformer command-line', () => { `); expect(source).toMatch(/ngInjectableDef.*return ..\(..\.inject\(Existing, undefined, 1\)/); }); + + it('compiles a service that depends on a token', () => { + const source = compileService(` + import {Inject, Injectable, InjectionToken} from '@angular/core'; + import {Module} from './module'; + + export const TOKEN = new InjectionToken('desc', {scope: Module, factory: () => true}); + + @Injectable({ + scope: Module, + }) + export class Service { + constructor(@Inject(TOKEN) value: boolean) {} + } + `); + expect(source).toMatch(/ngInjectableDef = .+\.defineInjectable\(/); + expect(source).toMatch(/ngInjectableDef.*token: Service/); + expect(source).toMatch(/ngInjectableDef.*scope: .+\.Module/); + }); }); }); diff --git a/packages/compiler/src/injectable_compiler.ts b/packages/compiler/src/injectable_compiler.ts index a991dc6b8f..2ae1c7af7b 100644 --- a/packages/compiler/src/injectable_compiler.ts +++ b/packages/compiler/src/injectable_compiler.ts @@ -48,12 +48,14 @@ export class InjectableCompiler { } else if (v.ngMetadataName === 'Self') { flags |= InjectFlags.Self; } else if (v.ngMetadataName === 'Inject') { - throw new Error('@Inject() is not implemented'); + token = v.token; } else { token = v; } } } + } + if (flags !== InjectFlags.Default || defaultValue !== undefined) { args = [ctx.importExpr(token), o.literal(defaultValue), o.literal(flags)]; } else { args = [ctx.importExpr(token)]; diff --git a/packages/core/src/di/injection_token.ts b/packages/core/src/di/injection_token.ts index 81329e7eef..933051c340 100644 --- a/packages/core/src/di/injection_token.ts +++ b/packages/core/src/di/injection_token.ts @@ -8,11 +8,7 @@ import {Type} from '../type'; -import {Injectable, convertInjectableProviderToFactory, defineInjectable} from './injectable'; -import {ClassSansProvider, ExistingSansProvider, FactorySansProvider, StaticClassSansProvider, ValueSansProvider} from './provider'; - -export type InjectionTokenProvider = ValueSansProvider | ExistingSansProvider | - FactorySansProvider | ClassSansProvider | StaticClassSansProvider; +import {Injectable, defineInjectable} from './injectable'; /** * Creates a token that can be used in a DI Provider. @@ -42,11 +38,11 @@ export class InjectionToken { readonly ngInjectableDef: Injectable|undefined; - constructor(protected _desc: string, options?: {scope: Type}&InjectionTokenProvider) { + constructor(protected _desc: string, options?: {scope: Type, factory: () => T}) { if (options !== undefined) { this.ngInjectableDef = defineInjectable({ scope: options.scope, - factory: convertInjectableProviderToFactory(this as any, options), + factory: options.factory, }); } else { this.ngInjectableDef = undefined; diff --git a/tools/public_api_guard/core/core.d.ts b/tools/public_api_guard/core/core.d.ts index e5bc87993e..8f82abbeea 100644 --- a/tools/public_api_guard/core/core.d.ts +++ b/tools/public_api_guard/core/core.d.ts @@ -492,7 +492,8 @@ export declare class InjectionToken { readonly ngInjectableDef: Injectable | undefined; constructor(_desc: string, options?: { scope: Type; - } & InjectionTokenProvider); + factory: () => T; + }); toString(): string; }