feat(compiler): detect dangling property bindings

BREAKING CHANGE: compiler will throw on binding to non-existing properties.

Till now it was possible to have a binding to a non-existing property,
ex.: `<div [foo]="exp">`. From now on this is compilation error - any
property binding needs to have at least one associated property:
eaither on an HTML element or on any directive associated with a
given element (directives' properites need to be declared using the
`properties` field in the `@Directive` / `@Component` annotation).

Closes #2598
This commit is contained in:
Pawel Kozlowski 2015-06-17 16:05:35 +02:00
parent f158fbd131
commit d7b9345b6d
10 changed files with 61 additions and 41 deletions

View File

@ -149,6 +149,7 @@ export class DirectiveParser implements CompileStep {
if (isPresent(bindingAst)) {
directiveBinderBuilder.bindProperty(dirProperty, bindingAst);
}
compileElement.bindElement().bindPropertyToDirective(dashCaseToCamelCase(elProp));
}
_bindDirectiveEvent(eventName, action, compileElement, directiveBinderBuilder) {

View File

@ -18,7 +18,7 @@ const CLASS_PREFIX = 'class.';
const STYLE_PREFIX = 'style.';
export class PropertySetterFactory {
private static _noopSetter(el, value) {}
static noopSetter(el, value) {}
private _lazyPropertySettersCache: StringMap<string, Function> = StringMapWrapper.create();
private _eagerPropertySettersCache: StringMap<string, Function> = StringMapWrapper.create();
@ -69,7 +69,7 @@ export class PropertySetterFactory {
if (DOM.hasProperty(protoElement, property)) {
setterFn = reflector.setter(property);
} else {
setterFn = PropertySetterFactory._noopSetter;
setterFn = PropertySetterFactory.noopSetter;
}
StringMapWrapper.set(this._eagerPropertySettersCache, property, setterFn);
}

View File

@ -75,10 +75,17 @@ export class ProtoViewBuilder {
});
MapWrapper.forEach(ebb.propertyBindings, (_, propertyName) => {
var propSetter =
setterFactory.createSetter(ebb.element, isPresent(ebb.componentId), propertyName);
propertySetters.set(
propertyName,
setterFactory.createSetter(ebb.element, isPresent(ebb.componentId), propertyName));
if (propSetter === PropertySetterFactory.noopSetter) {
if (!SetWrapper.has(ebb.propertyBindingsToDirectives, propertyName)) {
throw new BaseException(
`Can't bind to '${propertyName}' since it isn't a know property of the '${DOM.tagName(ebb.element).toLowerCase()}' element and there are no matching directives with a corresponding property`);
}
}
propertySetters.set(propertyName, propSetter);
});
var nestedProtoView =
@ -170,6 +177,7 @@ export class ElementBinderBuilder {
nestedProtoView: ProtoViewBuilder = null;
propertyBindings: Map<string, ASTWithSource> = new Map();
variableBindings: Map<string, string> = new Map();
propertyBindingsToDirectives: Set<string> = new Set();
eventBindings: List<api.EventBinding> = [];
eventBuilder: EventBuilder = new EventBuilder();
textBindingNodes: List</*node*/ any> = [];
@ -210,6 +218,12 @@ export class ElementBinderBuilder {
bindProperty(name, expression) { this.propertyBindings.set(name, expression); }
bindPropertyToDirective(name: string) {
// we are filling in a set of property names that are bound to a property
// of at least one directive. This allows us to report "dangling" bindings.
this.propertyBindingsToDirectives.add(name);
}
bindVariable(name, value) {
// When current is a view root, the variable bindings are set to the *nested* proto view.
// The root view conceptually signifies a new "block scope" (the nested view), to which

View File

@ -192,22 +192,6 @@ export function main() {
});
}));
it('should ignore bindings to unknown properties',
inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
tb.overrideView(MyComp,
new viewAnn.View({template: '<div unknown="{{ctxProp}}"></div>'}));
tb.createView(MyComp, {context: ctx})
.then((view) => {
ctx.ctxProp = 'Some value';
view.detectChanges();
expect(DOM.hasProperty(view.rootNodes[0], 'unknown')).toBeFalsy();
async.done();
});
}));
it('should consume directive watch expression change.',
inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
var tpl = '<div>' +
@ -247,7 +231,7 @@ export function main() {
it("should support pipes in bindings",
inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
tb.overrideView(MyComp, new viewAnn.View({
template: '<div [my-dir] #dir="mydir" [elprop]="ctxProp | double"></div>',
template: '<div my-dir #dir="mydir" [elprop]="ctxProp | double"></div>',
directives: [MyDir]
}));
@ -442,7 +426,7 @@ export function main() {
it('should assign a directive to a var-',
inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
tb.overrideView(MyComp, new viewAnn.View({
template: '<p><div [export-dir] #localdir="dir"></div></p>',
template: '<p><div export-dir #localdir="dir"></div></p>',
directives: [ExportDir]
}));
@ -1144,7 +1128,7 @@ export function main() {
it('should specify a location of an error that happened during change detection (element property)',
inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
tb.overrideView(MyComp, new viewAnn.View({template: '<div [prop]="a.b"></div>'}));
tb.overrideView(MyComp, new viewAnn.View({template: '<div [title]="a.b"></div>'}));
tb.createView(MyComp, {context: ctx})
.then((view) => {
@ -1157,10 +1141,10 @@ export function main() {
it('should specify a location of an error that happened during change detection (directive property)',
inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
tb.overrideView(
MyComp,
new viewAnn.View(
{template: '<child-cmp [prop]="a.b"></child-cmp>', directives: [ChildComp]}));
tb.overrideView(MyComp, new viewAnn.View({
template: '<child-cmp [dir-prop]="a.b"></child-cmp>',
directives: [ChildComp]
}));
tb.createView(MyComp, {context: ctx})
.then((view) => {
@ -1205,6 +1189,30 @@ export function main() {
});
}));
describe('Missing property bindings', () => {
it('should throw on bindings to unknown properties',
inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
tb.overrideView(MyComp,
new viewAnn.View({template: '<div unknown="{{ctxProp}}"></div>'}));
PromiseWrapper.catchError(tb.createView(MyComp, {context: ctx}), (e) => {
expect(e.message).toEqual(
`Can't bind to 'unknown' since it isn't a know property of the 'div' element and there are no matching directives with a corresponding property`);
async.done();
return null;
});
}));
it('should not throw for property binding to a non-existing property when there is a matching directive property',
inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
tb.overrideView(
MyComp,
new viewAnn.View(
{template: '<div my-dir [elprop]="ctxProp"></div>', directives: [MyDir]}));
tb.createView(MyComp, {context: ctx}).then((val) => { async.done(); });
}));
});
// Disabled until a solution is found, refs:
// - https://github.com/angular/angular/issues/776
// - https://github.com/angular/angular/commit/81f3f32
@ -1353,10 +1361,7 @@ class ComponentWithPipes {
prop: string;
}
@Component({
selector: 'child-cmp',
appInjector: [MyService],
})
@Component({selector: 'child-cmp', properties: ['dirProp'], appInjector: [MyService]})
@View({directives: [MyDir], template: '{{ctxProp}}'})
@Injectable()
class ChildComp {

View File

@ -36,7 +36,7 @@ export function main() {
tb.overrideView(
MyComp,
new viewAnn.View(
{template: '<div [field]="123" [lifecycle]></div>', directives: [LifecycleDir]}));
{template: '<div [field]="123" lifecycle></div>', directives: [LifecycleDir]}));
tb.createView(MyComp, {context: ctx})
.then((view) => {

View File

@ -205,7 +205,7 @@ export function main() {
it('should repeat over nested arrays with no intermediate element',
inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
var template = '<div><template [ng-for] #item [ng-for-of]="items">' +
var template = '<div><template ng-for #item [ng-for-of]="items">' +
'<div template="ng-for #subitem of item">' +
'{{subitem}}-{{item.length}};' +
'</div></template></div>';

View File

@ -79,8 +79,8 @@ export function main() {
'<template [ng-switch-when]="\'b\'"><li>when b1;</li></template>' +
'<template [ng-switch-when]="\'a\'"><li>when a2;</li></template>' +
'<template [ng-switch-when]="\'b\'"><li>when b2;</li></template>' +
'<template [ng-switch-default]><li>when default1;</li></template>' +
'<template [ng-switch-default]><li>when default2;</li></template>' +
'<template ng-switch-default><li>when default1;</li></template>' +
'<template ng-switch-default><li>when default2;</li></template>' +
'</ul></div>';
tb.createView(TestComponent, {html: template})
@ -108,7 +108,7 @@ export function main() {
'<ul [ng-switch]="switchValue">' +
'<template [ng-switch-when]="when1"><li>when 1;</li></template>' +
'<template [ng-switch-when]="when2"><li>when 2;</li></template>' +
'<template [ng-switch-default]><li>when default;</li></template>' +
'<template ng-switch-default><li>when default;</li></template>' +
'</ul></div>';
tb.createView(TestComponent, {html: template})

View File

@ -511,7 +511,7 @@ var conditionalContentComponent = DirectiveMetadata.create({
});
var autoViewportDirective = DirectiveMetadata.create(
{selector: '[auto]', id: '[auto]', type: DirectiveMetadata.DIRECTIVE_TYPE});
{selector: '[auto]', id: 'auto', properties: ['auto'], type: DirectiveMetadata.DIRECTIVE_TYPE});
var tabComponent =
DirectiveMetadata.create({selector: 'tab', id: 'tab', type: DirectiveMetadata.COMPONENT_TYPE});

View File

@ -77,7 +77,7 @@ class DynamicDummy {
</div>
<div *ng-if="testingWithDirectives">
<dummy [dummy-decorator] *ng-for="#i of list"></dummy>
<dummy dummy-decorator *ng-for="#i of list"></dummy>
</div>
<div *ng-if="testingDynamicComponents">

View File

@ -232,7 +232,7 @@ class CellData {
</tbody>
<tbody template="ng-switch-when 'interpolationAttr'">
<tr template="ng-for #row of data">
<td template="ng-for #column of row" i="{{column.i}}" j="{{column.j}}">
<td template="ng-for #column of row" attr.i="{{column.i}}" attr.j="{{column.j}}">
i,j attrs
</td>
</tr>
@ -269,7 +269,7 @@ class LargetableComponent {
@Component({selector: 'app'})
@View({
directives: [LargetableComponent],
template: `<largetable [data]='data' [benchmarkType]='benchmarkType'></largetable>`
template: `<largetable [data]='data' [benchmark-type]='benchmarkType'></largetable>`
})
class AppComponent {
data;