From ef5bcfe542752b10d84f8ff8fd1553ebc7aa9043 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Mon, 4 Dec 2023 17:45:13 +0000 Subject: [PATCH] fix: ensure event handlers containing arguments are not hoisted (#9758) * fix: ensure event handlers containing arguments are not hoisted * add test * handle rest arguments --- .changeset/serious-socks-cover.md | 5 +++++ .../src/compiler/phases/2-analyze/index.js | 20 ++++++++++++------- packages/svelte/src/compiler/phases/scope.js | 2 +- packages/svelte/src/compiler/types/index.d.ts | 1 + .../samples/event-arguments-2/_config.js | 13 ++++++++++++ .../samples/event-arguments-2/main.svelte | 8 ++++++++ .../samples/event-arguments/_config.js | 13 ++++++++++++ .../samples/event-arguments/main.svelte | 8 ++++++++ 8 files changed, 62 insertions(+), 8 deletions(-) create mode 100644 .changeset/serious-socks-cover.md create mode 100644 packages/svelte/tests/runtime-runes/samples/event-arguments-2/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/event-arguments-2/main.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/event-arguments/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/event-arguments/main.svelte diff --git a/.changeset/serious-socks-cover.md b/.changeset/serious-socks-cover.md new file mode 100644 index 0000000000..709bb95beb --- /dev/null +++ b/.changeset/serious-socks-cover.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: ensure event handlers containing arguments are not hoisted diff --git a/packages/svelte/src/compiler/phases/2-analyze/index.js b/packages/svelte/src/compiler/phases/2-analyze/index.js index 71295b7059..517827f7e3 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/index.js +++ b/packages/svelte/src/compiler/phases/2-analyze/index.js @@ -168,17 +168,23 @@ function get_delegated_event(node, context) { const scope = target_function.metadata.scope; for (const [reference] of scope.references) { + // Bail-out if the arguments keyword is used + if (reference === 'arguments') { + return non_hoistable; + } const binding = scope.get(reference); if ( binding !== null && - // Bail-out if we reference anything from the EachBlock (for now) that mutates in non-runes mode, - ((!context.state.analysis.runes && binding.kind === 'each') || - // or any normal not reactive bindings that are mutated. - binding.kind === 'normal' || - // or any reactive imports (those are rewritten) (can only happen in legacy mode) - (binding.kind === 'state' && binding.declaration_kind === 'import')) && - binding.mutated + // Bail-out if the the binding is a rest param + (binding.declaration_kind === 'rest_param' || + // Bail-out if we reference anything from the EachBlock (for now) that mutates in non-runes mode, + (((!context.state.analysis.runes && binding.kind === 'each') || + // or any normal not reactive bindings that are mutated. + binding.kind === 'normal' || + // or any reactive imports (those are rewritten) (can only happen in legacy mode) + (binding.kind === 'state' && binding.declaration_kind === 'import')) && + binding.mutated)) ) { return non_hoistable; } diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js index f4b1265550..e8e56bfe78 100644 --- a/packages/svelte/src/compiler/phases/scope.js +++ b/packages/svelte/src/compiler/phases/scope.js @@ -255,7 +255,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) { function add_params(scope, params) { for (const param of params) { for (const node of extract_identifiers(param)) { - scope.declare(node, 'normal', 'param'); + scope.declare(node, 'normal', param.type === 'RestElement' ? 'rest_param' : 'param'); } } } diff --git a/packages/svelte/src/compiler/types/index.d.ts b/packages/svelte/src/compiler/types/index.d.ts index 2c5480eb7f..04b4a9d1c3 100644 --- a/packages/svelte/src/compiler/types/index.d.ts +++ b/packages/svelte/src/compiler/types/index.d.ts @@ -237,6 +237,7 @@ export type DeclarationKind = | 'function' | 'import' | 'param' + | 'rest_param' | 'synthetic'; export interface Binding { diff --git a/packages/svelte/tests/runtime-runes/samples/event-arguments-2/_config.js b/packages/svelte/tests/runtime-runes/samples/event-arguments-2/_config.js new file mode 100644 index 0000000000..f2e6c67f8a --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/event-arguments-2/_config.js @@ -0,0 +1,13 @@ +import { test } from '../../test'; + +export default test({ + html: ``, + + async test({ assert, target }) { + const [b1] = target.querySelectorAll('button'); + + b1?.click(); + await Promise.resolve(); + assert.htmlEqual(target.innerHTML, ''); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/event-arguments-2/main.svelte b/packages/svelte/tests/runtime-runes/samples/event-arguments-2/main.svelte new file mode 100644 index 0000000000..39333849f6 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/event-arguments-2/main.svelte @@ -0,0 +1,8 @@ + + + diff --git a/packages/svelte/tests/runtime-runes/samples/event-arguments/_config.js b/packages/svelte/tests/runtime-runes/samples/event-arguments/_config.js new file mode 100644 index 0000000000..f2e6c67f8a --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/event-arguments/_config.js @@ -0,0 +1,13 @@ +import { test } from '../../test'; + +export default test({ + html: ``, + + async test({ assert, target }) { + const [b1] = target.querySelectorAll('button'); + + b1?.click(); + await Promise.resolve(); + assert.htmlEqual(target.innerHTML, ''); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/event-arguments/main.svelte b/packages/svelte/tests/runtime-runes/samples/event-arguments/main.svelte new file mode 100644 index 0000000000..d6569e26fe --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/event-arguments/main.svelte @@ -0,0 +1,8 @@ + + +