From 22a4f81028c732d3eecf9bd22c49e95d4132c748 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 23 Nov 2017 21:38:38 -0500 Subject: [PATCH] leaner event handling (fixes #945) --- .../dom/visitors/Element/Element.ts | 34 ++++++---- .../dom/visitors/Element/addBindings.ts | 68 +++++++++++++------ src/shared/dom.js | 7 ++ .../expected-bundle.js | 22 +++--- .../input-without-blowback-guard/expected.js | 15 ++-- .../samples/media-bindings/expected-bundle.js | 35 +++++----- test/js/samples/media-bindings/expected.js | 28 ++++---- 7 files changed, 122 insertions(+), 87 deletions(-) diff --git a/src/generators/dom/visitors/Element/Element.ts b/src/generators/dom/visitors/Element/Element.ts index 8de38c766d..712efaf0d1 100644 --- a/src/generators/dom/visitors/Element/Element.ts +++ b/src/generators/dom/visitors/Element/Element.ts @@ -183,8 +183,9 @@ export default function visitElement( // get a name for the event handler that is globally unique // if hoisted, locally unique otherwise + const sanitized = attribute.name.replace(/[^a-zA-Z0-9_$]/g, '_'); const handlerName = (shouldHoist ? generator : block).getUniqueName( - `${attribute.name.replace(/[^a-zA-Z0-9_$]/g, '_')}_handler` + `${sanitized}_handler` ); // create the handler body @@ -201,7 +202,7 @@ export default function visitElement( block.addVariable(handlerName); block.builders.hydrate.addBlock(deindent` - ${handlerName} = %events-${attribute.name}.call(#component, ${name}, function(event) { + ${handlerName} = %events-${sanitized}.call(#component, ${name}, function(event) { ${handlerBody} }); `); @@ -210,24 +211,29 @@ export default function visitElement( ${handlerName}.teardown(); `); } else { - const handler = deindent` - function ${handlerName}(event) { - ${handlerBody} - } - `; + const remove_handler = block.getUniqueName(`remove_${name}_${sanitized}_handler`); + block.addVariable(remove_handler); if (shouldHoist) { - generator.blocks.push(handler); + generator.blocks.push(deindent` + function ${handlerName}(event) { + ${handlerBody} + } + `); + + block.builders.hydrate.addLine( + `${remove_handler} = @listen(${name}, "${attribute.name}", ${handlerName});` + ); } else { - block.builders.init.addBlock(handler); + block.builders.hydrate.addBlock(deindent` + ${remove_handler} = @listen(${name}, "${attribute.name}", function (event) { + ${handlerBody} + }); + `); } - block.builders.hydrate.addLine( - `@addListener(${name}, "${attribute.name}", ${handlerName});` - ); - block.builders.destroy.addLine( - `@removeListener(${name}, "${attribute.name}", ${handlerName});` + `${remove_handler}();` ); } }); diff --git a/src/generators/dom/visitors/Element/addBindings.ts b/src/generators/dom/visitors/Element/addBindings.ts index a2433abf93..67c4c57cf2 100644 --- a/src/generators/dom/visitors/Element/addBindings.ts +++ b/src/generators/dom/visitors/Element/addBindings.ts @@ -213,37 +213,63 @@ export default function addBindings( block.addVariable(animation_frame); } - block.builders.init.addBlock(deindent` - function ${handler}() { - ${ - animation_frame && deindent` - cancelAnimationFrame(${animation_frame}); - if (!${node.var}.paused) ${animation_frame} = requestAnimationFrame(${handler});` - } - ${usesContext && `var context = ${node.var}._svelte;`} - ${usesState && `var state = #component.get();`} - ${needsLock && `${lock} = true;`} - ${mutations.length > 0 && mutations} - #component.set({ ${Array.from(props).join(', ')} }); - ${needsLock && `${lock} = false;`} + const handlerBody = deindent` + ${ + animation_frame && deindent` + cancelAnimationFrame(${animation_frame}); + if (!${node.var}.paused) ${animation_frame} = requestAnimationFrame(${handler});` } - `); + ${usesContext && `var context = ${node.var}._svelte;`} + ${usesState && `var state = #component.get();`} + ${needsLock && `${lock} = true;`} + ${mutations.length > 0 && mutations} + #component.set({ ${Array.from(props).join(', ')} }); + ${needsLock && `${lock} = false;`} + `; + + const shouldImmediatelyUpdateState = node.name === 'select' || group.bindings.find(binding => binding.name === 'indeterminate' || readOnlyMediaAttributes.has(binding.name)); + + if (group.events.length > 1 || shouldImmediatelyUpdateState) { + block.builders.init.addBlock(deindent` + function ${handler}() { + ${handlerBody} + } + `); - group.events.forEach(name => { - block.builders.hydrate.addLine( - `@addListener(${node.var}, "${name}", ${handler});` - ); + group.events.forEach(name => { + const remove_handler = block.getUniqueName(`remove_${node.var}_${name}_handler`); + block.addVariable(remove_handler); + + block.builders.hydrate.addLine( + `${remove_handler} = @listen(${node.var}, "${name}", ${handler});` + ); + + block.builders.destroy.addLine( + `${remove_handler}();` + ); + }); + } else { + const name = group.events[0]; + + const remove_handler = block.getUniqueName(`remove_${node.var}_${name}_handler`); + block.addVariable(remove_handler); + + block.builders.hydrate.addBlock(deindent` + ${remove_handler} = @listen(${node.var}, "${name}", function() { + ${handlerBody} + }); + `); block.builders.destroy.addLine( - `@removeListener(${node.var}, "${name}", ${handler});` + `${remove_handler}();` ); - }); + } const allInitialStateIsDefined = group.bindings .map(binding => `'${binding.object}' in state`) .join(' && '); - if (node.name === 'select' || group.bindings.find(binding => binding.name === 'indeterminate' || readOnlyMediaAttributes.has(binding.name))) { + if (shouldImmediatelyUpdateState) { generator.hasComplexBindings = true; block.builders.hydrate.addLine( diff --git a/src/shared/dom.js b/src/shared/dom.js index 9003faee3e..93348d529d 100644 --- a/src/shared/dom.js +++ b/src/shared/dom.js @@ -73,6 +73,13 @@ export function createComment() { return document.createComment(''); } +export function listen(node, event, handler) { + node.addEventListener(event, handler, false); + return function() { + node.removeEventListener(event, handler, false); + }; +} + export function addListener(node, event, handler) { node.addEventListener(event, handler, false); } diff --git a/test/js/samples/input-without-blowback-guard/expected-bundle.js b/test/js/samples/input-without-blowback-guard/expected-bundle.js index 1918f9f6d1..d98e3c65bc 100644 --- a/test/js/samples/input-without-blowback-guard/expected-bundle.js +++ b/test/js/samples/input-without-blowback-guard/expected-bundle.js @@ -25,12 +25,11 @@ function createElement(name) { return document.createElement(name); } -function addListener(node, event, handler) { +function listen(node, event, handler) { node.addEventListener(event, handler, false); -} - -function removeListener(node, event, handler) { - node.removeEventListener(event, handler, false); + return function() { + node.removeEventListener(event, handler, false); + }; } function blankObject() { @@ -189,11 +188,7 @@ var proto = { /* generated by Svelte vX.Y.Z */ function create_main_fragment(state, component) { - var input; - - function input_change_handler() { - component.set({ foo: input.checked }); - } + var input, remove_input_change_handler; return { c: function create() { @@ -202,7 +197,10 @@ function create_main_fragment(state, component) { }, h: function hydrate() { - addListener(input, "change", input_change_handler); + remove_input_change_handler = listen(input, "change", function() { + component.set({ foo: input.checked }); + }); + input.type = "checkbox"; }, @@ -221,7 +219,7 @@ function create_main_fragment(state, component) { }, d: function destroy$$1() { - removeListener(input, "change", input_change_handler); + remove_input_change_handler(); } }; } diff --git a/test/js/samples/input-without-blowback-guard/expected.js b/test/js/samples/input-without-blowback-guard/expected.js index 03f27ab6d9..89a876bd85 100644 --- a/test/js/samples/input-without-blowback-guard/expected.js +++ b/test/js/samples/input-without-blowback-guard/expected.js @@ -1,12 +1,8 @@ /* generated by Svelte vX.Y.Z */ -import { addListener, assign, createElement, detachNode, init, insertNode, proto, removeListener } from "svelte/shared.js"; +import { assign, createElement, detachNode, init, insertNode, listen, proto } from "svelte/shared.js"; function create_main_fragment(state, component) { - var input; - - function input_change_handler() { - component.set({ foo: input.checked }); - } + var input, remove_input_change_handler; return { c: function create() { @@ -15,7 +11,10 @@ function create_main_fragment(state, component) { }, h: function hydrate() { - addListener(input, "change", input_change_handler); + remove_input_change_handler = listen(input, "change", function() { + component.set({ foo: input.checked }); + }); + input.type = "checkbox"; }, @@ -34,7 +33,7 @@ function create_main_fragment(state, component) { }, d: function destroy() { - removeListener(input, "change", input_change_handler); + remove_input_change_handler(); } }; } diff --git a/test/js/samples/media-bindings/expected-bundle.js b/test/js/samples/media-bindings/expected-bundle.js index 80545b9402..33ea46024c 100644 --- a/test/js/samples/media-bindings/expected-bundle.js +++ b/test/js/samples/media-bindings/expected-bundle.js @@ -25,12 +25,11 @@ function createElement(name) { return document.createElement(name); } -function addListener(node, event, handler) { +function listen(node, event, handler) { node.addEventListener(event, handler, false); -} - -function removeListener(node, event, handler) { - node.removeEventListener(event, handler, false); + return function() { + node.removeEventListener(event, handler, false); + }; } function timeRangesToArray(ranges) { @@ -197,7 +196,7 @@ var proto = { /* generated by Svelte vX.Y.Z */ function create_main_fragment(state, component) { - var audio, audio_is_paused = true, audio_updating = false, audio_animationframe; + var audio, audio_is_paused = true, audio_updating = false, audio_animationframe, remove_audio_timeupdate_handler, remove_audio_durationchange_handler, remove_audio_play_handler, remove_audio_pause_handler, remove_audio_progress_handler, remove_audio_loadedmetadata_handler; function audio_timeupdate_handler() { cancelAnimationFrame(audio_animationframe); @@ -232,15 +231,15 @@ function create_main_fragment(state, component) { }, h: function hydrate() { - addListener(audio, "timeupdate", audio_timeupdate_handler); + remove_audio_timeupdate_handler = listen(audio, "timeupdate", audio_timeupdate_handler); if (!('played' in state && 'currentTime' in state)) component._root._beforecreate.push(audio_timeupdate_handler); - addListener(audio, "durationchange", audio_durationchange_handler); + remove_audio_durationchange_handler = listen(audio, "durationchange", audio_durationchange_handler); if (!('duration' in state)) component._root._beforecreate.push(audio_durationchange_handler); - addListener(audio, "play", audio_play_pause_handler); - addListener(audio, "pause", audio_play_pause_handler); - addListener(audio, "progress", audio_progress_handler); + remove_audio_play_handler = listen(audio, "play", audio_play_pause_handler); + remove_audio_pause_handler = listen(audio, "pause", audio_play_pause_handler); + remove_audio_progress_handler = listen(audio, "progress", audio_progress_handler); if (!('buffered' in state)) component._root._beforecreate.push(audio_progress_handler); - addListener(audio, "loadedmetadata", audio_loadedmetadata_handler); + remove_audio_loadedmetadata_handler = listen(audio, "loadedmetadata", audio_loadedmetadata_handler); if (!('buffered' in state && 'seekable' in state)) component._root._beforecreate.push(audio_loadedmetadata_handler); }, @@ -258,12 +257,12 @@ function create_main_fragment(state, component) { }, d: function destroy$$1() { - removeListener(audio, "timeupdate", audio_timeupdate_handler); - removeListener(audio, "durationchange", audio_durationchange_handler); - removeListener(audio, "play", audio_play_pause_handler); - removeListener(audio, "pause", audio_play_pause_handler); - removeListener(audio, "progress", audio_progress_handler); - removeListener(audio, "loadedmetadata", audio_loadedmetadata_handler); + remove_audio_timeupdate_handler(); + remove_audio_durationchange_handler(); + remove_audio_play_handler(); + remove_audio_pause_handler(); + remove_audio_progress_handler(); + remove_audio_loadedmetadata_handler(); } }; } diff --git a/test/js/samples/media-bindings/expected.js b/test/js/samples/media-bindings/expected.js index c14eb2aa83..5f7c386084 100644 --- a/test/js/samples/media-bindings/expected.js +++ b/test/js/samples/media-bindings/expected.js @@ -1,8 +1,8 @@ /* generated by Svelte vX.Y.Z */ -import { addListener, assign, callAll, createElement, detachNode, init, insertNode, proto, removeListener, timeRangesToArray } from "svelte/shared.js"; +import { assign, callAll, createElement, detachNode, init, insertNode, listen, proto, timeRangesToArray } from "svelte/shared.js"; function create_main_fragment(state, component) { - var audio, audio_is_paused = true, audio_updating = false, audio_animationframe; + var audio, audio_is_paused = true, audio_updating = false, audio_animationframe, remove_audio_timeupdate_handler, remove_audio_durationchange_handler, remove_audio_play_handler, remove_audio_pause_handler, remove_audio_progress_handler, remove_audio_loadedmetadata_handler; function audio_timeupdate_handler() { cancelAnimationFrame(audio_animationframe); @@ -37,15 +37,15 @@ function create_main_fragment(state, component) { }, h: function hydrate() { - addListener(audio, "timeupdate", audio_timeupdate_handler); + remove_audio_timeupdate_handler = listen(audio, "timeupdate", audio_timeupdate_handler); if (!('played' in state && 'currentTime' in state)) component._root._beforecreate.push(audio_timeupdate_handler); - addListener(audio, "durationchange", audio_durationchange_handler); + remove_audio_durationchange_handler = listen(audio, "durationchange", audio_durationchange_handler); if (!('duration' in state)) component._root._beforecreate.push(audio_durationchange_handler); - addListener(audio, "play", audio_play_pause_handler); - addListener(audio, "pause", audio_play_pause_handler); - addListener(audio, "progress", audio_progress_handler); + remove_audio_play_handler = listen(audio, "play", audio_play_pause_handler); + remove_audio_pause_handler = listen(audio, "pause", audio_play_pause_handler); + remove_audio_progress_handler = listen(audio, "progress", audio_progress_handler); if (!('buffered' in state)) component._root._beforecreate.push(audio_progress_handler); - addListener(audio, "loadedmetadata", audio_loadedmetadata_handler); + remove_audio_loadedmetadata_handler = listen(audio, "loadedmetadata", audio_loadedmetadata_handler); if (!('buffered' in state && 'seekable' in state)) component._root._beforecreate.push(audio_loadedmetadata_handler); }, @@ -63,12 +63,12 @@ function create_main_fragment(state, component) { }, d: function destroy() { - removeListener(audio, "timeupdate", audio_timeupdate_handler); - removeListener(audio, "durationchange", audio_durationchange_handler); - removeListener(audio, "play", audio_play_pause_handler); - removeListener(audio, "pause", audio_play_pause_handler); - removeListener(audio, "progress", audio_progress_handler); - removeListener(audio, "loadedmetadata", audio_loadedmetadata_handler); + remove_audio_timeupdate_handler(); + remove_audio_durationchange_handler(); + remove_audio_play_handler(); + remove_audio_pause_handler(); + remove_audio_progress_handler(); + remove_audio_loadedmetadata_handler(); } }; }