Skip to content

Commit ef5bcfe

Browse files
authored
fix: ensure event handlers containing arguments are not hoisted (#9758)
* fix: ensure event handlers containing arguments are not hoisted * add test * handle rest arguments
1 parent 2017af4 commit ef5bcfe

File tree

8 files changed

+62
-8
lines changed

8 files changed

+62
-8
lines changed

.changeset/serious-socks-cover.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: ensure event handlers containing arguments are not hoisted

packages/svelte/src/compiler/phases/2-analyze/index.js

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -168,17 +168,23 @@ function get_delegated_event(node, context) {
168168

169169
const scope = target_function.metadata.scope;
170170
for (const [reference] of scope.references) {
171+
// Bail-out if the arguments keyword is used
172+
if (reference === 'arguments') {
173+
return non_hoistable;
174+
}
171175
const binding = scope.get(reference);
172176

173177
if (
174178
binding !== null &&
175-
// Bail-out if we reference anything from the EachBlock (for now) that mutates in non-runes mode,
176-
((!context.state.analysis.runes && binding.kind === 'each') ||
177-
// or any normal not reactive bindings that are mutated.
178-
binding.kind === 'normal' ||
179-
// or any reactive imports (those are rewritten) (can only happen in legacy mode)
180-
(binding.kind === 'state' && binding.declaration_kind === 'import')) &&
181-
binding.mutated
179+
// Bail-out if the the binding is a rest param
180+
(binding.declaration_kind === 'rest_param' ||
181+
// Bail-out if we reference anything from the EachBlock (for now) that mutates in non-runes mode,
182+
(((!context.state.analysis.runes && binding.kind === 'each') ||
183+
// or any normal not reactive bindings that are mutated.
184+
binding.kind === 'normal' ||
185+
// or any reactive imports (those are rewritten) (can only happen in legacy mode)
186+
(binding.kind === 'state' && binding.declaration_kind === 'import')) &&
187+
binding.mutated))
182188
) {
183189
return non_hoistable;
184190
}

packages/svelte/src/compiler/phases/scope.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
255255
function add_params(scope, params) {
256256
for (const param of params) {
257257
for (const node of extract_identifiers(param)) {
258-
scope.declare(node, 'normal', 'param');
258+
scope.declare(node, 'normal', param.type === 'RestElement' ? 'rest_param' : 'param');
259259
}
260260
}
261261
}

packages/svelte/src/compiler/types/index.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,7 @@ export type DeclarationKind =
237237
| 'function'
238238
| 'import'
239239
| 'param'
240+
| 'rest_param'
240241
| 'synthetic';
241242

242243
export interface Binding {
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { test } from '../../test';
2+
3+
export default test({
4+
html: `<button>0</button>`,
5+
6+
async test({ assert, target }) {
7+
const [b1] = target.querySelectorAll('button');
8+
9+
b1?.click();
10+
await Promise.resolve();
11+
assert.htmlEqual(target.innerHTML, '<button>1</button>');
12+
}
13+
});
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<script>
2+
let count = $state(0);
3+
function increment(...args) {
4+
count += args.length;
5+
}
6+
</script>
7+
8+
<button on:click={increment}>{count}</button>
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { test } from '../../test';
2+
3+
export default test({
4+
html: `<button>0</button>`,
5+
6+
async test({ assert, target }) {
7+
const [b1] = target.querySelectorAll('button');
8+
9+
b1?.click();
10+
await Promise.resolve();
11+
assert.htmlEqual(target.innerHTML, '<button>1</button>');
12+
}
13+
});
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<script>
2+
let count = $state(0);
3+
function increment() {
4+
count += arguments.length;
5+
}
6+
</script>
7+
8+
<button on:click={increment}>{count}</button>

0 commit comments

Comments
 (0)