Skip to content

Commit 25cd6e4

Browse files
committed
fix(Compiler): add an error when a directive is null or undefined
fixes #1908
1 parent 666336b commit 25cd6e4

3 files changed

Lines changed: 82 additions & 12 deletions

File tree

‎modules/angular2/src/core/compiler/compiler.js‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ export class Compiler {
9292
// Used for bootstrapping.
9393
compileInHost(componentTypeOrBinding:any):Promise<ProtoViewRef> {
9494
var componentBinding = this._bindDirective(componentTypeOrBinding);
95-
this._assertTypeIsComponent(componentBinding);
95+
Compiler._assertTypeIsComponent(componentBinding);
9696

9797
var directiveMetadata = componentBinding.metadata;
9898
return this._render.compileHost(directiveMetadata).then( (hostRenderPv) => {
@@ -104,7 +104,7 @@ export class Compiler {
104104

105105
compile(component: Type):Promise<ProtoViewRef> {
106106
var componentBinding = this._bindDirective(component);
107-
this._assertTypeIsComponent(componentBinding);
107+
Compiler._assertTypeIsComponent(componentBinding);
108108
var protoView = this._compile(componentBinding);
109109
var pvPromise = PromiseWrapper.isPromise(protoView) ? protoView : PromiseWrapper.resolve(protoView);
110110
return pvPromise.then( (appProtoView) => {
@@ -134,13 +134,21 @@ export class Compiler {
134134
if (isBlank(template)) {
135135
return null;
136136
}
137-
var directives = ListWrapper.map(
138-
this._flattenDirectives(template),
139-
(directive) => this._bindDirective(directive)
140-
);
141-
var renderTemplate = this._buildRenderTemplate(component, template, directives);
137+
138+
var directives = this._flattenDirectives(template);
139+
140+
for (var i = 0; i < directives.length; i++) {
141+
if (!Compiler._isValidDirective(directives[i])) {
142+
throw new BaseException(
143+
`Unexpected directive value '${stringify(directives[i])}' on the View of component '${stringify(component)}'`);
144+
}
145+
}
146+
147+
var boundDirectives = ListWrapper.map(directives, (directive) => this._bindDirective(directive));
148+
149+
var renderTemplate = this._buildRenderTemplate(component, template, boundDirectives);
142150
pvPromise = this._render.compile(renderTemplate).then( (renderPv) => {
143-
return this._compileNestedProtoViews(componentBinding, renderPv, directives);
151+
return this._compileNestedProtoViews(componentBinding, renderPv, boundDirectives);
144152
});
145153

146154
MapWrapper.set(this._compiling, component, pvPromise);
@@ -227,7 +235,7 @@ export class Compiler {
227235
return directives;
228236
}
229237

230-
_flattenList(tree:List<any>, out:List<Type>):void {
238+
_flattenList(tree:List<any>, out:List<any> /*<Type|Binding>*/):void {
231239
for (var i = 0; i < tree.length; i++) {
232240
var item = tree[i];
233241
if (ListWrapper.isList(item)) {
@@ -238,7 +246,11 @@ export class Compiler {
238246
}
239247
}
240248

241-
_assertTypeIsComponent(directiveBinding:DirectiveBinding):void {
249+
static _isValidDirective(value: any): boolean {
250+
return isPresent(value) && (value instanceof Type || value instanceof Binding);
251+
}
252+
253+
static _assertTypeIsComponent(directiveBinding:DirectiveBinding):void {
242254
if (directiveBinding.metadata.type !== renderApi.DirectiveMetadata.COMPONENT_TYPE) {
243255
throw new BaseException(`Could not load '${stringify(directiveBinding.key.token)}' because it is not a component.`);
244256
}

‎modules/angular2/test/core/compiler/integration_spec.js‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
expect,
1010
iit,
1111
inject,
12+
IS_DARTIUM,
1213
beforeEachBindings,
1314
it,
1415
xit
@@ -823,6 +824,57 @@ export function main() {
823824
});
824825

825826
describe("error handling", () => {
827+
it('should report a meaningful error when a directive is missing annotation',
828+
inject([TestBed, AsyncTestCompleter], (tb, async) => {
829+
830+
tb.overrideView(MyComp, new View({
831+
directives: [SomeDirectiveMissingAnnotation]
832+
}));
833+
834+
PromiseWrapper.catchError(
835+
tb.createView(MyComp, {context: ctx}),
836+
(e) => {
837+
expect(e.message).toEqual('No Directive annotation found on SomeDirectiveMissingAnnotation');
838+
async.done();
839+
}
840+
);
841+
}));
842+
843+
it('should report a meaningful error when a directive is null',
844+
inject([TestBed, AsyncTestCompleter], (tb, async) => {
845+
846+
tb.overrideView(MyComp, new View({
847+
directives: [[null]]
848+
}));
849+
850+
PromiseWrapper.catchError(
851+
tb.createView(MyComp, {context: ctx}),
852+
(e) => {
853+
expect(e.message).toEqual("Unexpected directive value 'null' on the View of component 'MyComp'");
854+
async.done();
855+
}
856+
);
857+
}));
858+
859+
if (!IS_DARTIUM) {
860+
it('should report a meaningful error when a directive is undefined',
861+
inject([TestBed, AsyncTestCompleter], (tb, async) => {
862+
863+
var undefinedValue;
864+
865+
tb.overrideView(MyComp, new View({
866+
directives: [undefinedValue]
867+
}));
868+
869+
PromiseWrapper.catchError(
870+
tb.createView(MyComp, {context: ctx}),
871+
(e) => {
872+
expect(e.message).toEqual("Unexpected directive value 'undefined' on the View of component 'MyComp'");
873+
async.done();
874+
}
875+
);
876+
}));
877+
}
826878

827879
it('should specify a location of an error that happened during change detection (text)',
828880
inject([TestBed, AsyncTestCompleter], (tb, async) => {
@@ -1110,7 +1162,6 @@ class MyComp {
11101162
}
11111163
}
11121164

1113-
11141165
@Component({
11151166
selector: 'component-with-pipes',
11161167
properties: {
@@ -1159,6 +1210,8 @@ class ChildCompUsingService {
11591210
})
11601211
class SomeDirective { }
11611212

1213+
class SomeDirectiveMissingAnnotation { }
1214+
11621215
@Component({
11631216
selector: 'cmp-with-parent'
11641217
})

‎modules/angular2/test/reflection/reflector_spec.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,11 @@ export function main() {
121121
reflector.registerType(TestObj, {"annotations" : [1,2]});
122122
expect(reflector.annotations(TestObj)).toEqual([1,2]);
123123
});
124+
125+
it("should work for a clas without annotations", () => {
126+
var p = reflector.annotations(ClassWithoutAnnotations);
127+
expect(p).toEqual([]);
128+
});
124129
});
125130

126131
describe("getter", () => {
@@ -165,4 +170,4 @@ export function main() {
165170
});
166171
});
167172
});
168-
}
173+
}

0 commit comments

Comments
 (0)