From 8ca33c916b3b38991aba148ace735dcb825bd7e3 Mon Sep 17 00:00:00 2001 From: "K." Date: Fri, 29 Jul 2016 11:58:43 +0500 Subject: [PATCH 1/5] New implementation of eventlisteners module --- modules/eventlisteners.js | 115 ++++++++++++++++++++++---------------- test/eventlisteners.js | 59 ++++++++++++++++++- 2 files changed, 124 insertions(+), 50 deletions(-) diff --git a/modules/eventlisteners.js b/modules/eventlisteners.js index 4b03b28..203d846 100644 --- a/modules/eventlisteners.js +++ b/modules/eventlisteners.js @@ -1,62 +1,79 @@ -var is = require('../is'); - -function arrInvoker(arr) { - return function() { - if (!arr.length) return; - // Special case when length is two, for performance - arr.length === 2 ? arr[0](arr[1]) : arr[0].apply(undefined, arr.slice(1)); - }; +function invokeHandler(handler, vnode, event) { + if (typeof handler === "function") { + // call function handler + handler.call(vnode, event); + } else if (typeof handler === "object") { + // call handler with arguments + if (typeof handler[0] === "function") { + // special case for single argument for performance + handler.length === 2 ? + handler[0].call(vnode, handler[1], event) : + handler[0].apply(vnode, handler.slice(1).concat(event)); + } else { + // call multiple handlers + for (var i = 0; i < handler.length; i++) { + invokeHandler(handler[i]); + } + } + } +} + +function handleEvent(event, vnode) { + var name = event.type, + on = vnode.data.on; + + // call event handler(s) if exists + if (on && on[name]) { + invokeHandler(on[name], vnode, event); + } } -function fnInvoker(o) { - return function(ev) { - if (o.fn === null) return; - o.fn(ev); - }; +function createListener() { + return function handler(event) { + handleEvent(event, handler.vnode); + } } function updateEventListeners(oldVnode, vnode) { - var name, cur, old, elm = vnode.elm, - oldOn = oldVnode.data.on, on = vnode.data.on; - - if (!on && !oldOn) return; - on = on || {}; - oldOn = oldOn || {}; - - for (name in on) { - cur = on[name]; - old = oldOn[name]; - if (old === undefined) { - if (is.array(cur)) { - elm.addEventListener(name, arrInvoker(cur)); - } else { - cur = {fn: cur}; - on[name] = cur; - elm.addEventListener(name, fnInvoker(cur)); + var oldOn = oldVnode.data.on, + oldListener = oldVnode.listener, + oldElm = oldVnode.elm, + on = vnode && vnode.data.on, + elm = vnode && vnode.elm; + + // improvement for immutable handlers + if (oldElm === elm && oldOn === on) { + return; + } + + // remove existing listeners which no longer used + if (oldOn && oldListener) { + for (var _name in oldOn) { + // remove listener if element was changed or existing listener(s) removed + if (elm !== oldElm || !on || !on[_name]) { + oldElm.removeEventListener(_name, oldListener, false); } - } else if (is.array(old)) { - // Deliberately modify old array since it's captured in closure created with `arrInvoker` - old.length = cur.length; - for (var i = 0; i < old.length; ++i) old[i] = cur[i]; - on[name] = old; - } else { - old.fn = cur; - on[name] = old; } } - if (oldOn) { - for (name in oldOn) { - if (on[name] === undefined) { - var old = oldOn[name]; - if (is.array(old)) { - old.length = 0; - } - else { - old.fn = null; - } + + // add new listeners which has not already attached + if (on) { + // reuse existing listener or create new + var listener = vnode.listener = oldVnode.listener || createListener(); + // update vnode for listener + listener.vnode = vnode; + + for (var name in on) { + // add listener if element was changed or new listener(s) added + if (elm !== oldElm || !oldOn || !oldOn[name]) { + elm.addEventListener(name, listener, false); } } } } -module.exports = {create: updateEventListeners, update: updateEventListeners}; +module.exports = { + create: updateEventListeners, + update: updateEventListeners, + destroy: updateEventListeners +}; diff --git a/test/eventlisteners.js b/test/eventlisteners.js index 1e66b45..6ec2879 100644 --- a/test/eventlisteners.js +++ b/test/eventlisteners.js @@ -69,7 +69,7 @@ describe('event listeners', function() { }); it('handles changed several values in array', function() { var result = []; - function clicked() { result.push([].slice.call(arguments)); } + function clicked() { result.push([].slice.call(arguments, 0, arguments.length-1)); } var vnode1 = h('div', {on: {click: [clicked, 1, 2, 3]}}, [ h('a', 'Click my parent'), ]); @@ -87,4 +87,61 @@ describe('event listeners', function() { elm.click(); assert.deepEqual(result, [[1, 2, 3], [1, 2], [2, 3]]); }); + it('detach attached click event handler to element', function() { + var result = []; + function clicked(ev) { result.push(ev); } + var vnode1 = h('div', {on: {click: clicked}}, [ + h('a', 'Click my parent'), + ]); + elm = patch(vnode0, vnode1).elm; + elm.click(); + assert.equal(1, result.length); + var vnode2 = h('div', {on: {}}, [ + h('a', 'Click my parent'), + ]); + elm = patch(vnode1, vnode2).elm; + elm.click(); + assert.equal(1, result.length); + }); + it('multiple event handlers for same event on same element', function() { + var result = []; + function clicked(ev) { result.push(ev); } + var vnode1 = h('div', {on: {click: [[clicked], [clicked], [clicked]]}}, [ + h('a', 'Click my parent'), + ]); + elm = patch(vnode0, vnode1).elm; + elm.click(); + assert.equal(3, result.length); + var vnode2 = h('div', {on: {click: [[clicked], [clicked]]}}, [ + h('a', 'Click my parent'), + ]); + elm = patch(vnode1, vnode2).elm; + elm.click(); + assert.equal(5, result.length); + }); + it('access to virtual node in event handler', function() { + var result = []; + function clicked(ev) { result.push(this); } + var vnode1 = h('div', {on: {click: clicked }}, [ + h('a', 'Click my parent'), + ]); + elm = patch(vnode0, vnode1).elm; + elm.click(); + assert.equal(1, result.length); + assert.equal(vnode1, result[0]); + }); + it('shared handlers in parent and child nodes', function() { + var result = []; + var sharedHandlers = { + click: function(ev) { result.push(ev); } + }; + var vnode1 = h('div', {on: sharedHandlers}, [ + h('a', {on: sharedHandlers}, 'Click my parent'), + ]); + elm = patch(vnode0, vnode1).elm; + elm.click(); + assert.equal(1, result.length); + elm.firstChild.click(); + assert.equal(3, result.length); + }); }); From 831bd0e1eede28c8f19990b7c402e35fb546b727 Mon Sep 17 00:00:00 2001 From: Kayo Phoenix Date: Wed, 3 Aug 2016 02:05:48 +0500 Subject: [PATCH 2/5] Optimizations for eventlistener.js --- modules/eventlisteners.js | 40 ++++++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/modules/eventlisteners.js b/modules/eventlisteners.js index 203d846..5f79c0c 100644 --- a/modules/eventlisteners.js +++ b/modules/eventlisteners.js @@ -39,19 +39,29 @@ function updateEventListeners(oldVnode, vnode) { oldListener = oldVnode.listener, oldElm = oldVnode.elm, on = vnode && vnode.data.on, - elm = vnode && vnode.elm; + elm = vnode && vnode.elm, + elmChanged = oldElm !== elm, + name; - // improvement for immutable handlers - if (oldElm === elm && oldOn === on) { + // optimization for immutable handlers + if (!elmChanged && oldOn === on) { return; } // remove existing listeners which no longer used if (oldOn && oldListener) { - for (var _name in oldOn) { - // remove listener if element was changed or existing listener(s) removed - if (elm !== oldElm || !on || !on[_name]) { - oldElm.removeEventListener(_name, oldListener, false); + // if element changed or deleted we remove all existing listeners unconditionally + if (elmChanged || !on) { + for (name in oldOn) { + // remove listener if element was changed or existing listeners removed + oldElm.removeEventListener(name, oldListener, false); + } + } else { + for (name in oldOn) { + // remove listener if existing listener removed + if (!on[name]) { + oldElm.removeEventListener(name, oldListener, false); + } } } } @@ -62,12 +72,20 @@ function updateEventListeners(oldVnode, vnode) { var listener = vnode.listener = oldVnode.listener || createListener(); // update vnode for listener listener.vnode = vnode; - - for (var name in on) { - // add listener if element was changed or new listener(s) added - if (elm !== oldElm || !oldOn || !oldOn[name]) { + + // if element changed or added we add all needed listeners unconditionally + if (elmChanged || !oldOn) { + for (name in on) { + // add listener if element was changed or new listeners added elm.addEventListener(name, listener, false); } + } else { + for (name in on) { + // add listener if new listener added + if (!oldOn[name]) { + elm.addEventListener(name, listener, false); + } + } } } } From 174941c6a49fff91e4e3949fa39423c7cb28939c Mon Sep 17 00:00:00 2001 From: Kayo Phoenix Date: Wed, 3 Aug 2016 02:33:43 +0500 Subject: [PATCH 3/5] Added vnode as last argument to event handler --- modules/eventlisteners.js | 6 +++--- test/eventlisteners.js | 31 ++++++++++++++++++++++++++++--- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/modules/eventlisteners.js b/modules/eventlisteners.js index 5f79c0c..53ffffb 100644 --- a/modules/eventlisteners.js +++ b/modules/eventlisteners.js @@ -1,14 +1,14 @@ function invokeHandler(handler, vnode, event) { if (typeof handler === "function") { // call function handler - handler.call(vnode, event); + handler.call(vnode, event, vnode); } else if (typeof handler === "object") { // call handler with arguments if (typeof handler[0] === "function") { // special case for single argument for performance handler.length === 2 ? - handler[0].call(vnode, handler[1], event) : - handler[0].apply(vnode, handler.slice(1).concat(event)); + handler[0].call(vnode, handler[1], event, vnode) : + handler[0].apply(vnode, handler.slice(1).concat(event, vnode)); } else { // call multiple handlers for (var i = 0; i < handler.length; i++) { diff --git a/test/eventlisteners.js b/test/eventlisteners.js index 6ec2879..7bc5eeb 100644 --- a/test/eventlisteners.js +++ b/test/eventlisteners.js @@ -69,7 +69,7 @@ describe('event listeners', function() { }); it('handles changed several values in array', function() { var result = []; - function clicked() { result.push([].slice.call(arguments, 0, arguments.length-1)); } + function clicked() { result.push([].slice.call(arguments, 0, arguments.length-2)); } var vnode1 = h('div', {on: {click: [clicked, 1, 2, 3]}}, [ h('a', 'Click my parent'), ]); @@ -121,14 +121,39 @@ describe('event listeners', function() { }); it('access to virtual node in event handler', function() { var result = []; - function clicked(ev) { result.push(this); } + function clicked(ev, vnode) { result.push(this); result.push(vnode); } var vnode1 = h('div', {on: {click: clicked }}, [ h('a', 'Click my parent'), ]); elm = patch(vnode0, vnode1).elm; elm.click(); - assert.equal(1, result.length); + assert.equal(2, result.length); + assert.equal(vnode1, result[0]); + assert.equal(vnode1, result[1]); + }), + it('access to virtual node in event handler with argument', function() { + var result = []; + function clicked(arg, ev, vnode) { result.push(this); result.push(vnode); } + var vnode1 = h('div', {on: {click: [clicked, 1] }}, [ + h('a', 'Click my parent'), + ]); + elm = patch(vnode0, vnode1).elm; + elm.click(); + assert.equal(2, result.length); + assert.equal(vnode1, result[0]); + assert.equal(vnode1, result[1]); + }), + it('access to virtual node in event handler with arguments', function() { + var result = []; + function clicked(arg1, arg2, ev, vnode) { result.push(this); result.push(vnode); } + var vnode1 = h('div', {on: {click: [clicked, 1, "2"] }}, [ + h('a', 'Click my parent'), + ]); + elm = patch(vnode0, vnode1).elm; + elm.click(); + assert.equal(2, result.length); assert.equal(vnode1, result[0]); + assert.equal(vnode1, result[1]); }); it('shared handlers in parent and child nodes', function() { var result = []; From cf4793c10a15762196cccf87570ba44a185744a8 Mon Sep 17 00:00:00 2001 From: "K." Date: Thu, 25 Aug 2016 11:32:16 +0500 Subject: [PATCH 4/5] Removing 'elm changed' case. --- modules/eventlisteners.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/modules/eventlisteners.js b/modules/eventlisteners.js index 53ffffb..f086a43 100644 --- a/modules/eventlisteners.js +++ b/modules/eventlisteners.js @@ -40,18 +40,17 @@ function updateEventListeners(oldVnode, vnode) { oldElm = oldVnode.elm, on = vnode && vnode.data.on, elm = vnode && vnode.elm, - elmChanged = oldElm !== elm, name; - // optimization for immutable handlers - if (!elmChanged && oldOn === on) { + // optimization for reused immutable handlers + if (oldOn === on) { return; } // remove existing listeners which no longer used if (oldOn && oldListener) { // if element changed or deleted we remove all existing listeners unconditionally - if (elmChanged || !on) { + if (!on) { for (name in oldOn) { // remove listener if element was changed or existing listeners removed oldElm.removeEventListener(name, oldListener, false); @@ -74,7 +73,7 @@ function updateEventListeners(oldVnode, vnode) { listener.vnode = vnode; // if element changed or added we add all needed listeners unconditionally - if (elmChanged || !oldOn) { + if (!oldOn) { for (name in on) { // add listener if element was changed or new listeners added elm.addEventListener(name, listener, false); From 41394317942e5151a3218511b772cbc955cc02e9 Mon Sep 17 00:00:00 2001 From: "K." Date: Thu, 25 Aug 2016 11:34:11 +0500 Subject: [PATCH 5/5] Optimization invoking handler with multiple arguments. --- modules/eventlisteners.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/modules/eventlisteners.js b/modules/eventlisteners.js index f086a43..34ad98d 100644 --- a/modules/eventlisteners.js +++ b/modules/eventlisteners.js @@ -6,9 +6,14 @@ function invokeHandler(handler, vnode, event) { // call handler with arguments if (typeof handler[0] === "function") { // special case for single argument for performance - handler.length === 2 ? - handler[0].call(vnode, handler[1], event, vnode) : - handler[0].apply(vnode, handler.slice(1).concat(event, vnode)); + if (handler.length === 2) { + handler[0].call(vnode, handler[1], event, vnode); + } else { + var args = handler.slice(1); + args.push(event); + args.push(vnode); + handler[0].apply(vnode, args); + } } else { // call multiple handlers for (var i = 0; i < handler.length; i++) {