fix(compiler): remove outdated and invalid warning for unresolved DI parameters (#36985)
In past versions of the View Engine compiler, we added a warning that is printed whenever the compiler comes across an Angular declaration with a constructor that does not match suitable DI tokens. The warning mentioned that in `v6.x` it will turn into an actual error. This actually happened as expected for most cases. e.g. the constructor of `@NgModule`, `@Component`'s, `@Pipe`'s etc will be checked and an error will be reported if constructor is not DI compatible. The warning has never been removed though as it was still relevant for unprovided injectables, or injectables serialized into summaries of the Angular compiler. As of version 10, classes that use Angular features need an Angular decorator. This includes base classes of services that use the lifecycles Angular feature. Due to this being a common pattern now, we can remove the warning in View Engine. The warning is not correct, and also quite confusing as it mentions the planned removal in `v6.x`. Resolves FW-2147. PR Close #36985
This commit is contained in:
parent
97d6d909e7
commit
d0280a0335
|
@ -989,8 +989,6 @@ export class CompileMetadataResolver {
|
||||||
`Can't resolve all parameters for ${stringifyType(typeOrFunc)}: (${depsTokens}).`;
|
`Can't resolve all parameters for ${stringifyType(typeOrFunc)}: (${depsTokens}).`;
|
||||||
if (throwOnUnknownDeps || this._config.strictInjectionParameters) {
|
if (throwOnUnknownDeps || this._config.strictInjectionParameters) {
|
||||||
this._reportError(syntaxError(message), typeOrFunc);
|
this._reportError(syntaxError(message), typeOrFunc);
|
||||||
} else {
|
|
||||||
this._console.warn(`Warning: ${message} This will become an error in Angular v6.x`);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -190,23 +190,64 @@ describe('compiler (unbundled Angular)', () => {
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('errors', () => {
|
describe('errors', () => {
|
||||||
it('should only warn if not all arguments of an @Injectable class can be resolved', () => {
|
it('should not error or warn if an unprovided @Injectable with DI-incompatible ' +
|
||||||
|
'constructor is discovered',
|
||||||
|
() => {
|
||||||
|
const FILES: MockDirectory = {
|
||||||
|
app: {
|
||||||
|
'app.ts': `
|
||||||
|
import {Injectable, NgModule} from '@angular/core';
|
||||||
|
|
||||||
|
// This injectable is not provided. It is used as a base class for another
|
||||||
|
// service but is not directly provided. It's allowed for such classes to
|
||||||
|
// have a decorator applied as they use Angular features.
|
||||||
|
@Injectable()
|
||||||
|
export class ServiceBase {
|
||||||
|
constructor(a: boolean) {}
|
||||||
|
|
||||||
|
ngOnDestroy() {}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Injectable()
|
||||||
|
export class MyService extends ServiceBase {
|
||||||
|
constructor() {
|
||||||
|
super(true);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@NgModule({providers: [MyService]})
|
||||||
|
export class AppModule {}
|
||||||
|
`
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
spyOn(console, 'error');
|
||||||
|
spyOn(console, 'warn');
|
||||||
|
expect(() => compile([FILES, angularFiles])).not.toThrowError();
|
||||||
|
expect(console.warn).toHaveBeenCalledTimes(0);
|
||||||
|
expect(console.error).toHaveBeenCalledTimes(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should error if parameters of a provided @Injectable class cannot be resolved', () => {
|
||||||
const FILES: MockDirectory = {
|
const FILES: MockDirectory = {
|
||||||
app: {
|
app: {
|
||||||
'app.ts': `
|
'app.ts': `
|
||||||
import {Injectable} from '@angular/core';
|
import {Injectable, NgModule} from '@angular/core';
|
||||||
|
|
||||||
@Injectable()
|
@Injectable()
|
||||||
export class MyService {
|
export class MyService {
|
||||||
constructor(a: boolean) {}
|
constructor(a: boolean) {}
|
||||||
}
|
}
|
||||||
`
|
|
||||||
|
@NgModule({
|
||||||
|
providers: [MyService],
|
||||||
|
})
|
||||||
|
export class MyModule {}
|
||||||
|
`
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
const warnSpy = spyOn(console, 'warn');
|
expect(() => compile([FILES, angularFiles]))
|
||||||
compile([FILES, angularFiles]);
|
.toThrowError(`Can't resolve all parameters for MyService in /app/app.ts: (?).`);
|
||||||
expect(warnSpy).toHaveBeenCalledWith(
|
|
||||||
`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 strictInjectionParameters is true',
|
it('should error if not all arguments of an @Injectable class can be resolved if strictInjectionParameters is true',
|
||||||
|
|
Loading…
Reference in New Issue