feat(compiler): enabled strict checking of parameters to an `@Injectable` (#19412)

Added the compiler options `strictInjectionParameters` that defaults
to `false`. If enabled the compiler will report errors for parameters
of an `@Injectable` that cannot be determined instead of generating a
warning.

This is planned to be switched to default to `true` for Angular 6.0.
This commit is contained in:
Chuck Jazdzewski 2017-09-26 13:40:47 -07:00 committed by Victor Berchet
parent a75040d0a1
commit dfb8d21ef4
6 changed files with 37 additions and 5 deletions

View File

@ -52,6 +52,14 @@ export interface CompilerOptions extends ts.CompilerOptions {
// Don't produce .ngfactory.ts or .ngstyle.ts files
skipTemplateCodegen?: boolean;
// Always report errors when the type of a parameter supplied whose injection type cannot
// be determined. When this value option is not provided or is `false`, constructor
// parameters of classes marked with `@Injectable` whose type cannot be resolved will
// produce a warning. With this option `true`, they produce an error. When this option is
// not provided is treated as if it were `false`. In Angular 6.0, if this option is not
// provided, it will be treated as `true`.
strictInjectionParameters?: boolean;
// Whether to generate a flat module index of the given name and the corresponding
// flat module metadata. This option is intended to be used when creating flat
// modules similar to how `@angular/core` and `@angular/common` are packaged.

View File

@ -69,6 +69,7 @@ export function createAotCompiler(compilerHost: AotCompilerHost, options: AotCom
enableLegacyTemplate: options.enableLegacyTemplate === true,
missingTranslation: options.missingTranslation,
preserveWhitespaces: options.preserveWhitespaces,
strictInjectionParameters: options.strictInjectionParameters,
});
const normalizer = new DirectiveNormalizer(
{get: (url: string) => compilerHost.loadResource(url)}, urlResolver, htmlParser, config);

View File

@ -19,4 +19,5 @@ export interface AotCompilerOptions {
preserveWhitespaces?: boolean;
fullTemplateTypeCheck?: boolean;
allowEmptyCodegenFiles?: boolean;
strictInjectionParameters?: boolean;
}

View File

@ -21,17 +21,18 @@ export class CompilerConfig {
public jitDevMode: boolean;
public missingTranslation: MissingTranslationStrategy|null;
public preserveWhitespaces: boolean;
public strictInjectionParameters: boolean;
constructor(
{defaultEncapsulation = ViewEncapsulation.Emulated, useJit = true, jitDevMode = false,
missingTranslation, enableLegacyTemplate, preserveWhitespaces}: {
missingTranslation, enableLegacyTemplate, preserveWhitespaces, strictInjectionParameters}: {
defaultEncapsulation?: ViewEncapsulation,
useJit?: boolean,
jitDevMode?: boolean,
missingTranslation?: MissingTranslationStrategy,
enableLegacyTemplate?: boolean,
preserveWhitespaces?: boolean,
fullTemplateTypeCheck?: boolean
strictInjectionParameters?: boolean,
} = {}) {
this.defaultEncapsulation = defaultEncapsulation;
this.useJit = !!useJit;
@ -39,6 +40,7 @@ export class CompilerConfig {
this.missingTranslation = missingTranslation || null;
this.enableLegacyTemplate = enableLegacyTemplate === true;
this.preserveWhitespaces = preserveWhitespacesDefault(noUndefined(preserveWhitespaces));
this.strictInjectionParameters = strictInjectionParameters === true;
}
}

View File

@ -886,10 +886,10 @@ export class CompileMetadataResolver {
dependenciesMetadata.map((dep) => dep ? stringifyType(dep.token) : '?').join(', ');
const message =
`Can't resolve all parameters for ${stringifyType(typeOrFunc)}: (${depsTokens}).`;
if (throwOnUnknownDeps) {
if (throwOnUnknownDeps || this._config.strictInjectionParameters) {
this._reportError(syntaxError(message), typeOrFunc);
} else {
this._console.warn(`Warning: ${message} This will become an error in Angular v5.x`);
this._console.warn(`Warning: ${message} This will become an error in Angular v6.x`);
}
}

View File

@ -205,10 +205,30 @@ describe('compiler (unbundled Angular)', () => {
const warnSpy = spyOn(console, 'warn');
compile([FILES, angularFiles]);
expect(warnSpy).toHaveBeenCalledWith(
`Warning: Can't resolve all parameters for MyService in /app/app.ts: (?). This will become an error in Angular v5.x`);
`Warning: Can't resolve all parameters for MyService in /app/app.ts: (?). This will become an error in Angular v6.x`);
});
it('should error if not all arguments of an @Injectable class can be resolved if strictInjectionParamters is true',
() => {
const FILES: MockDirectory = {
app: {
'app.ts': `
import {Injectable} from '@angular/core';
@Injectable()
export class MyService {
constructor(a: boolean) {}
}
`
}
};
const warnSpy = spyOn(console, 'warn');
expect(() => compile([FILES, angularFiles], {strictInjectionParameters: true}))
.toThrowError(`Can't resolve all parameters for MyService in /app/app.ts: (?).`);
expect(warnSpy).not.toHaveBeenCalled();
});
it('should be able to suppress a null access', () => {
const FILES: MockDirectory = {
app: {