From b3a763a718610aa322848aef009b70c112f33377 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Mon, 13 Jul 2015 11:43:56 -0700 Subject: [PATCH] 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 --- modules/angular2/src/dom/browser_adapter.dart | 12 ++-- modules/angular2/src/dom/html_adapter.dart | 4 +- .../test/core/compiler/integration_spec.ts | 52 ++++++++------- .../dom/view/proto_view_builder_spec.ts | 66 ++++++++++--------- 4 files changed, 71 insertions(+), 63 deletions(-) diff --git a/modules/angular2/src/dom/browser_adapter.dart b/modules/angular2/src/dom/browser_adapter.dart index 49293a2be3..db4a160d5f 100644 --- a/modules/angular2/src/dom/browser_adapter.dart +++ b/modules/angular2/src/dom/browser_adapter.dart @@ -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]); diff --git a/modules/angular2/src/dom/html_adapter.dart b/modules/angular2/src/dom/html_adapter.dart index 28de7a1782..d8b36ca78d 100644 --- a/modules/angular2/src/dom/html_adapter.dart +++ b/modules/angular2/src/dom/html_adapter.dart @@ -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; } diff --git a/modules/angular2/test/core/compiler/integration_spec.ts b/modules/angular2/test/core/compiler/integration_spec.ts index 122858bb38..e019d97468 100644 --- a/modules/angular2/test/core/compiler/integration_spec.ts +++ b/modules/angular2/test/core/compiler/integration_spec.ts @@ -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: '
'})) - - 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: '
', 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: '
'})) + + 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: '
', directives: [MyDir]})) + .createAsync(MyComp) + .then((val) => { async.done(); }); + })); + }); + } // Disabled until a solution is found, refs: // - https://github.com/angular/angular/issues/776 diff --git a/modules/angular2/test/render/dom/view/proto_view_builder_spec.ts b/modules/angular2/test/render/dom/view/proto_view_builder_spec.ts index d21d6f2599..a627d64a25 100644 --- a/modules/angular2/test/render/dom/view/proto_view_builder_spec.ts +++ b/modules/angular2/test/render/dom/view/proto_view_builder_spec.ts @@ -22,38 +22,40 @@ export function main() { var builder; beforeEach(() => { builder = new ProtoViewBuilder(el('
'), ViewType.EMBEDDED); }); - describe('verification of properties', () => { - - it('should throw for unknown properties', () => { - builder.bindElement(el('
')).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('
')); - 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('')); - 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('')); - 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('
')).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('
')); + 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('')); + 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('')); + 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"', () => {