FIX #2446: apply bindings and event handlers in order

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_directives_in_order(block);
-		this.add_event_handlers(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<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
+		[bindingGroup].forEach(group => {
-			.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 => {
 			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');
+	add_this_binding(block: Block, this_binding: Binding) {
-		if (this_binding) {
+		const { renderer } = this;
-			const binding_callback = bind_this(renderer.component, block, this_binding.node, this.var);
 		
-			block.chunks.mount.push(binding_callback);
+		renderer.component.has_reactive_assignments = true;
-		}
+
+		const binding_callback = bind_this(renderer.component, block, this_binding.node, this.var);
+		block.chunks.mount.push(binding_callback);
 	}
 
 	add_attributes(block: Block) {

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 (/*duration*/ ctx[4] === void 0) add_render_callback(() => /*audio_durationchange_handler*/ ctx[13].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 (/*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, "durationchange", /*audio_durationchange_handler*/ ctx[13]),
-				listen(audio, "play", /*audio_play_pause_handler*/ ctx[12]),
+				listen(audio, "play", /*audio_play_pause_handler*/ ctx[14]),
-				listen(audio, "pause", /*audio_play_pause_handler*/ ctx[12]),
+				listen(audio, "pause", /*audio_play_pause_handler*/ ctx[14]),
-				listen(audio, "progress", /*audio_progress_handler*/ ctx[13]),
-				listen(audio, "loadedmetadata", /*audio_loadedmetadata_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,

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[5].call(video));
-			if (/*videoHeight*/ ctx[1] === void 0 || /*videoWidth*/ ctx[2] === void 0) add_render_callback(() => /*video_resize_handler*/ ctx[6].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
 	];
 }
 

test/runtime/samples/apply-directives-in-order/_config.js

@@ -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>
+		`);
+	},
+};

test/runtime/samples/apply-directives-in-order/main.svelte

@@ -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>