fix: ensure dynamic event handlers are wrapped in a derived (#12563)

* fix: ensure dynamic event handlers are wrapped in a derived

* fix test

* feedback

* more feedback

* address feedback

* we have .svelte.js files

---------

Co-authored-by: Rich Harris <[email protected]>

Author: trueadm

.changeset/bright-colts-play.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: ensure dynamic event handlers are wrapped in a derived

packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js

@@ -714,7 +714,7 @@ function serialize_inline_component(node, component_name, context, anchor = cont
 			lets.push(/** @type {ExpressionStatement} */ (context.visit(attribute)));
 		} else if (attribute.type === 'OnDirective') {
 			events[attribute.name] ||= [];
-			let handler = serialize_event_handler(attribute, context);
+			let handler = serialize_event_handler(attribute, null, context);
 			if (attribute.modifiers.includes('once')) {
 				handler = b.call('$.once', handler);
 			}
@@ -1122,9 +1122,10 @@ function serialize_render_stmt(update) {
 /**
  * Serializes the event handler function of the `on:` directive
  * @param {Pick<import('#compiler').OnDirective, 'name' | 'modifiers' | 'expression'>} node
+ * @param {null | { contains_call_expression: boolean; dynamic: boolean; } | null} metadata
  * @param {import('../types.js').ComponentContext} context
  */
-function serialize_event_handler(node, { state, visit }) {
+function serialize_event_handler(node, metadata, { state, visit }) {
 	/** @type {Expression} */
 	let handler;
 
@@ -1166,10 +1167,33 @@ function serialize_event_handler(node, { state, visit }) {
 				handler = /** @type {Expression} */ (visit(handler));
 			}
 		} else if (
-			handler.type === 'CallExpression' ||
-			handler.type === 'ConditionalExpression' ||
-			handler.type === 'LogicalExpression'
+			metadata?.contains_call_expression &&
+			!(
+				(handler.type === 'ArrowFunctionExpression' || handler.type === 'FunctionExpression') &&
+				handler.metadata.hoistable
+			)
 		) {
+			// Create a derived dynamic event handler
+			const id = b.id(state.scope.generate('event_handler'));
+
+			state.init.push(
+				b.var(id, b.call('$.derived', b.thunk(/** @type {Expression} */ (visit(handler)))))
+			);
+
+			handler = b.function(
+				null,
+				[b.rest(b.id('$$args'))],
+				b.block([
+					b.return(
+						b.call(
+							b.member(b.call('$.get', id), b.id('apply'), false, true),
+							b.this,
+							b.id('$$args')
+						)
+					)
+				])
+			);
+		} else if (handler.type === 'ConditionalExpression' || handler.type === 'LogicalExpression') {
 			handler = dynamic_handler();
 		} else {
 			handler = /** @type {Expression} */ (visit(handler));
@@ -1206,17 +1230,18 @@ function serialize_event_handler(node, { state, visit }) {
 
 /**
  * Serializes an event handler function of the `on:` directive or an attribute starting with `on`
- * @param {{name: string; modifiers: string[]; expression: Expression | null; delegated?: import('#compiler').DelegatedEvent | null; }} node
+ * @param {{name: string;modifiers: string[];expression: Expression | null;delegated?: import('#compiler').DelegatedEvent | null;}} node
+ * @param {null | { contains_call_expression: boolean; dynamic: boolean; }} metadata
  * @param {import('../types.js').ComponentContext} context
  */
-function serialize_event(node, context) {
+function serialize_event(node, metadata, context) {
 	const state = context.state;
 
 	/** @type {Statement} */
 	let statement;
 
 	if (node.expression) {
-		let handler = serialize_event_handler(node, context);
+		let handler = serialize_event_handler(node, metadata, context);
 		const event_name = node.name;
 		const delegated = node.delegated;
 
@@ -1285,7 +1310,12 @@ function serialize_event(node, context) {
 		statement = b.stmt(b.call('$.event', ...args));
 	} else {
 		statement = b.stmt(
-			b.call('$.event', b.literal(node.name), state.node, serialize_event_handler(node, context))
+			b.call(
+				'$.event',
+				b.literal(node.name),
+				state.node,
+				serialize_event_handler(node, metadata, context)
+			)
 		);
 	}
 
@@ -1323,6 +1353,7 @@ function serialize_event_attribute(node, context) {
 			modifiers,
 			delegated: node.metadata.delegated
 		},
+		!Array.isArray(node.value) && node.value?.type === 'ExpressionTag' ? node.value.metadata : null,
 		context
 	);
 }
@@ -2797,7 +2828,7 @@ export const template_visitors = {
 		context.next({ ...context.state, in_constructor: false });
 	},
 	OnDirective(node, context) {
-		serialize_event(node, context);
+		serialize_event(node, null, context);
 	},
 	UseDirective(node, { state, next, visit }) {
 		const params = [b.id('$$node')];

packages/svelte/tests/runtime-legacy/samples/dynamic-element-event-handler3/_config.js

@@ -0,0 +1,15 @@
+import { test } from '../../test';
+
+export default test({
+	html: '<button>Click</button>',
+
+	test({ assert, logs, target }) {
+		const button = target.querySelector('button');
+
+		button?.click();
+		button?.click();
+		button?.click();
+
+		assert.deepEqual(logs, ['create', 'trigger', 'trigger', 'trigger']);
+	}
+});

packages/svelte/tests/runtime-legacy/samples/dynamic-element-event-handler3/main.svelte

@@ -0,0 +1,9 @@
+<script>
+	let makeHandler = null;
+	makeHandler = () => {
+		console.log('create');
+		return () => console.log('trigger');
+	};
+</script>
+
+<button on:click={makeHandler()}>Click</button>

packages/svelte/tests/runtime-runes/samples/event-attribute-import/_config.js

@@ -1,20 +1,18 @@
 import { test } from '../../test';
-import { log, handler, log_a } from './event.js';
 
 export default test({
-	before_test() {
-		log.length = 0;
-		handler.value = log_a;
-	},
-
-	async test({ assert, target }) {
-		const [b1, b2] = target.querySelectorAll('button');
+	async test({ assert, logs, target, component }) {
+		const [b1, b2, b3] = target.querySelectorAll('button');
 
 		b1?.click();
-		assert.deepEqual(log, ['a']);
+		assert.deepEqual(logs, ['a']);
 
 		b2?.click();
 		b1?.click();
-		assert.deepEqual(log, ['a', 'b']);
+
+		b3?.click();
+		b1?.click();
+
+		assert.deepEqual(logs, ['a', 'b', 'a']);
 	}
 });

packages/svelte/tests/runtime-runes/samples/event-attribute-import/event.js

@@ -0,0 +1,18 @@
+export const log_a = () => {
+	console.log('a');
+};
+
+export const log_b = () => {
+	console.log('b');
+};
+
+let handle = $state(log_a);
+
+export const handler = {
+	get value() {
+		return handle;
+	},
+	set value(v) {
+		handle = v;
+	}
+};

packages/svelte/tests/runtime-runes/samples/event-attribute-import/event.svelte.js

@@ -1,6 +1,7 @@
 <script>
-	import {handler, log_b} from './event.js';
+	import { handler, log_a, log_b } from './event.svelte.js';
 </script>
 
 <button onclick={handler.value}>click</button>
-<button onclick={() => handler.value = log_b}>change</button>
+<button onclick={() => (handler.value = log_b)}>change</button>
+<button onclick={() => (handler.value = log_a)}>change back</button>