From 25cd6e4321ca263654294094332847c779d055ed Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Fri, 15 May 2015 10:16:04 +0200 Subject: [PATCH] fix(Compiler): add an error when a directive is null or undefined fixes #1908 --- .../angular2/src/core/compiler/compiler.js | 32 +++++++---- .../test/core/compiler/integration_spec.js | 55 ++++++++++++++++++- .../test/reflection/reflector_spec.js | 7 ++- 3 files changed, 82 insertions(+), 12 deletions(-) diff --git a/modules/angular2/src/core/compiler/compiler.js b/modules/angular2/src/core/compiler/compiler.js index 1ed78f0654..6683638b42 100644 --- a/modules/angular2/src/core/compiler/compiler.js +++ b/modules/angular2/src/core/compiler/compiler.js @@ -92,7 +92,7 @@ export class Compiler { // Used for bootstrapping. compileInHost(componentTypeOrBinding:any):Promise { var componentBinding = this._bindDirective(componentTypeOrBinding); - this._assertTypeIsComponent(componentBinding); + Compiler._assertTypeIsComponent(componentBinding); var directiveMetadata = componentBinding.metadata; return this._render.compileHost(directiveMetadata).then( (hostRenderPv) => { @@ -104,7 +104,7 @@ export class Compiler { compile(component: Type):Promise { var componentBinding = this._bindDirective(component); - this._assertTypeIsComponent(componentBinding); + Compiler._assertTypeIsComponent(componentBinding); var protoView = this._compile(componentBinding); var pvPromise = PromiseWrapper.isPromise(protoView) ? protoView : PromiseWrapper.resolve(protoView); return pvPromise.then( (appProtoView) => { @@ -134,13 +134,21 @@ export class Compiler { if (isBlank(template)) { return null; } - var directives = ListWrapper.map( - this._flattenDirectives(template), - (directive) => this._bindDirective(directive) - ); - var renderTemplate = this._buildRenderTemplate(component, template, directives); + + var directives = this._flattenDirectives(template); + + for (var i = 0; i < directives.length; i++) { + if (!Compiler._isValidDirective(directives[i])) { + throw new BaseException( + `Unexpected directive value '${stringify(directives[i])}' on the View of component '${stringify(component)}'`); + } + } + + var boundDirectives = ListWrapper.map(directives, (directive) => this._bindDirective(directive)); + + var renderTemplate = this._buildRenderTemplate(component, template, boundDirectives); pvPromise = this._render.compile(renderTemplate).then( (renderPv) => { - return this._compileNestedProtoViews(componentBinding, renderPv, directives); + return this._compileNestedProtoViews(componentBinding, renderPv, boundDirectives); }); MapWrapper.set(this._compiling, component, pvPromise); @@ -227,7 +235,7 @@ export class Compiler { return directives; } - _flattenList(tree:List, out:List):void { + _flattenList(tree:List, out:List /**/):void { for (var i = 0; i < tree.length; i++) { var item = tree[i]; if (ListWrapper.isList(item)) { @@ -238,7 +246,11 @@ export class Compiler { } } - _assertTypeIsComponent(directiveBinding:DirectiveBinding):void { + static _isValidDirective(value: any): boolean { + return isPresent(value) && (value instanceof Type || value instanceof Binding); + } + + static _assertTypeIsComponent(directiveBinding:DirectiveBinding):void { if (directiveBinding.metadata.type !== renderApi.DirectiveMetadata.COMPONENT_TYPE) { throw new BaseException(`Could not load '${stringify(directiveBinding.key.token)}' because it is not a component.`); } diff --git a/modules/angular2/test/core/compiler/integration_spec.js b/modules/angular2/test/core/compiler/integration_spec.js index 0ea48dac7e..7ad27a3e6a 100644 --- a/modules/angular2/test/core/compiler/integration_spec.js +++ b/modules/angular2/test/core/compiler/integration_spec.js @@ -9,6 +9,7 @@ import { expect, iit, inject, + IS_DARTIUM, beforeEachBindings, it, xit @@ -823,6 +824,57 @@ export function main() { }); describe("error handling", () => { + it('should report a meaningful error when a directive is missing annotation', + inject([TestBed, AsyncTestCompleter], (tb, async) => { + + tb.overrideView(MyComp, new View({ + directives: [SomeDirectiveMissingAnnotation] + })); + + PromiseWrapper.catchError( + tb.createView(MyComp, {context: ctx}), + (e) => { + expect(e.message).toEqual('No Directive annotation found on SomeDirectiveMissingAnnotation'); + async.done(); + } + ); + })); + + it('should report a meaningful error when a directive is null', + inject([TestBed, AsyncTestCompleter], (tb, async) => { + + tb.overrideView(MyComp, new View({ + directives: [[null]] + })); + + PromiseWrapper.catchError( + tb.createView(MyComp, {context: ctx}), + (e) => { + expect(e.message).toEqual("Unexpected directive value 'null' on the View of component 'MyComp'"); + async.done(); + } + ); + })); + + if (!IS_DARTIUM) { + it('should report a meaningful error when a directive is undefined', + inject([TestBed, AsyncTestCompleter], (tb, async) => { + + var undefinedValue; + + tb.overrideView(MyComp, new View({ + directives: [undefinedValue] + })); + + PromiseWrapper.catchError( + tb.createView(MyComp, {context: ctx}), + (e) => { + expect(e.message).toEqual("Unexpected directive value 'undefined' on the View of component 'MyComp'"); + async.done(); + } + ); + })); + } it('should specify a location of an error that happened during change detection (text)', inject([TestBed, AsyncTestCompleter], (tb, async) => { @@ -1110,7 +1162,6 @@ class MyComp { } } - @Component({ selector: 'component-with-pipes', properties: { @@ -1159,6 +1210,8 @@ class ChildCompUsingService { }) class SomeDirective { } +class SomeDirectiveMissingAnnotation { } + @Component({ selector: 'cmp-with-parent' }) diff --git a/modules/angular2/test/reflection/reflector_spec.js b/modules/angular2/test/reflection/reflector_spec.js index 9e5eb7e7e8..ec313bf0ed 100644 --- a/modules/angular2/test/reflection/reflector_spec.js +++ b/modules/angular2/test/reflection/reflector_spec.js @@ -121,6 +121,11 @@ export function main() { reflector.registerType(TestObj, {"annotations" : [1,2]}); expect(reflector.annotations(TestObj)).toEqual([1,2]); }); + + it("should work for a clas without annotations", () => { + var p = reflector.annotations(ClassWithoutAnnotations); + expect(p).toEqual([]); + }); }); describe("getter", () => { @@ -165,4 +170,4 @@ export function main() { }); }); }); -} \ No newline at end of file +}