From abe88f3b3adf35147146d8a4a36b440c04b8bb2a Mon Sep 17 00:00:00 2001 From: Colin Casey Date: Sat, 11 May 2019 11:29:11 -0300 Subject: [PATCH] FIX #2446: apply bindings and event handlers in order --- .../render_dom/wrappers/Element/index.ts | 74 ++++++++++++++----- test/js/samples/media-bindings/expected.js | 50 ++++++------- test/js/samples/video-bindings/expected.js | 26 +++---- .../apply-directives-in-order/_config.js | 37 ++++++++++ .../apply-directives-in-order/main.svelte | 10 +++ 5 files changed, 139 insertions(+), 58 deletions(-) create mode 100644 test/runtime/samples/apply-directives-in-order/_config.js create mode 100644 test/runtime/samples/apply-directives-in-order/main.svelte diff --git a/src/compiler/compile/render_dom/wrappers/Element/index.ts b/src/compiler/compile/render_dom/wrappers/Element/index.ts index cd660f202c..0478702c7c 100644 --- a/src/compiler/compile/render_dom/wrappers/Element/index.ts +++ b/src/compiler/compile/render_dom/wrappers/Element/index.ts @@ -377,8 +377,7 @@ export default class ElementWrapper extends Wrapper { } this.add_attributes(block); - this.add_bindings(block); - this.add_event_handlers(block); + this.add_directives_in_order(block); this.add_transitions(block); this.add_animation(block); this.add_actions(block); @@ -436,29 +435,62 @@ export default class ElementWrapper extends Wrapper { return x`@claim_element(${nodes}, "${name}", { ${attributes} }, ${svg})`; } - add_bindings(block: Block) { + add_directives_in_order (block: Block) { + interface BindingGroup { + events: string[]; + bindings: Binding[]; + } + + const bindingGroups = events + .map(event => ({ + events: event.event_names, + bindings: this.bindings + .filter(binding => binding.node.name !== 'this') + .filter(binding => event.filter(this.node, binding.node.name)) + })) + .filter(group => group.bindings.length); + + const this_binding = this.bindings.find(b => b.node.name === 'this'); + + function getOrder (item: EventHandler | BindingGroup | Binding) { + if (item instanceof EventHandler) { + return item.node.start; + } else if (item instanceof Binding) { + return item.node.start; + } else { + return item.bindings[0].node.start; + } + } + + const ordered: Array = [].concat(bindingGroups, this.event_handlers, this_binding).filter(Boolean); + + ordered.sort((a, b) => getOrder(a) - getOrder(b)); + + ordered.forEach(bindingGroupOrEventHandler => { + if (bindingGroupOrEventHandler instanceof EventHandler) { + add_event_handlers(block, this.var, [bindingGroupOrEventHandler]); + } else if (bindingGroupOrEventHandler instanceof Binding) { + this.add_this_binding(block, bindingGroupOrEventHandler); + } else { + this.add_bindings(block, bindingGroupOrEventHandler); + } + }); + } + + add_bindings(block: Block, bindingGroup) { const { renderer } = this; - if (this.bindings.length === 0) return; + if (bindingGroup.bindings.length === 0) return; renderer.component.has_reactive_assignments = true; - const lock = this.bindings.some(binding => binding.needs_lock) ? + const lock = bindingGroup.bindings.some(binding => binding.needs_lock) ? block.get_unique_name(`${this.var.name}_updating`) : null; if (lock) block.add_variable(lock, x`false`); - const groups = events - .map(event => ({ - events: event.event_names, - bindings: this.bindings - .filter(binding => binding.node.name !== 'this') - .filter(binding => event.filter(this.node, binding.node.name)) - })) - .filter(group => group.bindings.length); - - groups.forEach(group => { + [bindingGroup].forEach(group => { const handler = renderer.component.get_unique_name(`${this.var.name}_${group.events.join('_')}_handler`); renderer.add_to_context(handler.name); @@ -586,13 +618,15 @@ export default class ElementWrapper extends Wrapper { if (lock) { block.chunks.update.push(b`${lock} = false;`); } + } - const this_binding = this.bindings.find(b => b.node.name === 'this'); - if (this_binding) { - const binding_callback = bind_this(renderer.component, block, this_binding.node, this.var); + add_this_binding(block: Block, this_binding: Binding) { + const { renderer } = this; + + renderer.component.has_reactive_assignments = true; - block.chunks.mount.push(binding_callback); - } + const binding_callback = bind_this(renderer.component, block, this_binding.node, this.var); + block.chunks.mount.push(binding_callback); } add_attributes(block: Block) { diff --git a/test/js/samples/media-bindings/expected.js b/test/js/samples/media-bindings/expected.js index bfb1b8911d..b5548a3efe 100644 --- a/test/js/samples/media-bindings/expected.js +++ b/test/js/samples/media-bindings/expected.js @@ -29,26 +29,26 @@ function create_fragment(ctx) { audio_updating = true; } - /*audio_timeupdate_handler*/ ctx[10].call(audio); + /*audio_timeupdate_handler*/ ctx[12].call(audio); } return { c() { audio = element("audio"); + if (/*buffered*/ ctx[0] === void 0) add_render_callback(() => /*audio_progress_handler*/ ctx[10].call(audio)); + if (/*buffered*/ ctx[0] === void 0 || /*seekable*/ ctx[1] === void 0) add_render_callback(() => /*audio_loadedmetadata_handler*/ ctx[11].call(audio)); if (/*played*/ ctx[2] === void 0 || /*currentTime*/ ctx[3] === void 0 || /*ended*/ ctx[9] === void 0) add_render_callback(audio_timeupdate_handler); - if (/*duration*/ ctx[4] === void 0) add_render_callback(() => /*audio_durationchange_handler*/ ctx[11].call(audio)); - if (/*buffered*/ ctx[0] === void 0) add_render_callback(() => /*audio_progress_handler*/ ctx[13].call(audio)); - if (/*buffered*/ ctx[0] === void 0 || /*seekable*/ ctx[1] === void 0) add_render_callback(() => /*audio_loadedmetadata_handler*/ ctx[14].call(audio)); + if (/*duration*/ ctx[4] === void 0) add_render_callback(() => /*audio_durationchange_handler*/ ctx[13].call(audio)); if (/*seeking*/ ctx[8] === void 0) add_render_callback(() => /*audio_seeking_seeked_handler*/ ctx[17].call(audio)); if (/*ended*/ ctx[9] === void 0) add_render_callback(() => /*audio_ended_handler*/ ctx[18].call(audio)); dispose = [ + listen(audio, "progress", /*audio_progress_handler*/ ctx[10]), + listen(audio, "loadedmetadata", /*audio_loadedmetadata_handler*/ ctx[11]), listen(audio, "timeupdate", audio_timeupdate_handler), - listen(audio, "durationchange", /*audio_durationchange_handler*/ ctx[11]), - listen(audio, "play", /*audio_play_pause_handler*/ ctx[12]), - listen(audio, "pause", /*audio_play_pause_handler*/ ctx[12]), - listen(audio, "progress", /*audio_progress_handler*/ ctx[13]), - listen(audio, "loadedmetadata", /*audio_loadedmetadata_handler*/ ctx[14]), + listen(audio, "durationchange", /*audio_durationchange_handler*/ ctx[13]), + listen(audio, "play", /*audio_play_pause_handler*/ ctx[14]), + listen(audio, "pause", /*audio_play_pause_handler*/ ctx[14]), listen(audio, "volumechange", /*audio_volumechange_handler*/ ctx[15]), listen(audio, "ratechange", /*audio_ratechange_handler*/ ctx[16]), listen(audio, "seeking", /*audio_seeking_seeked_handler*/ ctx[17]), @@ -72,6 +72,8 @@ function create_fragment(ctx) { audio.currentTime = /*currentTime*/ ctx[3]; } + audio_updating = false; + if (dirty & /*paused*/ 32 && audio_is_paused !== (audio_is_paused = /*paused*/ ctx[5])) { audio[audio_is_paused ? "pause" : "play"](); } @@ -83,8 +85,6 @@ function create_fragment(ctx) { if (dirty & /*playbackRate*/ 128 && !isNaN(/*playbackRate*/ ctx[7])) { audio.playbackRate = /*playbackRate*/ ctx[7]; } - - audio_updating = false; }, i: noop, o: noop, @@ -107,6 +107,18 @@ function instance($$self, $$props, $$invalidate) { let { seeking } = $$props; let { ended } = $$props; + function audio_progress_handler() { + buffered = time_ranges_to_array(this.buffered); + $$invalidate(0, buffered); + } + + function audio_loadedmetadata_handler() { + buffered = time_ranges_to_array(this.buffered); + seekable = time_ranges_to_array(this.seekable); + $$invalidate(0, buffered); + $$invalidate(1, seekable); + } + function audio_timeupdate_handler() { played = time_ranges_to_array(this.played); currentTime = this.currentTime; @@ -126,18 +138,6 @@ function instance($$self, $$props, $$invalidate) { $$invalidate(5, paused); } - function audio_progress_handler() { - buffered = time_ranges_to_array(this.buffered); - $$invalidate(0, buffered); - } - - function audio_loadedmetadata_handler() { - buffered = time_ranges_to_array(this.buffered); - seekable = time_ranges_to_array(this.seekable); - $$invalidate(0, buffered); - $$invalidate(1, seekable); - } - function audio_volumechange_handler() { volume = this.volume; $$invalidate(6, volume); @@ -182,11 +182,11 @@ function instance($$self, $$props, $$invalidate) { playbackRate, seeking, ended, + audio_progress_handler, + audio_loadedmetadata_handler, audio_timeupdate_handler, audio_durationchange_handler, audio_play_pause_handler, - audio_progress_handler, - audio_loadedmetadata_handler, audio_volumechange_handler, audio_ratechange_handler, audio_seeking_seeked_handler, diff --git a/test/js/samples/video-bindings/expected.js b/test/js/samples/video-bindings/expected.js index e3d48f9922..5b734a70a6 100644 --- a/test/js/samples/video-bindings/expected.js +++ b/test/js/samples/video-bindings/expected.js @@ -17,8 +17,8 @@ import { function create_fragment(ctx) { let video; let video_updating = false; - let video_resize_listener; let video_animationframe; + let video_resize_listener; let dispose; function video_timeupdate_handler() { @@ -29,23 +29,23 @@ function create_fragment(ctx) { video_updating = true; } - /*video_timeupdate_handler*/ ctx[5].call(video); + /*video_timeupdate_handler*/ ctx[4].call(video); } return { c() { video = element("video"); - add_render_callback(() => /*video_elementresize_handler*/ ctx[4].call(video)); - if (/*videoHeight*/ ctx[1] === void 0 || /*videoWidth*/ ctx[2] === void 0) add_render_callback(() => /*video_resize_handler*/ ctx[6].call(video)); + if (/*videoHeight*/ ctx[1] === void 0 || /*videoWidth*/ ctx[2] === void 0) add_render_callback(() => /*video_resize_handler*/ ctx[5].call(video)); + add_render_callback(() => /*video_elementresize_handler*/ ctx[6].call(video)); dispose = [ listen(video, "timeupdate", video_timeupdate_handler), - listen(video, "resize", /*video_resize_handler*/ ctx[6]) + listen(video, "resize", /*video_resize_handler*/ ctx[5]) ]; }, m(target, anchor) { insert(target, video, anchor); - video_resize_listener = add_resize_listener(video, /*video_elementresize_handler*/ ctx[4].bind(video)); + video_resize_listener = add_resize_listener(video, /*video_elementresize_handler*/ ctx[6].bind(video)); }, p(ctx, [dirty]) { if (!video_updating && dirty & /*currentTime*/ 1 && !isNaN(/*currentTime*/ ctx[0])) { @@ -70,11 +70,6 @@ function instance($$self, $$props, $$invalidate) { let { videoWidth } = $$props; let { offsetWidth } = $$props; - function video_elementresize_handler() { - offsetWidth = this.offsetWidth; - $$invalidate(3, offsetWidth); - } - function video_timeupdate_handler() { currentTime = this.currentTime; $$invalidate(0, currentTime); @@ -87,6 +82,11 @@ function instance($$self, $$props, $$invalidate) { $$invalidate(2, videoWidth); } + function video_elementresize_handler() { + offsetWidth = this.offsetWidth; + $$invalidate(3, offsetWidth); + } + $$self.$set = $$props => { if ("currentTime" in $$props) $$invalidate(0, currentTime = $$props.currentTime); if ("videoHeight" in $$props) $$invalidate(1, videoHeight = $$props.videoHeight); @@ -99,9 +99,9 @@ function instance($$self, $$props, $$invalidate) { videoHeight, videoWidth, offsetWidth, - video_elementresize_handler, video_timeupdate_handler, - video_resize_handler + video_resize_handler, + video_elementresize_handler ]; } diff --git a/test/runtime/samples/apply-directives-in-order/_config.js b/test/runtime/samples/apply-directives-in-order/_config.js new file mode 100644 index 0000000000..e5e8980ed1 --- /dev/null +++ b/test/runtime/samples/apply-directives-in-order/_config.js @@ -0,0 +1,37 @@ +export default { + props: { + value: '' + }, + + html: ` + +

+ `, + + ssrHtml: ` + +

+ `, + + async test({ assert, component, target, window }) { + const input = target.querySelector('input'); + + const event = new window.Event('input'); + input.value = 'h'; + await input.dispatchEvent(event); + + assert.equal(input.value, 'H'); + assert.htmlEqual(target.innerHTML, ` + +

H

+ `); + + input.value = 'he'; + await input.dispatchEvent(event); + assert.equal(input.value, 'HE'); + assert.htmlEqual(target.innerHTML, ` + +

HE

+ `); + }, +}; diff --git a/test/runtime/samples/apply-directives-in-order/main.svelte b/test/runtime/samples/apply-directives-in-order/main.svelte new file mode 100644 index 0000000000..be652c7b79 --- /dev/null +++ b/test/runtime/samples/apply-directives-in-order/main.svelte @@ -0,0 +1,10 @@ + + + +

{value}