diff --git a/packages/compiler/src/compiler_facade_interface.ts b/packages/compiler/src/compiler_facade_interface.ts index 05da363184..79d06d41c1 100644 --- a/packages/compiler/src/compiler_facade_interface.ts +++ b/packages/compiler/src/compiler_facade_interface.ts @@ -67,6 +67,7 @@ export enum R3ResolvedDependencyType { Token = 0, Attribute = 1, ChangeDetectorRef = 2, + Invalid = 3, } export enum R3FactoryTarget { diff --git a/packages/compiler/src/render3/r3_factory.ts b/packages/compiler/src/render3/r3_factory.ts index 10d1edc7e7..7fe5eefc48 100644 --- a/packages/compiler/src/render3/r3_factory.ts +++ b/packages/compiler/src/render3/r3_factory.ts @@ -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]}`); diff --git a/packages/compiler/src/render3/r3_identifiers.ts b/packages/compiler/src/render3/r3_identifiers.ts index 63f22321a5..66d0b0a5b9 100644 --- a/packages/compiler/src/render3/r3_identifiers.ts +++ b/packages/compiler/src/render3/r3_identifiers.ts @@ -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}; diff --git a/packages/core/src/compiler/compiler_facade_interface.ts b/packages/core/src/compiler/compiler_facade_interface.ts index 05da363184..79d06d41c1 100644 --- a/packages/core/src/compiler/compiler_facade_interface.ts +++ b/packages/core/src/compiler/compiler_facade_interface.ts @@ -67,6 +67,7 @@ export enum R3ResolvedDependencyType { Token = 0, Attribute = 1, ChangeDetectorRef = 2, + Invalid = 3, } export enum R3FactoryTarget { diff --git a/packages/core/src/di/index.ts b/packages/core/src/di/index.ts index 4281c5dc81..e941091a9a 100644 --- a/packages/core/src/di/index.ts +++ b/packages/core/src/di/index.ts @@ -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'; diff --git a/packages/core/src/di/injector_compatibility.ts b/packages/core/src/di/injector_compatibility.ts index 04abc56919..bb420a7b23 100644 --- a/packages/core/src/di/injector_compatibility.ts +++ b/packages/core/src/di/injector_compatibility.ts @@ -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(token: Type| InjectionToken, 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. * diff --git a/packages/core/src/di/jit/environment.ts b/packages/core/src/di/jit/environment.ts index 3d2fc5a497..d544fcc446 100644 --- a/packages/core/src/di/jit/environment.ts +++ b/packages/core/src/di/jit/environment.ts @@ -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(type: Type): ((type?: Type) => T)|null { diff --git a/packages/core/src/di/jit/util.ts b/packages/core/src/di/jit/util.ts index 51270cc21c..6ba1920d17 100644 --- a/packages/core/src/di/jit/util.ts +++ b/packages/core/src/di/jit/util.ts @@ -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); } diff --git a/packages/core/src/render3/jit/environment.ts b/packages/core/src/render3/jit/environment.ts index bab9d77657..5e4612edb2 100644 --- a/packages/core/src/render3/jit/environment.ts +++ b/packages/core/src/render3/jit/environment.ts @@ -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, diff --git a/packages/core/test/render3/ivy/jit_spec.ts b/packages/core/test/render3/ivy/jit_spec.ts index 2f033346b3..37d9572772 100644 --- a/packages/core/test/render3/ivy/jit_spec.ts +++ b/packages/core/test/render3/ivy/jit_spec.ts @@ -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', () => {}); diff --git a/tools/public_api_guard/core/core.d.ts b/tools/public_api_guard/core/core.d.ts index b31ed025c7..7d0e7e2417 100644 --- a/tools/public_api_guard/core/core.d.ts +++ b/tools/public_api_guard/core/core.d.ts @@ -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(): QueryList;