Merge pull request #1949 from isiahmeadows/optimize-events

Optimize events, support objects with `handleEvent`

docs/change-log.md

@@ -1,6 +1,6 @@
 # Change log
 
-- [v2.0.0](#v113)
+- [v2.0.0](#v200-wip)
 - [v1.1.4](#v113)
 - [v1.1.3](#v113)
 - [v1.1.2](#v112)
@@ -22,6 +22,7 @@
 #### News
 
 - API: Introduction of `m.redraw.sync()` ([#1592](https://github.com/MithrilJS/mithril.js/pull/1592))
+- API: Event handlers may also be objects with `handleEvent` methods ([#1939](https://github.com/MithrilJS/mithril.js/issues/1939)).
 
 #### Ospec improvements:
 
@@ -33,6 +34,10 @@
 - API: `m.route.set()` causes all mount points to be redrawn ([#1592](https://github.com/MithrilJS/mithril.js/pull/1592))
 - API: If a user sets the Content-Type header within a request's options, that value will be the entire header value rather than being appended to the default value ([#1924](https://github.com/MithrilJS/mithril.js/pull/1924))
 - API: Using style objects in hyperscript calls will now properly diff style properties from one render to another as opposed to re-writing all element style properties every render.
+- core: `addEventListener` and `removeEventListener` are always used to manage event subscriptions, preventing external interference.
+- core: Event listeners allocate less memory, swap at low cost, and are properly diffed now when rendered via `m.mount()`/`m.redraw()`.
+- core: `Object.prototype` properties can no longer interfere with event listener calls.
+- API: Event handlers, when set to literally `undefined` (or any non-function), are now correctly removed.
 
 ---
 

render/render.js

@@ -472,13 +472,11 @@ module.exports = function($window) {
 		}
 	}
 	function setAttr(vnode, key, old, value, ns) {
+		if (key === "key" || key === "is" || isLifecycleMethod(key)) return
+		if (key[0] === "o" && key[1] === "n") return updateEvent(vnode, key, value)
+		if ((old === value && !isFormAttribute(vnode, key)) && typeof value !== "object" || value === undefined) return
 		var element = vnode.dom
-		if (key === "key" || key === "is" || (old === value && !isFormAttribute(vnode, key)) && typeof value !== "object" || typeof value === "undefined" || isLifecycleMethod(key)) return
-		var nsLastIndex = key.indexOf(":")
-		if (nsLastIndex > -1 && key.substr(0, nsLastIndex) === "xlink") {
-			element.setAttributeNS("http://www.w3.org/1999/xlink", key.slice(nsLastIndex + 1), value)
-		}
-		else if (key[0] === "o" && key[1] === "n" && typeof value === "function") updateEvent(vnode, key, value)
+		if (key.slice(0, 6) === "xlink:") element.setAttributeNS("http://www.w3.org/1999/xlink", key.slice(6), value)
 		else if (key === "style") updateStyle(element, old, value)
 		else if (key in element && !isAttribute(key) && ns === undefined && !isCustomElement(vnode)) {
 			if (key === "value") {
@@ -575,24 +573,39 @@ module.exports = function($window) {
 		}
 	}
 
+	// Here's an explanation of how this works:
+	// 1. The event names are always (by design) prefixed by `on`.
+	// 2. The EventListener interface accepts either a function or an object
+	//    with a `handleEvent` method.
+	// 3. The object does not inherit from `Object.prototype`, to avoid
+	//    any potential interference with that (e.g. setters).
+	// 4. The event name is remapped to the handler before calling it.
+	// 5. In function-based event handlers, `ev.target === this`. We replicate
+	//    that below.
+	function EventDict() {}
+	EventDict.prototype = Object.create(null)
+	EventDict.prototype.handleEvent = function (ev) {
+		var handler = this["on" + ev.type]
+		if (typeof handler === "function") handler.call(ev.target, ev)
+		else if (typeof handler.handleEvent === "function") handler.handleEvent(ev)
+		if (typeof onevent === "function") onevent.call(ev.target, ev)
+	}
+
 	//event
 	function updateEvent(vnode, key, value) {
-		var element = vnode.dom
-		var callback = typeof onevent !== "function" ? value : function(e) {
-			var result = value.call(element, e)
-			onevent.call(element, e)
-			return result
-		}
-		if (key in element) element[key] = typeof value === "function" ? callback : null
-		else {
-			var eventName = key.slice(2)
-			if (vnode.events === undefined) vnode.events = {}
-			if (vnode.events[key] === callback) return
-			if (vnode.events[key] != null) element.removeEventListener(eventName, vnode.events[key], false)
-			if (typeof value === "function") {
-				vnode.events[key] = callback
-				element.addEventListener(eventName, vnode.events[key], false)
+		if (vnode.events != null) {
+			if (vnode.events[key] === value) return
+			if (value != null && (typeof value === "function" || typeof value === "object")) {
+				if (vnode.events[key] == null) vnode.dom.addEventListener(key.slice(2), vnode.events, false)
+				vnode.events[key] = value
+			} else {
+				if (vnode.events[key] != null) vnode.dom.removeEventListener(key.slice(2), vnode.events, false)
+				vnode.events[key] = undefined
 			}
+		} else if (value != null && (typeof value === "function" || typeof value === "object")) {
+			vnode.events = new EventDict()
+			vnode.dom.addEventListener(key.slice(2), vnode.events, false)
+			vnode.events[key] = value
 		}
 	}
 

render/tests/test-event.js

@@ -33,7 +33,27 @@ o.spec("event", function() {
 		o(onevent.args[0].type).equals("click")
 		o(onevent.args[0].target).equals(div.dom)
 	})
-
+
+	o("handles click EventListener object", function() {
+		var spy = o.spy()
+		var listener = {handleEvent: spy}
+		var div = {tag: "div", attrs: {onclick: listener}}
+		var e = $window.document.createEvent("MouseEvents")
+		e.initEvent("click", true, true)
+
+		render(root, [div])
+		div.dom.dispatchEvent(e)
+
+		o(spy.callCount).equals(1)
+		o(spy.this).equals(listener)
+		o(spy.args[0].type).equals("click")
+		o(spy.args[0].target).equals(div.dom)
+		o(onevent.callCount).equals(1)
+		o(onevent.this).equals(div.dom)
+		o(onevent.args[0].type).equals("click")
+		o(onevent.args[0].target).equals(div.dom)
+	})
+
 	o("removes event", function() {
 		var spy = o.spy()
 		var vnode = {tag: "a", attrs: {onclick: spy}}
@@ -45,7 +65,130 @@ o.spec("event", function() {
 		var e = $window.document.createEvent("MouseEvents")
 		e.initEvent("click", true, true)
 		vnode.dom.dispatchEvent(e)
-
+
+		o(spy.callCount).equals(0)
+	})
+
+	o("removes event when null", function() {
+		var spy = o.spy()
+		var vnode = {tag: "a", attrs: {onclick: spy}}
+		var updated = {tag: "a", attrs: {onclick: null}}
+
+		render(root, [vnode])
+		render(root, [updated])
+
+		var e = $window.document.createEvent("MouseEvents")
+		e.initEvent("click", true, true)
+		vnode.dom.dispatchEvent(e)
+
+		o(spy.callCount).equals(0)
+	})
+
+	o("removes event when undefined", function() {
+		var spy = o.spy()
+		var vnode = {tag: "a", attrs: {onclick: spy}}
+		var updated = {tag: "a", attrs: {onclick: undefined}}
+
+		render(root, [vnode])
+		render(root, [updated])
+
+		var e = $window.document.createEvent("MouseEvents")
+		e.initEvent("click", true, true)
+		vnode.dom.dispatchEvent(e)
+
+		o(spy.callCount).equals(0)
+	})
+
+	o("removes event added via addEventListener when null", function() {
+		var spy = o.spy()
+		var vnode = {tag: "a", attrs: {ontouchstart: spy}}
+		var updated = {tag: "a", attrs: {ontouchstart: null}}
+
+		render(root, [vnode])
+		render(root, [updated])
+
+		var e = $window.document.createEvent("TouchEvents")
+		e.initEvent("touchstart", true, true)
+		vnode.dom.dispatchEvent(e)
+
+		o(spy.callCount).equals(0)
+	})
+
+	o("removes event added via addEventListener", function() {
+		var spy = o.spy()
+		var vnode = {tag: "a", attrs: {ontouchstart: spy}}
+		var updated = {tag: "a", attrs: {}}
+
+		render(root, [vnode])
+		render(root, [updated])
+
+		var e = $window.document.createEvent("TouchEvents")
+		e.initEvent("touchstart", true, true)
+		vnode.dom.dispatchEvent(e)
+
+		o(spy.callCount).equals(0)
+	})
+
+	o("removes event added via addEventListener when undefined", function() {
+		var spy = o.spy()
+		var vnode = {tag: "a", attrs: {ontouchstart: spy}}
+		var updated = {tag: "a", attrs: {ontouchstart: undefined}}
+
+		render(root, [vnode])
+		render(root, [updated])
+
+		var e = $window.document.createEvent("TouchEvents")
+		e.initEvent("touchstart", true, true)
+		vnode.dom.dispatchEvent(e)
+
+		o(spy.callCount).equals(0)
+	})
+
+	o("removes EventListener object", function() {
+		var spy = o.spy()
+		var listener = {handleEvent: spy}
+		var vnode = {tag: "a", attrs: {onclick: listener}}
+		var updated = {tag: "a", attrs: {}}
+
+		render(root, [vnode])
+		render(root, [updated])
+
+		var e = $window.document.createEvent("MouseEvents")
+		e.initEvent("click", true, true)
+		vnode.dom.dispatchEvent(e)
+
+		o(spy.callCount).equals(0)
+	})
+
+	o("removes EventListener object when null", function() {
+		var spy = o.spy()
+		var listener = {handleEvent: spy}
+		var vnode = {tag: "a", attrs: {onclick: listener}}
+		var updated = {tag: "a", attrs: {onclick: null}}
+
+		render(root, [vnode])
+		render(root, [updated])
+
+		var e = $window.document.createEvent("MouseEvents")
+		e.initEvent("click", true, true)
+		vnode.dom.dispatchEvent(e)
+
+		o(spy.callCount).equals(0)
+	})
+
+	o("removes EventListener object when undefined", function() {
+		var spy = o.spy()
+		var listener = {handleEvent: spy}
+		var vnode = {tag: "a", attrs: {onclick: listener}}
+		var updated = {tag: "a", attrs: {onclick: undefined}}
+
+		render(root, [vnode])
+		render(root, [updated])
+
+		var e = $window.document.createEvent("MouseEvents")
+		e.initEvent("click", true, true)
+		vnode.dom.dispatchEvent(e)
+
 		o(spy.callCount).equals(0)
 	})
 
@@ -72,6 +215,30 @@ o.spec("event", function() {
 		o(div.dom.attributes["id"].value).equals("b")
 	})
 
+	o("fires click EventListener object only once after redraw", function() {
+		var spy = o.spy()
+		var listener = {handleEvent: spy}
+		var div = {tag: "div", attrs: {id: "a", onclick: listener}}
+		var updated = {tag: "div", attrs: {id: "b", onclick: listener}}
+		var e = $window.document.createEvent("MouseEvents")
+		e.initEvent("click", true, true)
+
+		render(root, [div])
+		render(root, [updated])
+		div.dom.dispatchEvent(e)
+
+		o(spy.callCount).equals(1)
+		o(spy.this).equals(listener)
+		o(spy.args[0].type).equals("click")
+		o(spy.args[0].target).equals(div.dom)
+		o(onevent.callCount).equals(1)
+		o(onevent.this).equals(div.dom)
+		o(onevent.args[0].type).equals("click")
+		o(onevent.args[0].target).equals(div.dom)
+		o(div.dom).equals(updated.dom)
+		o(div.dom.attributes["id"].value).equals("b")
+	})
+
 	o("handles ontransitionend", function() {
 		var spy = o.spy()
 		var div = {tag: "div", attrs: {ontransitionend: spy}}
@@ -90,4 +257,24 @@ o.spec("event", function() {
 		o(onevent.args[0].type).equals("transitionend")
 		o(onevent.args[0].target).equals(div.dom)
 	})
+
+	o("handles transitionend EventListener object", function() {
+		var spy = o.spy()
+		var listener = {handleEvent: spy}
+		var div = {tag: "div", attrs: {ontransitionend: listener}}
+		var e = $window.document.createEvent("HTMLEvents")
+		e.initEvent("transitionend", true, true)
+
+		render(root, [div])
+		div.dom.dispatchEvent(e)
+
+		o(spy.callCount).equals(1)
+		o(spy.this).equals(listener)
+		o(spy.args[0].type).equals("transitionend")
+		o(spy.args[0].target).equals(div.dom)
+		o(onevent.callCount).equals(1)
+		o(onevent.this).equals(div.dom)
+		o(onevent.args[0].type).equals("transitionend")
+		o(onevent.args[0].target).equals(div.dom)
+	})
 })

test-utils/domMock.js

@@ -278,7 +278,9 @@ module.exports = function(options) {
 						e.target = this
 						if (events[e.type] != null) {
 							for (var i = 0; i < events[e.type].length; i++) {
-								events[e.type][i].call(this, e)
+								var handler = events[e.type][i]
+								if (typeof handler === "function") handler.call(this, e)
+								else handler.handleEvent(e)
 							}
 						}
 						e.preventDefault = function() {