Skip to content

Commit

Permalink
FIX #2446: apply bindings and event handlers in order
Browse files Browse the repository at this point in the history
  • Loading branch information
@colincasey @Conduitry
colincasey authored and Conduitry committed Dec 23, 2019
1 parent fb6d570 commit abe88f3
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 58 deletions.
74 changes: 54 additions & 20 deletions src/compiler/compile/render_dom/wrappers/Element/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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<EventHandler | BindingGroup | Binding> = [].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);

Expand Down Expand Up @@ -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) {
Expand Down
50 changes: 25 additions & 25 deletions test/js/samples/media-bindings/expected.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]),
Expand All @@ -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"]();
}
Expand All @@ -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,
Expand All @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
26 changes: 13 additions & 13 deletions test/js/samples/video-bindings/expected.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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])) {
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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
];
}

Expand Down
37 changes: 37 additions & 0 deletions test/runtime/samples/apply-directives-in-order/_config.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
export default {
props: {
value: ''
},

html: `
<input>
<p></p>
`,

ssrHtml: `
<input>
<p></p>
`,

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, `
<input>
<p>H</p>
`);

input.value = 'he';
await input.dispatchEvent(event);
assert.equal(input.value, 'HE');
assert.htmlEqual(target.innerHTML, `
<input>
<p>HE</p>
`);
},
};
10 changes: 10 additions & 0 deletions test/runtime/samples/apply-directives-in-order/main.svelte
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<script>
export let value;
function uppercase(event) {
event.target.value = event.target.value.toUpperCase()
}
</script>

<input on:input={uppercase} bind:value>
<p>{value}</p>

0 comments on commit abe88f3

Please sign in to comment.