diff --git a/modules/@angular/compiler/src/metadata_resolver.ts b/modules/@angular/compiler/src/metadata_resolver.ts index 273c5c8021..d632d7688e 100644 --- a/modules/@angular/compiler/src/metadata_resolver.ts +++ b/modules/@angular/compiler/src/metadata_resolver.ts @@ -146,8 +146,8 @@ export class CompileMetadataResolver { changeDetectionStrategy = cmpMeta.changeDetection; if (isPresent(dirMeta.viewProviders)) { viewProviders = this.getProvidersMetadata( - verifyNonBlankProviders(directiveType, dirMeta.viewProviders, 'viewProviders'), - entryComponentMetadata); + dirMeta.viewProviders, entryComponentMetadata, + `viewProviders for "${stringify(directiveType)}"`); } moduleUrl = componentModuleUrl(this._reflector, directiveType, cmpMeta); if (cmpMeta.entryComponents) { @@ -169,8 +169,8 @@ export class CompileMetadataResolver { var providers: Array = []; if (isPresent(dirMeta.providers)) { providers = this.getProvidersMetadata( - verifyNonBlankProviders(directiveType, dirMeta.providers, 'providers'), - entryComponentMetadata); + dirMeta.providers, entryComponentMetadata, + `providers for "${stringify(directiveType)}"`); } var queries: cpl.CompileQueryMetadata[] = []; var viewQueries: cpl.CompileQueryMetadata[] = []; @@ -227,8 +227,9 @@ export class CompileMetadataResolver { const moduleWithProviders: ModuleWithProviders = importedType; importedModuleType = moduleWithProviders.ngModule; if (moduleWithProviders.providers) { - providers.push( - ...this.getProvidersMetadata(moduleWithProviders.providers, entryComponents)); + providers.push(...this.getProvidersMetadata( + moduleWithProviders.providers, entryComponents, + `provider for the NgModule '${stringify(importedModuleType)}'`)); } } if (importedModuleType) { @@ -295,7 +296,9 @@ export class CompileMetadataResolver { // The providers of the module have to go last // so that they overwrite any other provider we already added. if (meta.providers) { - providers.push(...this.getProvidersMetadata(meta.providers, entryComponents)); + providers.push(...this.getProvidersMetadata( + meta.providers, entryComponents, + `provider for the NgModule '${stringify(moduleType)}'`)); } if (meta.entryComponents) { entryComponents.push( @@ -550,17 +553,18 @@ export class CompileMetadataResolver { return compileToken; } - getProvidersMetadata(providers: Provider[], targetEntryComponents: cpl.CompileTypeMetadata[]): - Array { + getProvidersMetadata( + providers: Provider[], targetEntryComponents: cpl.CompileTypeMetadata[], + debugInfo?: string): Array { const compileProviders: Array = []; - providers.forEach((provider: any) => { + providers.forEach((provider: any, providerIdx: number) => { provider = resolveForwardRef(provider); if (provider && typeof provider == 'object' && provider.hasOwnProperty('provide')) { provider = new cpl.ProviderMeta(provider.provide, provider); } let compileProvider: cpl.CompileProviderMetadata|cpl.CompileTypeMetadata|any[]; if (isArray(provider)) { - compileProvider = this.getProvidersMetadata(provider, targetEntryComponents); + compileProvider = this.getProvidersMetadata(provider, targetEntryComponents, debugInfo); } else if (provider instanceof cpl.ProviderMeta) { let tokenMeta = this.getTokenMetadata(provider.token); if (tokenMeta.equalsTo(identifierToken(Identifiers.ANALYZE_FOR_ENTRY_COMPONENTS))) { @@ -571,8 +575,22 @@ export class CompileMetadataResolver { } else if (isValidType(provider)) { compileProvider = this.getTypeMetadata(provider, staticTypeModuleUrl(provider)); } else { + let providersInfo = (providers.reduce( + (soFar: string[], seenProvider: any, seenProviderIdx: number) => { + if (seenProviderIdx < providerIdx) { + soFar.push(`${stringify(seenProvider)}`); + } else if (seenProviderIdx == providerIdx) { + soFar.push(`?${stringify(seenProvider)}?`); + } else if (seenProviderIdx == providerIdx + 1) { + soFar.push('...'); + } + return soFar; + }, + [])) + .join(', '); + throw new BaseException( - `Invalid provider - only instances of Provider and Type are allowed, got: ${stringify(provider)}`); + `Invalid ${debugInfo ? debugInfo : 'provider'} - only instances of Provider and Type are allowed, got: [${providersInfo}]`); } if (compileProvider) { compileProviders.push(compileProvider); @@ -696,23 +714,6 @@ function flattenArray(tree: any[], out: Array = []): Array { return out; } -function verifyNonBlankProviders( - directiveType: Type, providersTree: any[], providersType: string): any[] { - var flat: any[] = []; - var errMsg: string; - - flattenArray(providersTree, flat); - for (var i = 0; i < flat.length; i++) { - if (isBlank(flat[i])) { - errMsg = flat.map(provider => isBlank(provider) ? '?' : stringify(provider)).join(', '); - throw new BaseException( - `One or more of ${providersType} for "${stringify(directiveType)}" were not defined: [${errMsg}].`); - } - } - - return providersTree; -} - function isValidType(value: any): boolean { return cpl.isStaticSymbol(value) || (value instanceof Type); } diff --git a/modules/@angular/compiler/test/metadata_resolver_spec.ts b/modules/@angular/compiler/test/metadata_resolver_spec.ts index 59e5ca0d20..70d50c9d92 100644 --- a/modules/@angular/compiler/test/metadata_resolver_spec.ts +++ b/modules/@angular/compiler/test/metadata_resolver_spec.ts @@ -130,14 +130,14 @@ export function main() { inject([CompileMetadataResolver], (resolver: CompileMetadataResolver) => { expect(() => resolver.getDirectiveMetadata(MyBrokenComp3)) .toThrowError( - `One or more of providers for "MyBrokenComp3" were not defined: [?, SimpleService, ?].`); + `Invalid providers for "MyBrokenComp3" - only instances of Provider and Type are allowed, got: [SimpleService, ?null?, ...]`); })); it('should throw with descriptive error message when one of viewProviders is not present', inject([CompileMetadataResolver], (resolver: CompileMetadataResolver) => { expect(() => resolver.getDirectiveMetadata(MyBrokenComp4)) .toThrowError( - `One or more of viewProviders for "MyBrokenComp4" were not defined: [?, SimpleService, ?].`); + `Invalid viewProviders for "MyBrokenComp4" - only instances of Provider and Type are allowed, got: [?null?, ...]`); })); it('should throw an error when the interpolation config has invalid symbols', @@ -220,7 +220,7 @@ class MyBrokenComp2 { class SimpleService { } -@Component({selector: 'my-broken-comp', template: '', providers: [null, SimpleService, [null]]}) +@Component({selector: 'my-broken-comp', template: '', providers: [SimpleService, null, [null]]}) class MyBrokenComp3 { } diff --git a/modules/@angular/core/test/linker/ng_module_integration_spec.ts b/modules/@angular/core/test/linker/ng_module_integration_spec.ts index cf086a2854..42c28af8be 100644 --- a/modules/@angular/core/test/linker/ng_module_integration_spec.ts +++ b/modules/@angular/core/test/linker/ng_module_integration_spec.ts @@ -732,7 +732,13 @@ function declareTests({useJit}: {useJit: boolean}) { it('should throw when given invalid providers', () => { expect(() => createInjector(['blah'])) .toThrowError( - 'Invalid provider - only instances of Provider and Type are allowed, got: blah'); + `Invalid provider for the NgModule 'SomeModule' - only instances of Provider and Type are allowed, got: [?blah?]`); + }); + + it('should throw when given blank providers', () => { + expect(() => createInjector([null, {provide: 'token', useValue: 'value'}])) + .toThrowError( + `Invalid provider for the NgModule 'SomeModule' - only instances of Provider and Type are allowed, got: [?null?, ...]`); }); it('should provide itself', () => { @@ -1104,6 +1110,20 @@ function declareTests({useJit}: {useJit: boolean}) { const injector = createModule(SomeModule).injector; expect(injector.get('token1')).toBe('imported2'); }); + + it('should throw when given invalid providers in an imported ModuleWithProviders', () => { + @NgModule() + class ImportedModule1 { + } + + @NgModule({imports: [{ngModule: ImportedModule1, providers: ['broken']}]}) + class SomeModule { + } + + expect(() => createModule(SomeModule).injector) + .toThrowError( + `Invalid provider for the NgModule 'ImportedModule1' - only instances of Provider and Type are allowed, got: [?broken?]`); + }); }); }); });