sveltejs#2446 - apply directives in order

src/compile/render-dom/wrappers/Element/index.ts

@@ -21,6 +21,8 @@ import add_actions from '../shared/add_actions';
 import create_debugging_comment from '../shared/create_debugging_comment';
 import { get_context_merger } from '../shared/get_context_merger';
 import Slot from '../../../nodes/Slot';
+import EventHandler from "../../../nodes/EventHandler";
+import BindingWrapper from "./Binding";
 
 const events = [
 	{
@@ -308,8 +310,7 @@ export default class ElementWrapper extends Wrapper {
 			block.maintain_context = true;
 		}
 
-		this.add_bindings(block);
-		this.add_event_handlers(block);
+		this.add_directives_in_order(block);
 		this.add_attributes(block);
 		this.add_transitions(block);
 		this.add_animation(block);
@@ -389,6 +390,186 @@ export default class ElementWrapper extends Wrapper {
 			: `{}`}, ${this.node.namespace === namespaces.svg ? true : false})`;
 	}
 
+	add_directives_in_order (block: Block) {
+		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 ordered = [...bindingGroups, ...this.node.handlers];
+		ordered.sort((a, b) => {
+			const startA = a instanceof EventHandler ? (a as EventHandler).start : a.bindings[0].node.start;
+			const startB = b instanceof EventHandler ? (b as EventHandler).start : b.bindings[0].node.start;
+			return startA - startB;
+		});
+
+		ordered.forEach(bindingGroupOrEventHandler => {
+			if (bindingGroupOrEventHandler instanceof EventHandler) {
+				add_event_handlers(block, this.var, [bindingGroupOrEventHandler as EventHandler])
+			} else {
+				this.add_binding(block, bindingGroupOrEventHandler)
+			}
+		});
+	}
+
+	add_binding(block: Block, bindingGroup) {
+		const { renderer } = this;
+
+		if (bindingGroup.bindings.length === 0) return;
+
+		renderer.component.has_reactive_assignments = true;
+
+		const lock = bindingGroup.bindings.some(binding => binding.needs_lock) ?
+			block.get_unique_name(`${this.var}_updating`) :
+			null;
+
+		if (lock) block.add_variable(lock, 'false');
+
+		[bindingGroup].forEach(group => {
+			const handler = renderer.component.get_unique_name(`${this.var}_${group.events.join('_')}_handler`);
+
+			renderer.component.add_var({
+				name: handler,
+				internal: true,
+				referenced: true
+			});
+
+			// TODO figure out how to handle locks
+			const needs_lock = group.bindings.some(binding => binding.needs_lock);
+
+			const dependencies = new Set();
+			const contextual_dependencies = new Set();
+
+			group.bindings.forEach(binding => {
+				// TODO this is a mess
+				add_to_set(dependencies, binding.get_dependencies());
+				add_to_set(contextual_dependencies, binding.node.expression.contextual_dependencies);
+				add_to_set(contextual_dependencies, binding.handler.contextual_dependencies);
+
+				binding.render(block, lock);
+			});
+
+			// media bindings — awkward special case. The native timeupdate events
+			// fire too infrequently, so we need to take matters into our
+			// own hands
+			let animation_frame;
+			if (group.events[0] === 'timeupdate') {
+				animation_frame = block.get_unique_name(`${this.var}_animationframe`);
+				block.add_variable(animation_frame);
+			}
+
+			const has_local_function = contextual_dependencies.size > 0 || needs_lock || animation_frame;
+
+			let callee;
+
+			// TODO dry this out — similar code for event handlers and component bindings
+			if (has_local_function) {
+				// need to create a block-local function that calls an instance-level function
+				block.builders.init.add_block(deindent`
+					function ${handler}() {
+						${animation_frame && deindent`
+						cancelAnimationFrame(${animation_frame});
+						if (!${this.var}.paused) ${animation_frame} = requestAnimationFrame(${handler});`}
+						${needs_lock && `${lock} = true;`}
+						ctx.${handler}.call(${this.var}${contextual_dependencies.size > 0 ? ', ctx' : ''});
+					}
+				`);
+
+				callee = handler;
+			} else {
+				callee = `ctx.${handler}`;
+			}
+
+			this.renderer.component.partly_hoisted.push(deindent`
+				function ${handler}(${contextual_dependencies.size > 0 ? `{ ${Array.from(contextual_dependencies).join(', ')} }` : ``}) {
+					${group.bindings.map(b => b.handler.mutation)}
+					${Array.from(dependencies).filter(dep => dep[0] !== '$').map(dep => `${this.renderer.component.invalidate(dep)};`)}
+				}
+			`);
+
+			group.events.forEach(name => {
+				if (name === 'resize') {
+					// special case
+					const resize_listener = block.get_unique_name(`${this.var}_resize_listener`);
+					block.add_variable(resize_listener);
+
+					block.builders.mount.add_line(
+						`${resize_listener} = @add_resize_listener(${this.var}, ${callee}.bind(${this.var}));`
+					);
+
+					block.builders.destroy.add_line(
+						`${resize_listener}.cancel();`
+					);
+				} else {
+					block.event_listeners.push(
+						`@listen(${this.var}, "${name}", ${callee})`
+					);
+				}
+			});
+
+			const some_initial_state_is_undefined = group.bindings
+				.map(binding => `${binding.snippet} === void 0`)
+				.join(' || ');
+
+			if (this.node.name === 'select' || group.bindings.find(binding => binding.node.name === 'indeterminate' || binding.is_readonly_media_attribute())) {
+				const callback = has_local_function ? handler : `() => ${callee}.call(${this.var})`;
+				block.builders.hydrate.add_line(
+					`if (${some_initial_state_is_undefined}) @add_render_callback(${callback});`
+				);
+			}
+
+			if (group.events[0] === 'resize') {
+				block.builders.hydrate.add_line(
+					`@add_render_callback(() => ${callee}.call(${this.var}));`
+				);
+			}
+		});
+
+		if (lock) {
+			block.builders.update.add_line(`${lock} = false;`);
+		}
+
+		const this_binding = bindingGroup.bindings.find(b => b.node.name === 'this');
+		if (this_binding) {
+			const name = renderer.component.get_unique_name(`${this.var}_binding`);
+
+			renderer.component.add_var({
+				name,
+				internal: true,
+				referenced: true
+			});
+
+			const { handler, object } = this_binding;
+
+			const args = [];
+			for (const arg of handler.contextual_dependencies) {
+				args.push(arg);
+				block.add_variable(arg, `ctx.${arg}`);
+			}
+
+			renderer.component.partly_hoisted.push(deindent`
+				function ${name}(${['$$node', 'check'].concat(args).join(', ')}) {
+					${handler.snippet ? `if ($$node || (!$$node && ${handler.snippet} === check)) ` : ''}${handler.mutation}
+					${renderer.component.invalidate(object)};
+				}
+			`);
+
+			block.builders.mount.add_line(`@add_binding_callback(() => ctx.${name}(${[this.var, 'null'].concat(args).join(', ')}));`);
+			block.builders.destroy.add_line(`ctx.${name}(${['null', this.var].concat(args).join(', ')});`);
+			block.builders.update.add_line(deindent`
+				if (changed.items) {
+					ctx.${name}(${['null', this.var].concat(args).join(', ')});
+					${args.map(a => `${a} = ctx.${a}`).join(', ')};
+					ctx.${name}(${[this.var, 'null'].concat(args).join(', ')});
+				}`
+			);
+		}
+	}
+
 	add_bindings(block: Block) {
 		const { renderer } = this;
 

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>