fix(compiler): only warn for `@Injectable` classes with invalid args.

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
This commit is contained in:
Tobias Bosch 2017-03-13 14:39:34 -07:00 committed by Chuck Jazdzewski
parent 50ab06e29d
commit 5c34066058
5 changed files with 55 additions and 17 deletions

View File

@ -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),

View File

@ -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);

View File

@ -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<any>,
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<any>, dependencies: any[] = null):
@ -677,11 +681,12 @@ export class CompileMetadataResolver {
return this._getTypeMetadata(type, dependencies);
}
private _getTypeMetadata(type: Type<any>, dependencies: any[] = null): cpl.CompileTypeMetadata {
private _getTypeMetadata(type: Type<any>, 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<any>|Function, dependencies: any[]):
cpl.CompileDiDependencyMetadata[] {
private _getDependenciesMetadata(
typeOrFunc: Type<any>|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;

View File

@ -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)', () => {

View File

@ -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;
}