Skip to content
Permalink
Browse files

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

) (#34340)

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

PR Close #34340
  • Loading branch information
alxhub authored and AndrewKushnir committed Nov 11, 2019
1 parent 4b95ba4 commit 676aca108f0c04ef86ca71f87696ff247ea50f93
@@ -67,6 +67,7 @@ export enum R3ResolvedDependencyType {
Token = 0,
Attribute = 1,
ChangeDetectorRef = 2,
Invalid = 3,
}

export enum R3FactoryTarget {
@@ -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]}`);
@@ -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};
@@ -67,6 +67,7 @@ export enum R3ResolvedDependencyType {
Token = 0,
Attribute = 1,
ChangeDetectorRef = 2,
Invalid = 3,
}

export enum R3FactoryTarget {
@@ -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';
@@ -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.
*
@@ -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 {
@@ -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);
}
@@ -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,
@@ -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', () => {});
@@ -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>;

0 comments on commit 676aca1

Please sign in to comment.
You can’t perform that action at this time.