feat order attributes + actions too (#4156)

Co-authored-by: Conduitry <git@chor.date>

src/compiler/compile/render_dom/wrappers/Element/index.ts

@@ -16,8 +16,8 @@ import { dimensions } from '../../../../utils/patterns';
 import Binding from './Binding';
 import InlineComponentWrapper from '../InlineComponent';
 import add_to_set from '../../../utils/add_to_set';
-import add_event_handlers from '../shared/add_event_handlers';
+import { add_event_handler } from '../shared/add_event_handlers';
-import add_actions from '../shared/add_actions';
+import { add_action } from '../shared/add_actions';
 import create_debugging_comment from '../shared/create_debugging_comment';
 import { get_slot_definition } from '../shared/get_slot_definition';
 import bind_this from '../shared/bind_this';
@@ -25,6 +25,7 @@ import { is_head } from '../shared/is_head';
 import { Identifier } from 'estree';
 import EventHandler from './EventHandler';
 import { extract_names } from 'periscopic';
+import Action from '../../../nodes/Action';
 
 const events = [
 	{
@@ -380,7 +381,6 @@ export default class ElementWrapper extends Wrapper {
 		this.add_directives_in_order(block);
 		this.add_transitions(block);
 		this.add_animation(block);
-		this.add_actions(block);
 		this.add_classes(block);
 		this.add_manual_style_scoping(block);
 
@@ -441,6 +441,8 @@ export default class ElementWrapper extends Wrapper {
 			bindings: Binding[];
 		}
 
+		type OrderedAttribute = EventHandler | BindingGroup | Binding | Action;
+
 		const bindingGroups = events
 			.map(event => ({
 				events: event.event_names,
@@ -452,29 +454,37 @@ export default class ElementWrapper extends Wrapper {
 
 		const this_binding = this.bindings.find(b => b.node.name === 'this');
 
-		function getOrder (item: EventHandler | BindingGroup | Binding) {
+		function getOrder (item: OrderedAttribute) {
 			if (item instanceof EventHandler) {
 				return item.node.start;
 			} else if (item instanceof Binding) {
 				return item.node.start;
+			} else if (item instanceof Action) {
+				return item.start;
 			} else {
 				return item.bindings[0].node.start;
 			}
 		}
 
-		const ordered: Array<EventHandler | BindingGroup | Binding> = [].concat(bindingGroups, this.event_handlers, this_binding).filter(Boolean);
+		([
-
+			...bindingGroups,
-		ordered.sort((a, b) => getOrder(a) - getOrder(b));
+			...this.event_handlers,
-
+			this_binding,
-		ordered.forEach(bindingGroupOrEventHandler => {
+			...this.node.actions
-			if (bindingGroupOrEventHandler instanceof EventHandler) {
+		] as OrderedAttribute[])
-				add_event_handlers(block, this.var, [bindingGroupOrEventHandler]);
+			.filter(Boolean)
-			} else if (bindingGroupOrEventHandler instanceof Binding) {
+			.sort((a, b) => getOrder(a) - getOrder(b))
-				this.add_this_binding(block, bindingGroupOrEventHandler);
+			.forEach(item => {
-			} else {
+				if (item instanceof EventHandler) {
-				this.add_bindings(block, bindingGroupOrEventHandler);
+					add_event_handler(block, this.var, item);
-			}
+				} else if (item instanceof Binding) {
-		});
+					this.add_this_binding(block, item);
+				} else if (item instanceof Action) {
+					add_action(block, this.var, item);
+				} else {
+					this.add_bindings(block, item);
+				}
+			});
 	}
 
 	add_bindings(block: Block, bindingGroup) {
@@ -701,10 +711,6 @@ export default class ElementWrapper extends Wrapper {
 		`);
 	}
 
-	add_event_handlers(block: Block) {
-		add_event_handlers(block, this.var, this.event_handlers);
-	}
-
 	add_transitions(
 		block: Block
 	) {
@@ -866,10 +872,6 @@ export default class ElementWrapper extends Wrapper {
 		`);
 	}
 
-	add_actions(block: Block) {
-		add_actions(block, this.var, this.node.actions);
-	}
-
 	add_classes(block: Block) {
 		const has_spread = this.node.attributes.some(attr => attr.is_spread);
 		this.node.classes.forEach(class_directive => {

src/compiler/compile/render_dom/wrappers/shared/add_actions.ts

@@ -7,42 +7,40 @@ export default function add_actions(
 	target: string,
 	actions: Action[]
 ) {
-	actions.forEach(action => {
+	actions.forEach(action => add_action(block, target, action));
-		const { expression } = action;
+}
-		let snippet;
-		let dependencies;
-
-		if (expression) {
-			snippet = expression.manipulate(block);
-			dependencies = expression.dynamic_dependencies();
-		}
 
-		const id = block.get_unique_name(
+export function add_action(block: Block, target: string, action: Action) {
-			`${action.name.replace(/[^a-zA-Z0-9_$]/g, '_')}_action`
+	const { expression } = action;
-		);
+	let snippet;
+	let dependencies;
 
-		block.add_variable(id);
+	if (expression) {
+		snippet = expression.manipulate(block);
+		dependencies = expression.dynamic_dependencies();
+	}
 
-		const fn = block.renderer.reference(action.name);
+	const id = block.get_unique_name(
+		`${action.name.replace(/[^a-zA-Z0-9_$]/g, '_')}_action`
+	);
 
-		block.chunks.mount.push(
+	block.add_variable(id);
-			b`${id} = ${fn}.call(null, ${target}, ${snippet}) || {};`
+
-		);
+	const fn = block.renderer.reference(action.name);
 
-		if (dependencies && dependencies.length > 0) {
+	block.event_listeners.push(
-			let condition = x`@is_function(${id}.update)`;
+		x`@action_destroyer(${id} = ${fn}.call(null, ${target}, ${snippet}))`
+	);
 
-			if (dependencies.length > 0) {
+	if (dependencies && dependencies.length > 0) {
-				condition = x`${condition} && ${block.renderer.dirty(dependencies)}`;
+		let condition = x`${id} && @is_function(${id}.update)`;
-			}
 
-			block.chunks.update.push(
+		if (dependencies.length > 0) {
-				b`if (${condition}) ${id}.update.call(null, ${snippet});`
+			condition = x`${condition} && ${block.renderer.dirty(dependencies)}`;
-			);
 		}
 
-		block.chunks.destroy.push(
+		block.chunks.update.push(
-			b`if (${id} && @is_function(${id}.destroy)) ${id}.destroy();`
+			b`if (${condition}) ${id}.update.call(null, ${snippet});`
 		);
-	});
+	}
 }

src/compiler/compile/render_dom/wrappers/shared/add_event_handlers.ts

@@ -6,5 +6,13 @@ export default function add_event_handlers(
 	target: string,
 	handlers: EventHandler[]
 ) {
-	handlers.forEach(handler => handler.render(block, target));
+	handlers.forEach(handler => add_event_handler(block, target, handler));
+}
+
+export function add_event_handler(
+	block: Block,
+	target: string,
+	handler: EventHandler
+) {
+	handler.render(block, target);
 }

src/runtime/internal/utils.ts

@@ -120,4 +120,8 @@ export function set_store_value(store, ret, value = ret) {
 	return ret;
 }
 
-export const has_prop = (obj, prop) => Object.prototype.hasOwnProperty.call(obj, prop);
+export const has_prop = (obj, prop) => Object.prototype.hasOwnProperty.call(obj, prop);
+
+export function action_destroyer(action_result) {
+	return action_result && is_function(action_result.destroy) ? action_result.destroy : noop;
+}

test/js/samples/action-custom-event-handler/expected.js

@@ -1,6 +1,7 @@
 /* generated by Svelte vX.Y.Z */
 import {
 	SvelteComponent,
+	action_destroyer,
 	detach,
 	element,
 	init,
@@ -13,24 +14,25 @@ import {
 function create_fragment(ctx) {
 	let button;
 	let foo_action;
+	let dispose;
 
 	return {
 		c() {
 			button = element("button");
 			button.textContent = "foo";
+			dispose = action_destroyer(foo_action = foo.call(null, button, /*foo_function*/ ctx[1]));
 		},
 		m(target, anchor) {
 			insert(target, button, anchor);
-			foo_action = foo.call(null, button, /*foo_function*/ ctx[1]) || ({});
 		},
 		p(ctx, [dirty]) {
-			if (is_function(foo_action.update) && dirty & /*bar*/ 1) foo_action.update.call(null, /*foo_function*/ ctx[1]);
+			if (foo_action && is_function(foo_action.update) && dirty & /*bar*/ 1) foo_action.update.call(null, /*foo_function*/ ctx[1]);
 		},
 		i: noop,
 		o: noop,
 		d(detaching) {
 			if (detaching) detach(button);
-			if (foo_action && is_function(foo_action.destroy)) foo_action.destroy();
+			dispose();
 		}
 	};
 }
@@ -40,7 +42,7 @@ function handleFoo(bar) {
 }
 
 function foo(node, callback) {
-	
+
 }
 
 function instance($$self, $$props, $$invalidate) {

test/js/samples/action/expected.js

@@ -1,12 +1,12 @@
 /* generated by Svelte vX.Y.Z */
 import {
 	SvelteComponent,
+	action_destroyer,
 	attr,
 	detach,
 	element,
 	init,
 	insert,
-	is_function,
 	noop,
 	safe_not_equal
 } from "svelte/internal";
@@ -14,23 +14,24 @@ import {
 function create_fragment(ctx) {
 	let a;
 	let link_action;
+	let dispose;
 
 	return {
 		c() {
 			a = element("a");
 			a.textContent = "Test";
 			attr(a, "href", "#");
+			dispose = action_destroyer(link_action = link.call(null, a));
 		},
 		m(target, anchor) {
 			insert(target, a, anchor);
-			link_action = link.call(null, a) || ({});
 		},
 		p: noop,
 		i: noop,
 		o: noop,
 		d(detaching) {
 			if (detaching) detach(a);
-			if (link_action && is_function(link_action.destroy)) link_action.destroy();
+			dispose();
 		}
 	};
 }

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

@@ -0,0 +1,38 @@
+const value = [];
+export default {
+	props: {
+		value,
+	},
+
+	async test({ assert, component, target, window }) {
+		const inputs = target.querySelectorAll('input');
+
+		const event = new window.Event('input');
+
+		for (const input of inputs) {
+			input.value = 'h';
+			await input.dispatchEvent(event);
+		}
+
+		assert.deepEqual(value, [
+			'1',
+			'2',
+			'3',
+			'4',
+			'5',
+			'6',
+			'7',
+			'8',
+			'9',
+			'10',
+			'11',
+			'12',
+			'13',
+			'14',
+			'15',
+			'16',
+			'17',
+			'18',
+		]);
+	},
+};

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

@@ -0,0 +1,34 @@
+<script>
+	export let value = [];
+
+	function one(elem) { elem.addEventListener('input', () => { value.push('1'); }); }
+	function four(elem) { elem.addEventListener('input', () => { value.push('4'); }); }
+	function eight(elem) { elem.addEventListener('input', () => { value.push('8'); }); }
+	function twelve(elem) { elem.addEventListener('input', () => { value.push('12'); }); }
+	function fifteen(elem) { elem.addEventListener('input', () => { value.push('15'); }); }
+	function seventeen(elem) { elem.addEventListener('input', () => { value.push('17'); }); }
+
+	const foo = {
+		set two(v) { value.push('2'); },
+		set six(v) { value.push('6'); },
+		set nine(v) { value.push('9'); },
+		set eleven(v) { value.push('11'); },
+		set thirteen(v) { value.push('13'); },
+		set sixteen(v) { value.push('16'); },
+	}
+
+	function three() { value.push('3'); }
+	function five() { value.push('5'); }
+	function seven() { value.push('7'); }
+	function ten() { value.push('10'); }
+	function fourteen() { value.push('14'); }
+	function eighteen() { value.push('18'); }
+
+</script>
+
+<input use:one bind:value={foo.two} on:input={three} />
+<input use:four on:input={five} bind:value={foo.six} />
+<input on:input={seven} use:eight bind:value={foo.nine} />
+<input on:input={ten} bind:value={foo.eleven} use:twelve />
+<input bind:value={foo.thirteen} on:input={fourteen} use:fifteen />
+<input bind:value={foo.sixteen} use:seventeen on:input={eighteen} />