From 5c34066058356dd1db7fa8261bd1c157977af058 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Mon, 13 Mar 2017 14:39:34 -0700 Subject: [PATCH] fix(compiler): only warn for `@Injectable` classes with invalid args. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In v2.x, users had to annotate classes that they intended to use as tokens with `@Injectable`. This is no longer required in v4.x for tokens, and we now require the constructor parameters of classes annotated with `@Injectable` to be statically analyzable by ngc. This commit reduces the error into a warning if the constructor parameters do not meet this condition. DEPRECATION: - Classes annotated with `@Injectable` but whose constructor’s parameter types are not statically analyzable by ngc will produce a warning. Closes #15003 --- packages/compiler/src/aot/compiler_factory.ts | 2 +- packages/compiler/src/i18n/extractor.ts | 4 +-- packages/compiler/src/metadata_resolver.ts | 31 ++++++++++++------- packages/compiler/test/aot/compiler_spec.ts | 29 +++++++++++++++++ .../language-service/src/typescript_host.ts | 6 ++-- 5 files changed, 55 insertions(+), 17 deletions(-) diff --git a/packages/compiler/src/aot/compiler_factory.ts b/packages/compiler/src/aot/compiler_factory.ts index 5cacaa97aa..ce76cf414a 100644 --- a/packages/compiler/src/aot/compiler_factory.ts +++ b/packages/compiler/src/aot/compiler_factory.ts @@ -67,7 +67,7 @@ export function createAotCompiler(compilerHost: AotCompilerHost, options: AotCom const resolver = new CompileMetadataResolver( config, new NgModuleResolver(staticReflector), new DirectiveResolver(staticReflector), new PipeResolver(staticReflector), summaryResolver, elementSchemaRegistry, normalizer, - symbolCache, staticReflector); + console, symbolCache, staticReflector); // TODO(vicb): do not pass options.i18nFormat here const importResolver = { getImportAs: (symbol: StaticSymbol) => symbolResolver.getImportAs(symbol), diff --git a/packages/compiler/src/i18n/extractor.ts b/packages/compiler/src/i18n/extractor.ts index be90bc40f2..b8c83a21e8 100644 --- a/packages/compiler/src/i18n/extractor.ts +++ b/packages/compiler/src/i18n/extractor.ts @@ -10,7 +10,7 @@ /** * Extract i18n messages from source code */ -import {ViewEncapsulation} from '@angular/core'; +import {ViewEncapsulation, ɵConsole as Console} from '@angular/core'; import {analyzeAndValidateNgModules, extractProgramSymbols} from '../aot/compiler'; import {StaticAndDynamicReflectionCapabilities} from '../aot/static_reflection_capabilities'; @@ -106,7 +106,7 @@ export class Extractor { const resolver = new CompileMetadataResolver( config, new NgModuleResolver(staticReflector), new DirectiveResolver(staticReflector), new PipeResolver(staticReflector), summaryResolver, elementSchemaRegistry, normalizer, - symbolCache, staticReflector); + new Console(), symbolCache, staticReflector); // TODO(vicb): implicit tags & attributes const messageBundle = new MessageBundle(htmlParser, [], {}, locale); diff --git a/packages/compiler/src/metadata_resolver.ts b/packages/compiler/src/metadata_resolver.ts index e51380b64d..a8af2ef7f9 100644 --- a/packages/compiler/src/metadata_resolver.ts +++ b/packages/compiler/src/metadata_resolver.ts @@ -6,7 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ -import {Attribute, ChangeDetectionStrategy, Component, ComponentFactory, Directive, Host, Inject, Injectable, InjectionToken, ModuleWithProviders, Optional, Provider, Query, RendererType2, SchemaMetadata, Self, SkipSelf, Type, resolveForwardRef, ɵERROR_COMPONENT_TYPE, ɵLIFECYCLE_HOOKS_VALUES, ɵReflectorReader, ɵccf as createComponentFactory, ɵreflector, ɵstringify as stringify} from '@angular/core'; +import {Attribute, ChangeDetectionStrategy, Component, ComponentFactory, Directive, Host, Inject, Injectable, InjectionToken, ModuleWithProviders, Optional, Provider, Query, RendererType2, SchemaMetadata, Self, SkipSelf, Type, resolveForwardRef, ɵConsole as Console, ɵERROR_COMPONENT_TYPE, ɵLIFECYCLE_HOOKS_VALUES, ɵReflectorReader, ɵccf as createComponentFactory, ɵreflector, ɵstringify as stringify} from '@angular/core'; + import {StaticSymbol, StaticSymbolCache} from './aot/static_symbol'; import {ngfactoryFilePath} from './aot/util'; import {assertArrayOfStrings, assertInterpolationSymbols} from './assertions'; @@ -49,7 +50,7 @@ export class CompileMetadataResolver { private _directiveResolver: DirectiveResolver, private _pipeResolver: PipeResolver, private _summaryResolver: SummaryResolver, private _schemaRegistry: ElementSchemaRegistry, - private _directiveNormalizer: DirectiveNormalizer, + private _directiveNormalizer: DirectiveNormalizer, private _console: Console, @Optional() private _staticSymbolCache: StaticSymbolCache, private _reflector: ɵReflectorReader = ɵreflector, @Optional() @Inject(ERROR_COLLECTOR_TOKEN) private _errorCollector?: ErrorCollector) {} @@ -665,7 +666,10 @@ export class CompileMetadataResolver { } getInjectableSummary(type: any): cpl.CompileTypeSummary { - return {summaryKind: cpl.CompileSummaryKind.Injectable, type: this._getTypeMetadata(type)}; + return { + summaryKind: cpl.CompileSummaryKind.Injectable, + type: this._getTypeMetadata(type, null, false) + }; } private _getInjectableMetadata(type: Type, dependencies: any[] = null): @@ -677,11 +681,12 @@ export class CompileMetadataResolver { return this._getTypeMetadata(type, dependencies); } - private _getTypeMetadata(type: Type, dependencies: any[] = null): cpl.CompileTypeMetadata { + private _getTypeMetadata(type: Type, dependencies: any[] = null, throwOnUnknownDeps = true): + cpl.CompileTypeMetadata { const identifier = this._getIdentifierMetadata(type); return { reference: identifier.reference, - diDeps: this._getDependenciesMetadata(identifier.reference, dependencies), + diDeps: this._getDependenciesMetadata(identifier.reference, dependencies, throwOnUnknownDeps), lifecycleHooks: ɵLIFECYCLE_HOOKS_VALUES.filter(hook => hasLifecycleHook(hook, identifier.reference)), }; @@ -742,8 +747,9 @@ export class CompileMetadataResolver { return pipeMeta; } - private _getDependenciesMetadata(typeOrFunc: Type|Function, dependencies: any[]): - cpl.CompileDiDependencyMetadata[] { + private _getDependenciesMetadata( + typeOrFunc: Type|Function, dependencies: any[], + throwOnUnknownDeps = true): cpl.CompileDiDependencyMetadata[] { let hasUnknownDeps = false; const params = dependencies || this._reflector.parameters(typeOrFunc) || []; @@ -797,10 +803,13 @@ export class CompileMetadataResolver { if (hasUnknownDeps) { const depsTokens = dependenciesMetadata.map((dep) => dep ? stringifyType(dep.token) : '?').join(', '); - this._reportError( - syntaxError( - `Can't resolve all parameters for ${stringifyType(typeOrFunc)}: (${depsTokens}).`), - typeOrFunc); + const message = + `Can't resolve all parameters for ${stringifyType(typeOrFunc)}: (${depsTokens}).`; + if (throwOnUnknownDeps) { + this._reportError(syntaxError(message), typeOrFunc); + } else { + this._console.warn(`Warning: ${message} This will become an error in Angular v5.x`); + } } return dependenciesMetadata; diff --git a/packages/compiler/test/aot/compiler_spec.ts b/packages/compiler/test/aot/compiler_spec.ts index cea4f7c6cf..846fb5a7f9 100644 --- a/packages/compiler/test/aot/compiler_spec.ts +++ b/packages/compiler/test/aot/compiler_spec.ts @@ -219,6 +219,35 @@ describe('compiler (unbundled Angular)', () => { })); } }); + + describe('errors', () => { + it('should only warn if not all arguments of an @Injectable class can be resolved', + fakeAsync(() => { + const FILES: MockData = { + app: { + 'app.ts': ` + import {Injectable} from '@angular/core'; + + @Injectable() + export class MyService { + constructor(a: boolean) {} + } + ` + } + }; + const host = new MockCompilerHost(['/app/app.ts'], FILES, angularFiles); + const aotHost = new MockAotCompilerHost(host); + let ok = false; + const warnSpy = spyOn(console, 'warn'); + compile(host, aotHost, expectNoDiagnostics).then(() => ok = true); + + tick(); + + expect(ok).toBe(true); + expect(warnSpy).toHaveBeenCalledWith( + `Warning: Can't resolve all parameters for MyService in /app/app.ts: (?). This will become an error in Angular v5.x`); + })); + }); }); describe('compiler (bundled Angular)', () => { diff --git a/packages/language-service/src/typescript_host.ts b/packages/language-service/src/typescript_host.ts index 3a1d161399..1faadf7fc5 100644 --- a/packages/language-service/src/typescript_host.ts +++ b/packages/language-service/src/typescript_host.ts @@ -7,7 +7,7 @@ */ import {AotSummaryResolver, CompileDirectiveMetadata, CompileMetadataResolver, CompilerConfig, DEFAULT_INTERPOLATION_CONFIG, DirectiveNormalizer, DirectiveResolver, DomElementSchemaRegistry, HtmlParser, InterpolationConfig, NgAnalyzedModules, NgModuleResolver, ParseTreeResult, Parser, PipeResolver, ResourceLoader, StaticReflector, StaticSymbol, StaticSymbolCache, StaticSymbolResolver, SummaryResolver, UrlResolver, analyzeNgModules, componentModuleUrl, createOfflineCompileUrlResolver, extractProgramSymbols} from '@angular/compiler'; -import {Type, ViewEncapsulation} from '@angular/core'; +import {Type, ViewEncapsulation, ɵConsole as Console} from '@angular/core'; import * as fs from 'fs'; import * as path from 'path'; import * as ts from 'typescript'; @@ -115,8 +115,8 @@ export class TypeScriptServiceHost implements LanguageServiceHost { result = this._resolver = new CompileMetadataResolver( config, moduleResolver, directiveResolver, pipeResolver, new SummaryResolver(), - elementSchemaRegistry, directiveNormalizer, this._staticSymbolCache, this.reflector, - (error, type) => this.collectError(error, type && type.filePath)); + elementSchemaRegistry, directiveNormalizer, new Console(), this._staticSymbolCache, + this.reflector, (error, type) => this.collectError(error, type && type.filePath)); } return result; }