Skip to content

Commit

Permalink
fix(compiler): keep DOM.hasProperty in sync between browser and tra…
Browse files Browse the repository at this point in the history
…nsformer.

Right now, we always return true until
we have property schema support (#2014).

Fixes #2984
Closes #2981
  • Loading branch information
tbosch committed Jul 13, 2015
1 parent 7ee6963 commit b3a763a
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 63 deletions.
12 changes: 7 additions & 5 deletions modules/angular2/src/dom/browser_adapter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,19 @@ final _keyCodeToKeyMap = const {
class BrowserDomAdapter extends GenericBrowserDomAdapter {
js.JsFunction _setProperty;
js.JsFunction _getProperty;
js.JsFunction _hasProperty;
BrowserDomAdapter() {
_setProperty = js.context.callMethod('eval', ['(function(el, prop, value) { el[prop] = value; })']);
_setProperty = js.context.callMethod('eval', ['(function(el, prop, value) { if (prop in el) el[prop] = value; })']);
_getProperty = js.context.callMethod('eval', ['(function(el, prop) { return el[prop]; })']);
_hasProperty = js.context.callMethod('eval', ['(function(el, prop) { return prop in el; })']);
}
static void makeCurrent() {
setRootDomAdapter(new BrowserDomAdapter());
}
bool hasProperty(Element element, String name) =>
_hasProperty.apply([element, name]);
bool hasProperty(Element element, String name) {
// Always return true as the serverside version html_adapter.dart does so.
// TODO: change this once we have schema support.
// Note: This nees to kept in sync with html_adapter.dart!
return true;
}

void setProperty(Element element, String name, Object value) =>
_setProperty.apply([element, name, value]);
Expand Down
4 changes: 3 additions & 1 deletion modules/angular2/src/dom/html_adapter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ class Html5LibDomAdapter implements DomAdapter {
}

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

Expand Down
52 changes: 27 additions & 25 deletions modules/angular2/test/core/compiler/integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1245,31 +1245,33 @@ export function main() {
});
}));

describe('Missing property bindings', () => {
it('should throw on bindings to unknown properties',
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
tcb =
tcb.overrideView(MyComp,
new viewAnn.View({template: '<div unknown="{{ctxProp}}"></div>'}))

PromiseWrapper.catchError(tcb.createAsync(MyComp), (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([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
tcb.overrideView(
MyComp,
new viewAnn.View(
{template: '<div my-dir [elprop]="ctxProp"></div>', directives: [MyDir]}))
.createAsync(MyComp)
.then((val) => { async.done(); });
}));
});
if (!IS_DARTIUM) {
describe('Missing property bindings', () => {
it('should throw on bindings to unknown properties',
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
tcb =
tcb.overrideView(MyComp,
new viewAnn.View({template: '<div unknown="{{ctxProp}}"></div>'}))

PromiseWrapper.catchError(tcb.createAsync(MyComp), (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([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
tcb.overrideView(
MyComp,
new viewAnn.View(
{template: '<div my-dir [elprop]="ctxProp"></div>', directives: [MyDir]}))
.createAsync(MyComp)
.then((val) => { async.done(); });
}));
});
}

// Disabled until a solution is found, refs:
// - https://github.com/angular/angular/issues/776
Expand Down
66 changes: 34 additions & 32 deletions modules/angular2/test/render/dom/view/proto_view_builder_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,38 +22,40 @@ export function main() {
var builder;
beforeEach(() => { builder = new ProtoViewBuilder(el('<div/>'), ViewType.EMBEDDED); });

describe('verification of properties', () => {

it('should throw for unknown properties', () => {
builder.bindElement(el('<div/>')).bindProperty('unknownProperty', emptyExpr());
expect(() => builder.build())
.toThrowError(
`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`);
});

it('should allow unknown properties if a directive uses it', () => {
var binder = builder.bindElement(el('<div/>'));
binder.bindDirective(0).bindProperty('someDirProperty', emptyExpr(), 'directiveProperty');
binder.bindProperty('directiveProperty', emptyExpr());
expect(() => builder.build()).not.toThrow();
});

it('should allow unknown properties on custom elements', () => {
var binder = builder.bindElement(el('<some-custom/>'));
binder.bindProperty('unknownProperty', emptyExpr());
expect(() => builder.build()).not.toThrow();
});

it('should throw for unknown properties on custom elements if there is an ng component', () => {
var binder = builder.bindElement(el('<some-custom/>'));
binder.bindProperty('unknownProperty', emptyExpr());
binder.setComponentId('someComponent');
expect(() => builder.build())
.toThrowError(
`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`);
});

});
if (!IS_DARTIUM) {
describe('verification of properties', () => {

it('should throw for unknown properties', () => {
builder.bindElement(el('<div/>')).bindProperty('unknownProperty', emptyExpr());
expect(() => builder.build())
.toThrowError(
`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`);
});

it('should allow unknown properties if a directive uses it', () => {
var binder = builder.bindElement(el('<div/>'));
binder.bindDirective(0).bindProperty('someDirProperty', emptyExpr(), 'directiveProperty');
binder.bindProperty('directiveProperty', emptyExpr());
expect(() => builder.build()).not.toThrow();
});

it('should allow unknown properties on custom elements', () => {
var binder = builder.bindElement(el('<some-custom/>'));
binder.bindProperty('unknownProperty', emptyExpr());
expect(() => builder.build()).not.toThrow();
});

it('should throw for unknown properties on custom elements if there is an ng component', () => {
var binder = builder.bindElement(el('<some-custom/>'));
binder.bindProperty('unknownProperty', emptyExpr());
binder.setComponentId('someComponent');
expect(() => builder.build())
.toThrowError(
`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`);
});

});
}

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

0 comments on commit b3a763a

Please sign in to comment.