fix(ivy): throw a better error when DI can't inject a ctor param (#33739)

Occasionally a factory function needs to be generated for an "invalid"
constructor (one with parameters types which aren't injectable). Typically
this happens in JIT mode where understanding of parameters cannot be done in
the same "up-front" way that the AOT compiler can.

This commit changes the JIT compiler to generate a new `invalidFactoryDep`
call for each invalid parameter. This instruction will error at runtime if
called, indicating both the index of the invalid parameter as well as (via
the stack trace) the factory function which was generated for the type being
constructed.

Fixes #33637

PR Close #33739
This commit is contained in:
Alex Rickabaugh 2019-11-11 00:18:43 -08:00 committed by Andrew Kushnir
parent 7823c23932
commit 13c2fad240
11 changed files with 97 additions and 10 deletions

View File

@ -67,6 +67,7 @@ export enum R3ResolvedDependencyType {
Token = 0,
Attribute = 1,
ChangeDetectorRef = 2,
Invalid = 3,
}
export enum R3FactoryTarget {

View File

@ -125,6 +125,11 @@ export enum R3ResolvedDependencyType {
* Injecting the `ChangeDetectorRef` token. Needs special handling when injected into a pipe.
*/
ChangeDetectorRef = 2,
/**
* An invalid dependency (no token could be determined). An error should be thrown at runtime.
*/
Invalid = 3,
}
/**
@ -271,11 +276,12 @@ export function compileFactoryFunction(meta: R3FactoryMetadata): R3FactoryFn {
function injectDependencies(
deps: R3DependencyMetadata[], injectFn: o.ExternalReference, isPipe: boolean): o.Expression[] {
return deps.map(dep => compileInjectDependency(dep, injectFn, isPipe));
return deps.map((dep, index) => compileInjectDependency(dep, injectFn, isPipe, index));
}
function compileInjectDependency(
dep: R3DependencyMetadata, injectFn: o.ExternalReference, isPipe: boolean): o.Expression {
dep: R3DependencyMetadata, injectFn: o.ExternalReference, isPipe: boolean,
index: number): o.Expression {
// Interpret the dependency according to its resolved type.
switch (dep.resolved) {
case R3ResolvedDependencyType.Token:
@ -305,6 +311,8 @@ function compileInjectDependency(
case R3ResolvedDependencyType.Attribute:
// In the case of attributes, the attribute name in question is given as the token.
return o.importExpr(R3.injectAttribute).callFn([dep.token]);
case R3ResolvedDependencyType.Invalid:
return o.importExpr(R3.invalidFactoryDep).callFn([o.literal(index)]);
default:
return unsupported(
`Unknown R3ResolvedDependencyType: ${R3ResolvedDependencyType[dep.resolved]}`);

View File

@ -212,6 +212,7 @@ export class Identifiers {
static directiveInject: o.ExternalReference = {name: 'ɵɵdirectiveInject', moduleName: CORE};
static invalidFactory: o.ExternalReference = {name: 'ɵɵinvalidFactory', moduleName: CORE};
static invalidFactoryDep: o.ExternalReference = {name: 'ɵɵinvalidFactoryDep', moduleName: CORE};
static templateRefExtractor:
o.ExternalReference = {name: 'ɵɵtemplateRefExtractor', moduleName: CORE};

View File

@ -67,6 +67,7 @@ export enum R3ResolvedDependencyType {
Token = 0,
Attribute = 1,
ChangeDetectorRef = 2,
Invalid = 3,
}
export enum R3FactoryTarget {

View File

@ -18,7 +18,7 @@ export {ɵɵdefineInjectable, defineInjectable, ɵɵdefineInjector, InjectableTy
export {forwardRef, resolveForwardRef, ForwardRefFn} from './forward_ref';
export {Injectable, InjectableDecorator, InjectableProvider} from './injectable';
export {Injector} from './injector';
export {ɵɵinject, inject, INJECTOR} from './injector_compatibility';
export {ɵɵinject, inject, INJECTOR, ɵɵinvalidFactoryDep} from './injector_compatibility';
export {ReflectiveInjector} from './reflective_injector';
export {ClassProvider, ClassSansProvider, ConstructorProvider, ConstructorSansProvider, ExistingProvider, ExistingSansProvider, FactoryProvider, FactorySansProvider, Provider, StaticClassProvider, StaticClassSansProvider, StaticProvider, TypeProvider, ValueProvider, ValueSansProvider} from './interface/provider';
export {ResolvedReflectiveFactory, ResolvedReflectiveProvider} from './reflective_provider';

View File

@ -6,6 +6,8 @@
* found in the LICENSE file at https://angular.io/license
*/
import '../util/ng_dev_mode';
import {Type} from '../interface/type';
import {getClosureSafeProperty} from '../util/property';
import {stringify} from '../util/stringify';
@ -114,6 +116,28 @@ export function ɵɵinject<T>(token: Type<T>| InjectionToken<T>, flags = InjectF
return (_injectImplementation || injectInjectorOnly)(resolveForwardRef(token), flags);
}
/**
* Throws an error indicating that a factory function could not be generated by the compiler for a
* particular class.
*
* This instruction allows the actual error message to be optimized away when ngDevMode is turned
* off, saving bytes of generated code while still providing a good experience in dev mode.
*
* The name of the class is not mentioned here, but will be in the generated factory function name
* and thus in the stack trace.
*
* @codeGenApi
*/
export function ɵɵinvalidFactoryDep(index: number): never {
const msg = ngDevMode ?
`This constructor is not compatible with Angular Dependency Injection because its dependency at index ${index} of the parameter list is invalid.
This can happen if the dependency type is a primitive like a string or if an ancestor of this class is missing an Angular decorator.
Please check that 1) the type for the parameter at index ${index} is correct and 2) the correct Angular decorators are defined for this class and its ancestors.` :
'invalid';
throw new Error(msg);
}
/**
* Injects a token from the currently active injector.
*

View File

@ -8,7 +8,7 @@
import {Type} from '../../interface/type';
import {isForwardRef, resolveForwardRef} from '../forward_ref';
import {ɵɵinject} from '../injector_compatibility';
import {ɵɵinject, ɵɵinvalidFactoryDep} from '../injector_compatibility';
import {getInjectableDef, getInjectorDef, ɵɵdefineInjectable, ɵɵdefineInjector} from '../interface/defs';
@ -23,6 +23,7 @@ export const angularCoreDiEnv: {[name: string]: Function} = {
'ɵɵdefineInjector': ɵɵdefineInjector,
'ɵɵinject': ɵɵinject,
'ɵɵgetFactoryOf': getFactoryOf,
'ɵɵinvalidFactoryDep': ɵɵinvalidFactoryDep,
};
function getFactoryOf<T>(type: Type<any>): ((type?: Type<T>) => T)|null {

View File

@ -7,7 +7,7 @@
*/
import {ChangeDetectorRef} from '../../change_detection/change_detector_ref';
import {CompilerFacade, R3DependencyMetadataFacade, getCompilerFacade} from '../../compiler/compiler_facade';
import {CompilerFacade, R3DependencyMetadataFacade, R3ResolvedDependencyType, getCompilerFacade} from '../../compiler/compiler_facade';
import {Type} from '../../interface/type';
import {ReflectionCapabilities} from '../../reflection/reflection_capabilities';
import {Attribute, Host, Inject, Optional, Self, SkipSelf} from '../metadata';
@ -42,10 +42,7 @@ function reflectDependency(compiler: CompilerFacade, dep: any | any[]): R3Depend
meta.token = token;
}
if (Array.isArray(dep)) {
if (dep.length === 0) {
throw new Error('Dependency array must have arguments.');
}
if (Array.isArray(dep) && dep.length > 0) {
for (let j = 0; j < dep.length; j++) {
const param = dep[j];
if (param === undefined) {
@ -74,6 +71,9 @@ function reflectDependency(compiler: CompilerFacade, dep: any | any[]): R3Depend
setTokenAndResolvedType(param);
}
}
} else if (dep === undefined || (Array.isArray(dep) && dep.length === 0)) {
meta.token = undefined;
meta.resolved = R3ResolvedDependencyType.Invalid;
} else {
setTokenAndResolvedType(dep);
}

View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {ɵɵinject} from '../../di/injector_compatibility';
import {ɵɵinject, ɵɵinvalidFactoryDep} from '../../di/injector_compatibility';
import {ɵɵdefineInjectable, ɵɵdefineInjector} from '../../di/interface/defs';
import * as sanitization from '../../sanitization/sanitization';
import * as r3 from '../index';
@ -42,6 +42,7 @@ export const angularCoreEnv: {[name: string]: Function} =
'ɵɵinject': ɵɵinject,
'ɵɵinjectAttribute': r3.ɵɵinjectAttribute,
'ɵɵinvalidFactory': r3.ɵɵinvalidFactory,
'ɵɵinvalidFactoryDep': ɵɵinvalidFactoryDep,
'ɵɵinjectPipeChangeDetectorRef': r3.ɵɵinjectPipeChangeDetectorRef,
'ɵɵtemplateRefExtractor': r3.ɵɵtemplateRefExtractor,
'ɵɵNgOnChangesFeature': r3.ɵɵNgOnChangesFeature,

View File

@ -342,6 +342,54 @@ ivyEnabled && describe('render3 jit', () => {
expect((TestComponent as any).ɵcmp.viewQuery).not.toBeNull();
});
describe('invalid parameters', () => {
it('should error when creating an @Injectable that extends a class with a faulty parameter', () => {
@Injectable({providedIn: 'root'})
class Legit {
}
@Injectable()
class Base {
constructor(first: Legit, second: any) {}
}
@Injectable({providedIn: 'root'})
class Service extends Base {
}
const ServiceAny = Service as any;
expect(ServiceAny.ɵprov).toBeDefined();
expect(ServiceAny.ɵprov.providedIn).toBe('root');
expect(() => ɵɵinject(Service))
.toThrowError(
/constructor is not compatible with Angular Dependency Injection because its dependency at index 1 of the parameter list is invalid/);
});
it('should error when creating an @Directive that extends an undecorated class with parameters',
() => {
@Injectable({providedIn: 'root'})
class Legit {
}
class BaseDir {
constructor(first: Legit) {}
}
@Directive({selector: 'test'})
class TestDir extends BaseDir {
}
const TestDirAny = TestDir as any;
expect(TestDirAny.ɵfac).toBeDefined();
expect(() => TestDirAny.ɵfac())
.toThrowError(
/constructor is not compatible with Angular Dependency Injection because its dependency at index 0 of the parameter list is invalid/);
});
});
});
it('ensure at least one spec exists', () => {});

View File

@ -897,6 +897,8 @@ export declare function ɵɵinjectPipeChangeDetectorRef(flags?: InjectFlags): Ch
export declare function ɵɵinvalidFactory(): never;
export declare function ɵɵinvalidFactoryDep(index: number): never;
export declare function ɵɵlistener(eventName: string, listenerFn: (e?: any) => any, useCapture?: boolean, eventTargetResolver?: GlobalTargetResolver): typeof ɵɵlistener;
export declare function ɵɵloadQuery<T>(): QueryList<T>;