Skip to content

Commit b3a763a

Browse files
tboschtbosch
committed
fix(compiler): keep DOM.hasProperty in sync between browser and transformer.
Right now, we always return true until we have property schema support (#2014). Fixes #2984 Closes #2981
1 parent 7ee6963 commit b3a763a

4 files changed

Lines changed: 71 additions & 63 deletions

File tree

‎modules/angular2/src/dom/browser_adapter.dart‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,17 +98,19 @@ final _keyCodeToKeyMap = const {
9898
class BrowserDomAdapter extends GenericBrowserDomAdapter {
9999
js.JsFunction _setProperty;
100100
js.JsFunction _getProperty;
101-
js.JsFunction _hasProperty;
102101
BrowserDomAdapter() {
103-
_setProperty = js.context.callMethod('eval', ['(function(el, prop, value) { el[prop] = value; })']);
102+
_setProperty = js.context.callMethod('eval', ['(function(el, prop, value) { if (prop in el) el[prop] = value; })']);
104103
_getProperty = js.context.callMethod('eval', ['(function(el, prop) { return el[prop]; })']);
105-
_hasProperty = js.context.callMethod('eval', ['(function(el, prop) { return prop in el; })']);
106104
}
107105
static void makeCurrent() {
108106
setRootDomAdapter(new BrowserDomAdapter());
109107
}
110-
bool hasProperty(Element element, String name) =>
111-
_hasProperty.apply([element, name]);
108+
bool hasProperty(Element element, String name) {
109+
// Always return true as the serverside version html_adapter.dart does so.
110+
// TODO: change this once we have schema support.
111+
// Note: This nees to kept in sync with html_adapter.dart!
112+
return true;
113+
}
112114

113115
void setProperty(Element element, String name, Object value) =>
114116
_setProperty.apply([element, name, value]);

‎modules/angular2/src/dom/html_adapter.dart‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ class Html5LibDomAdapter implements DomAdapter {
1111
}
1212

1313
hasProperty(element, String name) {
14-
// This is needed for serverside compile to generate the right getters/setters...
14+
// This is needed for serverside compile to generate the right getters/setters.
15+
// TODO: change this once we have property schema support.
16+
// Attention: Keep this in sync with browser_adapter.dart!
1517
return true;
1618
}
1719

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

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,31 +1245,33 @@ export function main() {
12451245
});
12461246
}));
12471247

1248-
describe('Missing property bindings', () => {
1249-
it('should throw on bindings to unknown properties',
1250-
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
1251-
tcb =
1252-
tcb.overrideView(MyComp,
1253-
new viewAnn.View({template: '<div unknown="{{ctxProp}}"></div>'}))
1254-
1255-
PromiseWrapper.catchError(tcb.createAsync(MyComp), (e) => {
1256-
expect(e.message).toEqual(
1257-
`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`);
1258-
async.done();
1259-
return null;
1260-
});
1261-
}));
1262-
1263-
it('should not throw for property binding to a non-existing property when there is a matching directive property',
1264-
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
1265-
tcb.overrideView(
1266-
MyComp,
1267-
new viewAnn.View(
1268-
{template: '<div my-dir [elprop]="ctxProp"></div>', directives: [MyDir]}))
1269-
.createAsync(MyComp)
1270-
.then((val) => { async.done(); });
1271-
}));
1272-
});
1248+
if (!IS_DARTIUM) {
1249+
describe('Missing property bindings', () => {
1250+
it('should throw on bindings to unknown properties',
1251+
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
1252+
tcb =
1253+
tcb.overrideView(MyComp,
1254+
new viewAnn.View({template: '<div unknown="{{ctxProp}}"></div>'}))
1255+
1256+
PromiseWrapper.catchError(tcb.createAsync(MyComp), (e) => {
1257+
expect(e.message).toEqual(
1258+
`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`);
1259+
async.done();
1260+
return null;
1261+
});
1262+
}));
1263+
1264+
it('should not throw for property binding to a non-existing property when there is a matching directive property',
1265+
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
1266+
tcb.overrideView(
1267+
MyComp,
1268+
new viewAnn.View(
1269+
{template: '<div my-dir [elprop]="ctxProp"></div>', directives: [MyDir]}))
1270+
.createAsync(MyComp)
1271+
.then((val) => { async.done(); });
1272+
}));
1273+
});
1274+
}
12731275

12741276
// Disabled until a solution is found, refs:
12751277
// - https://github.com/angular/angular/issues/776

‎modules/angular2/test/render/dom/view/proto_view_builder_spec.ts‎

Lines changed: 34 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -22,38 +22,40 @@ export function main() {
2222
var builder;
2323
beforeEach(() => { builder = new ProtoViewBuilder(el('<div/>'), ViewType.EMBEDDED); });
2424

25-
describe('verification of properties', () => {
26-
27-
it('should throw for unknown properties', () => {
28-
builder.bindElement(el('<div/>')).bindProperty('unknownProperty', emptyExpr());
29-
expect(() => builder.build())
30-
.toThrowError(
31-
`Can't bind to 'unknownProperty' since it isn't a know property of the 'div' element and there are no matching directives with a corresponding property`);
32-
});
33-
34-
it('should allow unknown properties if a directive uses it', () => {
35-
var binder = builder.bindElement(el('<div/>'));
36-
binder.bindDirective(0).bindProperty('someDirProperty', emptyExpr(), 'directiveProperty');
37-
binder.bindProperty('directiveProperty', emptyExpr());
38-
expect(() => builder.build()).not.toThrow();
39-
});
40-
41-
it('should allow unknown properties on custom elements', () => {
42-
var binder = builder.bindElement(el('<some-custom/>'));
43-
binder.bindProperty('unknownProperty', emptyExpr());
44-
expect(() => builder.build()).not.toThrow();
45-
});
46-
47-
it('should throw for unknown properties on custom elements if there is an ng component', () => {
48-
var binder = builder.bindElement(el('<some-custom/>'));
49-
binder.bindProperty('unknownProperty', emptyExpr());
50-
binder.setComponentId('someComponent');
51-
expect(() => builder.build())
52-
.toThrowError(
53-
`Can't bind to 'unknownProperty' since it isn't a know property of the 'some-custom' element and there are no matching directives with a corresponding property`);
54-
});
55-
56-
});
25+
if (!IS_DARTIUM) {
26+
describe('verification of properties', () => {
27+
28+
it('should throw for unknown properties', () => {
29+
builder.bindElement(el('<div/>')).bindProperty('unknownProperty', emptyExpr());
30+
expect(() => builder.build())
31+
.toThrowError(
32+
`Can't bind to 'unknownProperty' since it isn't a know property of the 'div' element and there are no matching directives with a corresponding property`);
33+
});
34+
35+
it('should allow unknown properties if a directive uses it', () => {
36+
var binder = builder.bindElement(el('<div/>'));
37+
binder.bindDirective(0).bindProperty('someDirProperty', emptyExpr(), 'directiveProperty');
38+
binder.bindProperty('directiveProperty', emptyExpr());
39+
expect(() => builder.build()).not.toThrow();
40+
});
41+
42+
it('should allow unknown properties on custom elements', () => {
43+
var binder = builder.bindElement(el('<some-custom/>'));
44+
binder.bindProperty('unknownProperty', emptyExpr());
45+
expect(() => builder.build()).not.toThrow();
46+
});
47+
48+
it('should throw for unknown properties on custom elements if there is an ng component', () => {
49+
var binder = builder.bindElement(el('<some-custom/>'));
50+
binder.bindProperty('unknownProperty', emptyExpr());
51+
binder.setComponentId('someComponent');
52+
expect(() => builder.build())
53+
.toThrowError(
54+
`Can't bind to 'unknownProperty' since it isn't a know property of the 'some-custom' element and there are no matching directives with a corresponding property`);
55+
});
56+
57+
});
58+
}
5759

5860
describe('property normalization', () => {
5961
it('should normalize "innerHtml" to "innerHTML"', () => {

0 commit comments

Comments
 (0)