Skip to content

Commit

Permalink
fix: improve event delegation handler hoisting (sveltejs#9929)
Browse files Browse the repository at this point in the history
* fix: improve event delegation handler hoisting

* fixes
  • Loading branch information
trueadm authored Dec 15, 2023
1 parent 59c7487 commit b8f3c49
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changeset/real-guests-do.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: improve event delegation handler hoisting
14 changes: 14 additions & 0 deletions packages/svelte/src/compiler/phases/1-parse/utils/html.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,16 @@ function validate_code(code) {

// based on http://developers.whatwg.org/syntax.html#syntax-tag-omission

const interactive_elements = new Set([
'a',
'button',
'iframe',
'embed',
'input',
'select',
'textarea'
]);

/** @type {Record<string, Set<string>>} */
const disallowed_contents = {
li: new Set(['li']),
Expand All @@ -143,6 +153,10 @@ const disallowed_contents = {
th: new Set(['td', 'th', 'tr'])
};

for (const interactive_element of interactive_elements) {
disallowed_contents[interactive_element] = interactive_elements;
}

// can this be a child of the parent element, or does it implicitly
// close it, like `<li>one<li>two`?

Expand Down
11 changes: 11 additions & 0 deletions packages/svelte/src/compiler/phases/2-analyze/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ function get_delegated_event(node, context) {
return non_hoistable;
}

const visited_references = new Set();
const scope = target_function.metadata.scope;
for (const [reference] of scope.references) {
// Bail-out if the arguments keyword is used
Expand All @@ -174,6 +175,15 @@ function get_delegated_event(node, context) {
}
const binding = scope.get(reference);

// If we have multiple references to the same store using $ prefix, bail out.
if (
binding !== null &&
binding.kind === 'store_sub' &&
visited_references.has(reference.slice(1))
) {
return non_hoistable;
}

if (
binding !== null &&
// Bail-out if the the binding is a rest param
Expand All @@ -188,6 +198,7 @@ function get_delegated_event(node, context) {
) {
return non_hoistable;
}
visited_references.add(reference);
}
return { type: 'hoistable', function: target_function };
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,7 @@ function get_hoistable_params(node, context) {
binding.kind === 'prop' &&
!binding.reassigned &&
binding.initial === null &&
!context.state.analysis.accessors &&
context.state.analysis.runes
!context.state.analysis.accessors
) {
// Handle $$props.something use-cases
if (!added_props) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { test } from '../../test';

export default test({
compileOptions: { dev: true }, // tests `@validate_store` code generation

html: `<button>clicks:\n0</button>`
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<script>
import {writable} from 'svelte/store'
let store = writable(0)
</script>

<button onclick={() => {
if(store && $store){

}
}}>
clicks: {$store}
</button>

0 comments on commit b8f3c49

Please sign in to comment.