Skip to content

Commit

Permalink
fix: properly add owners to function bindings (#14962)
Browse files Browse the repository at this point in the history
Closes #14956
  • Loading branch information
Rich-Harris authored Jan 9, 2025
1 parent 68cffa8 commit c0842d1
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 10 deletions.
5 changes: 5 additions & 0 deletions .changeset/yellow-dodos-smell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: properly add owners to function bindings
Original file line number Diff line number Diff line change
Expand Up @@ -174,23 +174,32 @@ export function build_component(node, component_name, context, anchor = context.
} else if (attribute.type === 'BindDirective') {
const expression = /** @type {Expression} */ (context.visit(attribute.expression));

if (dev && attribute.name !== 'this' && attribute.expression.type !== 'SequenceExpression') {
const left = object(attribute.expression);
let binding;
if (dev && attribute.name !== 'this') {
let should_add_owner = true;

if (left?.type === 'Identifier') {
binding = context.state.scope.get(left.name);
if (attribute.expression.type !== 'SequenceExpression') {
const left = object(attribute.expression);

if (left?.type === 'Identifier') {
const binding = context.state.scope.get(left.name);

// Only run ownership addition on $state fields.
// Theoretically someone could create a `$state` while creating `$state.raw` or inside a `$derived.by`,
// but that feels so much of an edge case that it doesn't warrant a perf hit for the common case.
if (binding?.kind === 'derived' || binding?.kind === 'raw_state') {
should_add_owner = false;
}
}
}

// Only run ownership addition on $state fields.
// Theoretically someone could create a `$state` while creating `$state.raw` or inside a `$derived.by`,
// but that feels so much of an edge case that it doesn't warrant a perf hit for the common case.
if (binding?.kind !== 'derived' && binding?.kind !== 'raw_state') {
if (should_add_owner) {
binding_initializers.push(
b.stmt(
b.call(
b.id('$.add_owner_effect'),
b.thunk(expression),
expression.type === 'SequenceExpression'
? expression.expressions[0]
: b.thunk(expression),
b.id(component_name),
is_ignored(node, 'ownership_invalid_binding') && b.true
)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<script>
let { arr = $bindable() } = $props();
</script>

<button onclick={() => arr.push(arr.length)}></button>
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { flushSync } from 'svelte';
import { test } from '../../test';

export default test({
compileOptions: {
dev: true
},
test({ target, warnings, assert }) {
const btn = target.querySelector('button');
flushSync(() => {
btn?.click();
});
assert.deepEqual(warnings, []);

flushSync(() => {
btn?.click();
});
assert.deepEqual(warnings, []);
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<script>
import Child from './Child.svelte';
let arr = $state([]);
let arr2 = $state([]);
let len = $derived(arr.length + arr2.length);
</script>

<Child bind:arr={() => len % 2 === 0 ? arr : arr2, (v) => {}} />

0 comments on commit c0842d1

Please sign in to comment.