Skip to content

Commit

Permalink
fix: don't hoist function when already referenced in module scope
Browse files Browse the repository at this point in the history
fixes #10279
  • Loading branch information
dummdidumm committed Jan 24, 2024
1 parent 14d7b26 commit 1538264
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 12 deletions.
5 changes: 5 additions & 0 deletions .changeset/many-trees-fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"svelte": patch
---

fix: don't hoist function when already referenced in module scope
5 changes: 5 additions & 0 deletions packages/svelte/src/compiler/phases/2-analyze/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ function get_delegated_event(event_name, handler, context) {
} else if (handler.type === 'Identifier') {
binding = context.state.scope.get(handler.name);

if (context.state.analysis.module.scope.references.has(handler.name)) {
// If a binding with the same name is referenced in the module scope (even if not declared there), bail-out
return non_hoistable;
}

if (binding != null) {
for (const { path } of binding.references) {
const parent = path.at(-1);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,25 @@
import { test } from '../../test';

// Checks that event handlers are not hoisted when one of them is not delegateable
export default test({
html: `<button>0</button>`,
html: `<button>0</button><button>x0</button><button>y0</button>`,

async test({ assert, target }) {
const [button] = target.querySelectorAll('button');
const [btn1, btn2, btn3] = target.querySelectorAll('button');

button.click();
btn1.click();
await Promise.resolve();
assert.htmlEqual(target.innerHTML, '<button>1</button>');
assert.htmlEqual(target.innerHTML, '<button>1</button><button>x0</button><button>y0</button>');

button.dispatchEvent(new MouseEvent('mouseenter'));
btn1.dispatchEvent(new MouseEvent('mouseenter'));
await Promise.resolve();
assert.htmlEqual(target.innerHTML, '<button>2</button>');
assert.htmlEqual(target.innerHTML, '<button>2</button><button>x0</button><button>y0</button>');

btn2.click();
await Promise.resolve();
assert.htmlEqual(target.innerHTML, '<button>2</button><button>x1</button><button>y0</button>');

btn3.click();
await Promise.resolve();
assert.htmlEqual(target.innerHTML, '<button>2</button><button>x1</button><button>y1</button>');
}
});
Original file line number Diff line number Diff line change
@@ -1,11 +1,36 @@
<script context="module">
function declared_in_module_scope() {
return 'x';
}
let a = declared_in_module_scope();
let b = 'x';
try {
b = doesnt_exist();
} catch (e) {
b = 'y';
}
</script>

<script>
let count = $state(0)
let count1 = $state(0);
let count2 = $state(0);
let count3 = $state(0);
function increment() {
count += 1
count1 += 1;
}
function declared_in_module_scope() {
count2 += 1;
}
function doesnt_exist() {
count3 += 1;
}
</script>

<button onclick={increment} onmouseenter={increment}>
{count}
</button>
<!-- Checks that event handlers are not hoisted when one of them is not delegateable -->
<button onclick={increment} onmouseenter={increment}>{count1}</button>

<!-- Checks that event handler is not hoisted if the same name is used in the module context -->
<button onclick={declared_in_module_scope}>{a}{count2}</button>
<button onclick={doesnt_exist}>{b}{count3}</button>

0 comments on commit 1538264

Please sign in to comment.