From c4fd862e15b582d42633d2ea1a2fe3de237b4abd Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Wed, 17 Aug 2016 15:57:02 -0700 Subject: [PATCH] fix(metadata): throw better errors when components are passed to imports or modules are passed to declarations. (#10888) Closes #10823 --- .../compiler/src/metadata_resolver.ts | 25 +++++++- .../compiler/test/metadata_resolver_spec.ts | 58 ++++++++++++++++++- 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/modules/@angular/compiler/src/metadata_resolver.ts b/modules/@angular/compiler/src/metadata_resolver.ts index aed3736c3a..6651227537 100644 --- a/modules/@angular/compiler/src/metadata_resolver.ts +++ b/modules/@angular/compiler/src/metadata_resolver.ts @@ -255,7 +255,12 @@ export class CompileMetadataResolver { } } if (importedModuleType) { - importedModules.push(this.getNgModuleMetadata(importedModuleType, false)); + let importedMeta = this.getNgModuleMetadata(importedModuleType, false); + if (importedMeta === null) { + throw new BaseException( + `Unexpected ${this._getTypeDescriptor(importedType)} '${stringify(importedType)}' imported by the module '${stringify(moduleType)}'`); + } + importedModules.push(importedMeta); } else { throw new BaseException( `Unexpected value '${stringify(importedType)}' imported by the module '${stringify(moduleType)}'`); @@ -280,7 +285,7 @@ export class CompileMetadataResolver { exportedModules.push(exportedModuleMeta); } else { throw new BaseException( - `Unexpected value '${stringify(exportedType)}' exported by the module '${stringify(moduleType)}'`); + `Unexpected ${this._getTypeDescriptor(exportedType)} '${stringify(exportedType)}' exported by the module '${stringify(moduleType)}'`); } }); } @@ -305,7 +310,7 @@ export class CompileMetadataResolver { declaredPipeMeta, moduleType, transitiveModule, declaredPipes, true); } else { throw new BaseException( - `Unexpected value '${stringify(declaredType)}' declared by the module '${stringify(moduleType)}'`); + `Unexpected ${this._getTypeDescriptor(declaredType)} '${stringify(declaredType)}' declared by the module '${stringify(moduleType)}'`); } }); } @@ -397,6 +402,20 @@ export class CompileMetadataResolver { (dirMeta) => { this._getTransitiveViewDirectivesAndPipes(dirMeta, moduleMeta); }); } + private _getTypeDescriptor(type: Type): string { + if (this._directiveResolver.resolve(type, false) !== null) { + return 'directive'; + } else if (this._pipeResolver.resolve(type, false) !== null) { + return 'pipe'; + } else if (this._ngModuleResolver.resolve(type, false) !== null) { + return 'module'; + } else if ((type as any).provide) { + return 'provider'; + } else { + return 'value'; + } + } + private _addTypeToModule(type: Type, moduleType: Type) { const oldModule = this._ngModuleOfTypes.get(type); if (oldModule && oldModule !== moduleType) { diff --git a/modules/@angular/compiler/test/metadata_resolver_spec.ts b/modules/@angular/compiler/test/metadata_resolver_spec.ts index 1127b03c55..b4c8b33631 100644 --- a/modules/@angular/compiler/test/metadata_resolver_spec.ts +++ b/modules/@angular/compiler/test/metadata_resolver_spec.ts @@ -7,7 +7,7 @@ */ import {CompilerConfig} from '@angular/compiler/src/config'; -import {AfterContentChecked, AfterContentInit, AfterViewChecked, AfterViewInit, ChangeDetectionStrategy, Component, Directive, DoCheck, Injectable, OnChanges, OnDestroy, OnInit, SimpleChanges, ViewEncapsulation} from '@angular/core'; +import {AfterContentChecked, AfterContentInit, AfterViewChecked, AfterViewInit, ChangeDetectionStrategy, Component, Directive, DoCheck, Injectable, NgModule, OnChanges, OnDestroy, OnInit, Pipe, SimpleChanges, ViewEncapsulation} from '@angular/core'; import {LIFECYCLE_HOOKS_VALUES} from '@angular/core/src/metadata/lifecycle_hooks'; import {TestBed} from '@angular/core/testing'; import {afterEach, beforeEach, beforeEachProviders, ddescribe, describe, expect, iit, inject, it, xdescribe, xit} from '@angular/core/testing/testing_internal'; @@ -66,6 +66,62 @@ export function main() { expect(() => resolver.getDirectiveMetadata(MyBrokenComp1)) .toThrowError(`Can't resolve all parameters for MyBrokenComp1: (?).`); })); + it('should throw with descriptive error message when a directive is passed to imports', + inject([CompileMetadataResolver], (resolver: CompileMetadataResolver) => { + @NgModule({imports: [ComponentWithoutModuleId]}) + class ModuleWithImportedComponent { + } + expect(() => resolver.getNgModuleMetadata(ModuleWithImportedComponent)) + .toThrowError( + `Unexpected directive 'ComponentWithoutModuleId' imported by the module 'ModuleWithImportedComponent'`); + })); + + it('should throw with descriptive error message when a pipe is passed to imports', + inject([CompileMetadataResolver], (resolver: CompileMetadataResolver) => { + @Pipe({name: 'somePipe'}) + class SomePipe { + } + @NgModule({imports: [SomePipe]}) + class ModuleWithImportedPipe { + } + expect(() => resolver.getNgModuleMetadata(ModuleWithImportedPipe)) + .toThrowError( + `Unexpected pipe 'SomePipe' imported by the module 'ModuleWithImportedPipe'`); + })); + + it('should throw with descriptive error message when a module is passed to declarations', + inject([CompileMetadataResolver], (resolver: CompileMetadataResolver) => { + @NgModule({}) + class SomeModule { + } + @NgModule({declarations: [SomeModule]}) + class ModuleWithDeclaredModule { + } + expect(() => resolver.getNgModuleMetadata(ModuleWithDeclaredModule)) + .toThrowError( + `Unexpected module 'SomeModule' declared by the module 'ModuleWithDeclaredModule'`); + })); + + it('should throw with descriptive error message when null is passed to declarations', + inject([CompileMetadataResolver], (resolver: CompileMetadataResolver) => { + @NgModule({declarations: [null]}) + class ModuleWithNullDeclared { + } + expect(() => resolver.getNgModuleMetadata(ModuleWithNullDeclared)) + .toThrowError( + `Unexpected value 'null' declared by the module 'ModuleWithNullDeclared'`); + })); + + it('should throw with descriptive error message when null is passed to imports', + inject([CompileMetadataResolver], (resolver: CompileMetadataResolver) => { + @NgModule({imports: [null]}) + class ModuleWithNullImported { + } + expect(() => resolver.getNgModuleMetadata(ModuleWithNullImported)) + .toThrowError( + `Unexpected value 'null' imported by the module 'ModuleWithNullImported'`); + })); + it('should throw with descriptive error message when a param token of a dependency is undefined', inject([CompileMetadataResolver], (resolver: CompileMetadataResolver) => {