New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize events, support objects with `handleEvent` #1949

Merged
merged 6 commits into from Sep 18, 2017
View
@@ -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.
---
View
@@ -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
}
}
View
@@ -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() {

This comment has been minimized.

@spacejack

spacejack Aug 25, 2017

Contributor

Some of these tests may be redundant if everything is added with addEventListener unless you want to test MouseEvents and TouchEvents.

This comment has been minimized.

@isiahmeadows

isiahmeadows Aug 26, 2017

Collaborator

I'd rather stay safe, in case someone down the road decides it's a good idea to start using the raw properties again. That way, we're less likely to see regressions.

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)
})
})
View
@@ -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() {
ProTip! Use n and p to navigate between commits in a pull request.