From c93641f8ee441b3d948e47f8f1a8b75ed6d500f7 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Fri, 3 Feb 2017 17:56:31 -0500 Subject: [PATCH 1/5] Test for defining a DOM element Breaking test for #145 --- define-test.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/define-test.js b/define-test.js index 872ef37..b6a9353 100644 --- a/define-test.js +++ b/define-test.js @@ -4,6 +4,7 @@ var define = require("can-define"); var stache = require("can-stache"); var CanList = require("can-list"); var canBatch = require("can-event/batch/batch"); +var domDispatch = require("can-util/dom/dispatch/dispatch"); var isArray = require("can-util/js/is-array/is-array"); var each = require("can-util/js/each/each"); var types = require("can-types"); @@ -1504,3 +1505,23 @@ QUnit.test('setter with default value causes an infinite loop (#142)', function( var a = new A(); QUnit.equal(a.val, 'hello', 'creating an instance should not cause an inifinte loop'); }); + +QUnit.test('Works with DOM elements', function(){ + var el = document.createElement('div'); + define(el, { foo: 'string' }); + + var events = 0; + el.addEventListener('foo', function(){ + events++; + }); + + el.addEventListener('some-event', function(){ + events++; + }); + + el.foo = 'bar'; + QUnit.equal(events, 1, 'An event occurred'); + + domDispatch.call(el, "some-event"); + QUnit.equal(events, 2, 'Another event'); +}); From 4fbb7ebc549fe083e80d9d7a11d8cf448996e3ba Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Fri, 3 Feb 2017 18:26:52 -0500 Subject: [PATCH 2/5] Try using the element's own addEventListener This tries defering to the element's own addEventListener. --- can-define.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/can-define.js b/can-define.js index 5f1226e..a760baa 100644 --- a/can-define.js +++ b/can-define.js @@ -83,7 +83,6 @@ module.exports = define = ns.define = function(objPrototype, defines, baseDefine return data; }); - // Add necessary event methods to this object. for (var prop in eventsProto) { Object.defineProperty(objPrototype, prop, { @@ -177,7 +176,7 @@ define.property = function(objPrototype, prop, definition, dataInitializers, com if ((definition.value !== undefined || definition.Value !== undefined)) { getInitialValue = make.get.defaultValue(prop, definition, typeConvert, eventsSetter); } - + // If property has a getter, create the compute that stores its data. if (definition.get) { computedInitializers[prop] = make.compute(prop, definition.get, getInitialValue); @@ -657,7 +656,10 @@ assign(eventsProto, { } - return eventLifecycle.addAndSetup.apply(this, arguments); + var baseAddEventListener = this.__proto__.addEventListener || + eventLifecycle.addAndSetup; + + return baseAddEventListener.apply(this, arguments); }, // ### unbind From aead3d7d43e4f3344450a7ec94c6e8d4555fdf5f Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Tue, 7 Feb 2017 16:30:03 -0500 Subject: [PATCH 3/5] Use existing add/removeEventListener for elements This makes it possible to use can-define with HTML elements so that both property events and DOM events can be listened to and fire correctly. This is done by using the can-event/lifecycle/lifecycle mixin, and passing it our existing addEventListener (where it exists), rather than using canEvent.addEventListener as lifecycle did before. Closes #145 --- can-define.js | 114 ++++++++++++++++++++++++++++---------------------- list/list.js | 10 +---- map/map.js | 13 ++---- 3 files changed, 69 insertions(+), 68 deletions(-) diff --git a/can-define.js b/can-define.js index a760baa..ea83d13 100644 --- a/can-define.js +++ b/can-define.js @@ -83,15 +83,9 @@ module.exports = define = ns.define = function(objPrototype, defines, baseDefine return data; }); - // Add necessary event methods to this object. - for (var prop in eventsProto) { - Object.defineProperty(objPrototype, prop, { - enumerable: false, - value: eventsProto[prop], - configurable: true, - writable: true - }); - } + // Mixin the event methods + define.mixinEvents(objPrototype); + // add so instance defs can be dynamically added Object.defineProperty(objPrototype,"_define",{ enumerable: false, @@ -280,7 +274,7 @@ make = { if (newVal !== current) { setData.call(this, newVal); - canEvent.dispatch.call(this, { + canEvent.trigger.call(this, { type: prop, target: this }, [newVal, current]); @@ -642,50 +636,12 @@ replaceWith = function(obj, prop, cb, writable) { eventsProto = assign({}, event); assign(eventsProto, { _eventSetup: function() {}, - _eventTeardown: function() {}, - addEventListener: function(eventName, handler) { - - var computedBinding = this._computed && this._computed[eventName]; - if (computedBinding && computedBinding.compute) { - if (!computedBinding.count) { - computedBinding.count = 1; - computedBinding.compute.addEventListener("change", computedBinding.handler); - } else { - computedBinding.count++; - } - - } - - var baseAddEventListener = this.__proto__.addEventListener || - eventLifecycle.addAndSetup; - - return baseAddEventListener.apply(this, arguments); - }, - - // ### unbind - // Stops listening to an event. - // If this is the last listener of a computed property, - // stop forwarding events of the computed property to this map. - removeEventListener: function(eventName, handler) { - var computedBinding = this._computed && this._computed[eventName]; - if (computedBinding) { - if (computedBinding.count === 1) { - computedBinding.count = 0; - computedBinding.compute.removeEventListener("change", computedBinding.handler); - } else { - computedBinding.count--; - } - - } - - return eventLifecycle.removeAndTeardown.apply(this, arguments); - - } + _eventTeardown: function() {} }); -eventsProto.on = eventsProto.bind = eventsProto.addEventListener; -eventsProto.off = eventsProto.unbind = eventsProto.removeEventListener; delete eventsProto.one; +delete eventsProto.addEventListener; +delete eventsProto.removeEventListener; define.setup = function(props, sealed) { defineConfigurableAndNotEnumerable(this, "_cid"); @@ -721,6 +677,62 @@ define.setup = function(props, sealed) { }; define.replaceWith = replaceWith; define.eventsProto = eventsProto; +define.mixinEvents = function(objPrototype, makeEnumerable){ + // Add necessary event methods to this object. + for (var prop in eventsProto) { + Object.defineProperty(objPrototype, prop, { + enumerable: !!makeEnumerable, + value: eventsProto[prop], + configurable: true, + writable: true + }); + } + + var baseAddEventListener = objPrototype.addEventListener || + canEvent.addEventListener; + var baseRemoveEventListener = objPrototype.removeEventListener || + canEvent.removeEventListener; + + objPrototype.addEventListener = function(eventName, handler) { + var computedBinding = this._computed && this._computed[eventName]; + if (computedBinding && computedBinding.compute) { + if (!computedBinding.count) { + computedBinding.count = 1; + computedBinding.compute.addEventListener("change", computedBinding.handler); + } else { + computedBinding.count++; + } + } + + eventLifecycle.addAndSetup + + return baseAddEventListener.apply(this, arguments); + }; + + // ### unbind + // Stops listening to an event. + // If this is the last listener of a computed property, + // stop forwarding events of the computed property to this map. + objPrototype.removeEventListener = function(eventName, handler) { + var computedBinding = this._computed && this._computed[eventName]; + if (computedBinding) { + if (computedBinding.count === 1) { + computedBinding.count = 0; + computedBinding.compute.removeEventListener("change", computedBinding.handler); + } else { + computedBinding.count--; + } + + } + + return baseRemoveEventListener.apply(this, arguments); + }; + + eventLifecycle(objPrototype); + + objPrototype.on = objPrototype.bind = objPrototype.addEventListener; + objPrototype.off = objPrototype.unbind = objPrototype.removeEventListener; +}; define.defineConfigurableAndNotEnumerable = defineConfigurableAndNotEnumerable; define.make = make; define.getDefinitionOrMethod = getDefinitionOrMethod; diff --git a/list/list.js b/list/list.js index 1bb9728..db92c7a 100644 --- a/list/list.js +++ b/list/list.js @@ -1056,14 +1056,8 @@ assign(DefineList.prototype, { // Add necessary event methods to this object. -for (var prop in define.eventsProto) { - DefineList[prop] = define.eventsProto[prop]; - Object.defineProperty(DefineList.prototype, prop, { - enumerable: false, - value: define.eventsProto[prop], - writable: true - }); -} +define.mixinEvents(DefineList, true); +define.mixinEvents(DefineList.prototype); Object.defineProperty(DefineList.prototype, "length", { get: function() { diff --git a/map/map.js b/map/map.js index cf8f618..79cee87 100644 --- a/map/map.js +++ b/map/map.js @@ -56,7 +56,7 @@ var setProps = function(props, remove) { } else if( ("replace" in curVal) && isArray(newVal)) { curVal.replace(newVal); - } + } else if( ("set" in curVal) && (isPlainObject(newVal) || isArray(newVal))) { curVal.set(newVal, remove); } @@ -259,14 +259,9 @@ var DefineMap = Construct.extend("DefineMap",{ }); // Add necessary event methods to this object. -for(var prop in define.eventsProto) { - DefineMap[prop] = define.eventsProto[prop]; - Object.defineProperty(DefineMap.prototype, prop, { - enumerable:false, - value: define.eventsProto[prop], - writable: true - }); -} +define.mixinEvents(DefineMap, true); +define.mixinEvents(DefineMap.prototype); + types.DefineMap = DefineMap; types.DefaultMap = DefineMap; From de72066a15bd1adb1d7d540f080dcd47c42c3bdc Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Tue, 14 Feb 2017 08:13:36 -0500 Subject: [PATCH 4/5] Upgrade to can-event 3.1.0 Upgrading to can-event 3.1.0 makes it possible to implement the event lifecycle stuff. --- can-define.js | 2 -- package.json | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/can-define.js b/can-define.js index ea83d13..4022431 100644 --- a/can-define.js +++ b/can-define.js @@ -704,8 +704,6 @@ define.mixinEvents = function(objPrototype, makeEnumerable){ } } - eventLifecycle.addAndSetup - return baseAddEventListener.apply(this, arguments); }; diff --git a/package.json b/package.json index 32ad2d2..97e9474 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "can-cid": "^1.0.0", "can-compute": "^3.0.0", "can-construct": "^3.0.6", - "can-event": "^3.0.1", + "can-event": "^3.1.0", "can-namespace": "^1.0.0", "can-observation": "^3.0.1", "can-types": "^1.0.1", From 1f344be106e4d4104d45f891c5fcafeaf8174220 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Tue, 14 Feb 2017 11:44:27 -0500 Subject: [PATCH 5/5] Breaking test with computes This demonstrates that using computes doesn't work with this approach. The reason is that domDispatch works differently than canEvent.dispatch. --- define-test.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/define-test.js b/define-test.js index b6a9353..b295694 100644 --- a/define-test.js +++ b/define-test.js @@ -1510,6 +1510,10 @@ QUnit.test('Works with DOM elements', function(){ var el = document.createElement('div'); define(el, { foo: 'string' }); + var fooCompute = compute(function(){ + return el.foo; + }); + var events = 0; el.addEventListener('foo', function(){ events++; @@ -1519,6 +1523,13 @@ QUnit.test('Works with DOM elements', function(){ events++; }); + fooCompute.on('change', function(){ + QUnit.ok(true, "change was called"); + QUnit.start(); + }); + + QUnit.stop(); + el.foo = 'bar'; QUnit.equal(events, 1, 'An event occurred');