From c0842d14596dff8a26b487dc6a75207d45198261 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 9 Jan 2025 05:49:07 -0500 Subject: [PATCH] fix: properly add owners to function bindings (#14962) Closes #14956 --- .changeset/yellow-dodos-smell.md | 5 ++++ .../client/visitors/shared/component.js | 29 ++++++++++++------- .../ownership-function-bindings/Child.svelte | 5 ++++ .../ownership-function-bindings/_config.js | 20 +++++++++++++ .../ownership-function-bindings/main.svelte | 10 +++++++ 5 files changed, 59 insertions(+), 10 deletions(-) create mode 100644 .changeset/yellow-dodos-smell.md create mode 100644 packages/svelte/tests/runtime-runes/samples/ownership-function-bindings/Child.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/ownership-function-bindings/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/ownership-function-bindings/main.svelte diff --git a/.changeset/yellow-dodos-smell.md b/.changeset/yellow-dodos-smell.md new file mode 100644 index 0000000000..ea2aead662 --- /dev/null +++ b/.changeset/yellow-dodos-smell.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: properly add owners to function bindings diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js index 068971145c..f509cb41a7 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/component.js @@ -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 ) diff --git a/packages/svelte/tests/runtime-runes/samples/ownership-function-bindings/Child.svelte b/packages/svelte/tests/runtime-runes/samples/ownership-function-bindings/Child.svelte new file mode 100644 index 0000000000..ef91b0756d --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/ownership-function-bindings/Child.svelte @@ -0,0 +1,5 @@ + + + \ No newline at end of file diff --git a/packages/svelte/tests/runtime-runes/samples/ownership-function-bindings/_config.js b/packages/svelte/tests/runtime-runes/samples/ownership-function-bindings/_config.js new file mode 100644 index 0000000000..4c77aea206 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/ownership-function-bindings/_config.js @@ -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, []); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/ownership-function-bindings/main.svelte b/packages/svelte/tests/runtime-runes/samples/ownership-function-bindings/main.svelte new file mode 100644 index 0000000000..4a3ce82726 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/ownership-function-bindings/main.svelte @@ -0,0 +1,10 @@ + + + len % 2 === 0 ? arr : arr2, (v) => {}} /> \ No newline at end of file