From d17755a8b5ef54a47b161786610147ead7c24815 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 23 Jul 2024 20:27:44 +0100 Subject: [PATCH] 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 --- .changeset/bright-colts-play.md | 5 ++ .../3-transform/client/visitors/template.js | 51 +++++++++++++++---- .../dynamic-element-event-handler3/_config.js | 15 ++++++ .../main.svelte | 9 ++++ .../samples/event-attribute-import/_config.js | 18 +++---- .../samples/event-attribute-import/event.js | 14 ----- .../event-attribute-import/event.svelte.js | 18 +++++++ .../event-attribute-import/main.svelte | 5 +- 8 files changed, 99 insertions(+), 36 deletions(-) create mode 100644 .changeset/bright-colts-play.md create mode 100644 packages/svelte/tests/runtime-legacy/samples/dynamic-element-event-handler3/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/dynamic-element-event-handler3/main.svelte delete mode 100644 packages/svelte/tests/runtime-runes/samples/event-attribute-import/event.js create mode 100644 packages/svelte/tests/runtime-runes/samples/event-attribute-import/event.svelte.js diff --git a/.changeset/bright-colts-play.md b/.changeset/bright-colts-play.md new file mode 100644 index 000000000000..758264da1020 --- /dev/null +++ b/.changeset/bright-colts-play.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: ensure dynamic event handlers are wrapped in a derived diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js index af10f0a4cd6f..a1f231398ba8 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js +++ b/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} 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')]; diff --git a/packages/svelte/tests/runtime-legacy/samples/dynamic-element-event-handler3/_config.js b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-event-handler3/_config.js new file mode 100644 index 000000000000..01c02087f779 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-event-handler3/_config.js @@ -0,0 +1,15 @@ +import { test } from '../../test'; + +export default test({ + html: '', + + test({ assert, logs, target }) { + const button = target.querySelector('button'); + + button?.click(); + button?.click(); + button?.click(); + + assert.deepEqual(logs, ['create', 'trigger', 'trigger', 'trigger']); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/dynamic-element-event-handler3/main.svelte b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-event-handler3/main.svelte new file mode 100644 index 000000000000..38d088503999 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/dynamic-element-event-handler3/main.svelte @@ -0,0 +1,9 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/event-attribute-import/_config.js b/packages/svelte/tests/runtime-runes/samples/event-attribute-import/_config.js index 66bda3def6ce..957adf163f6f 100644 --- a/packages/svelte/tests/runtime-runes/samples/event-attribute-import/_config.js +++ b/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']); } }); diff --git a/packages/svelte/tests/runtime-runes/samples/event-attribute-import/event.js b/packages/svelte/tests/runtime-runes/samples/event-attribute-import/event.js deleted file mode 100644 index 978f672d3074..000000000000 --- a/packages/svelte/tests/runtime-runes/samples/event-attribute-import/event.js +++ /dev/null @@ -1,14 +0,0 @@ -/** @type {any[]} */ -export const log = []; - -export const log_a = () => { - log.push('a'); -}; - -export const log_b = () => { - log.push('b'); -}; - -export const handler = { - value: log_a -}; diff --git a/packages/svelte/tests/runtime-runes/samples/event-attribute-import/event.svelte.js b/packages/svelte/tests/runtime-runes/samples/event-attribute-import/event.svelte.js new file mode 100644 index 000000000000..3f0f41995ba9 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/event-attribute-import/event.svelte.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; + } +}; diff --git a/packages/svelte/tests/runtime-runes/samples/event-attribute-import/main.svelte b/packages/svelte/tests/runtime-runes/samples/event-attribute-import/main.svelte index 1b743653a7b2..ff07e70a0689 100644 --- a/packages/svelte/tests/runtime-runes/samples/event-attribute-import/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/event-attribute-import/main.svelte @@ -1,6 +1,7 @@ - + +