From 110e37ee518b0e94bfca90ca1d539ae7e2c9cdf3 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 May 2025 21:09:12 -0400 Subject: [PATCH 1/5] fix: only re-run directly applied attachment if it changed --- .changeset/slimy-drinks-divide.md | 5 ++++ .../client/dom/elements/attachments.js | 24 ++++++++++++++----- .../attachment-in-mutated-state/_config.js | 16 +++++++++++++ .../attachment-in-mutated-state/main.svelte | 15 ++++++++++++ 4 files changed, 54 insertions(+), 6 deletions(-) create mode 100644 .changeset/slimy-drinks-divide.md create mode 100644 packages/svelte/tests/runtime-legacy/samples/attachment-in-mutated-state/_config.js create mode 100644 packages/svelte/tests/runtime-legacy/samples/attachment-in-mutated-state/main.svelte diff --git a/.changeset/slimy-drinks-divide.md b/.changeset/slimy-drinks-divide.md new file mode 100644 index 000000000000..420d0bed5632 --- /dev/null +++ b/.changeset/slimy-drinks-divide.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: only re-run directly applied attachment if it changed diff --git a/packages/svelte/src/internal/client/dom/elements/attachments.js b/packages/svelte/src/internal/client/dom/elements/attachments.js index 6e3089a384c1..5c790dabaedc 100644 --- a/packages/svelte/src/internal/client/dom/elements/attachments.js +++ b/packages/svelte/src/internal/client/dom/elements/attachments.js @@ -1,15 +1,27 @@ -import { effect } from '../../reactivity/effects.js'; +/** @import { Effect } from '#client' */ +import { block, branch, destroy_effect, effect } from '../../reactivity/effects.js'; /** * @param {Element} node * @param {() => (node: Element) => void} get_fn */ export function attach(node, get_fn) { - effect(() => { - const fn = get_fn(); + /** @type {false | undefined | ((node: Element) => void)} */ + var fn = undefined; - // we use `&&` rather than `?.` so that things like - // `{@attach DEV && something_dev_only()}` work - return fn && fn(node); + /** @type {Effect | null} */ + var effect; + + block(() => { + if (fn !== (fn = get_fn())) { + if (effect) { + destroy_effect(effect); + effect = null; + } + + if (fn) { + effect = branch(() => /** @type {(node: Element) => void} */ (fn)(node)); + } + } }); } diff --git a/packages/svelte/tests/runtime-legacy/samples/attachment-in-mutated-state/_config.js b/packages/svelte/tests/runtime-legacy/samples/attachment-in-mutated-state/_config.js new file mode 100644 index 000000000000..5d3725235842 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/attachment-in-mutated-state/_config.js @@ -0,0 +1,16 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + test({ assert, target, logs }) { + assert.deepEqual(logs, ['up']); + + const button = target.querySelector('button'); + + flushSync(() => button?.click()); + assert.deepEqual(logs, ['up']); + + flushSync(() => button?.click()); + assert.deepEqual(logs, ['up', 'down']); + } +}); diff --git a/packages/svelte/tests/runtime-legacy/samples/attachment-in-mutated-state/main.svelte b/packages/svelte/tests/runtime-legacy/samples/attachment-in-mutated-state/main.svelte new file mode 100644 index 000000000000..fbec108c7a01 --- /dev/null +++ b/packages/svelte/tests/runtime-legacy/samples/attachment-in-mutated-state/main.svelte @@ -0,0 +1,15 @@ + + + + +{#if state.count < 2} +
+{/if} From b18407a99bea0d9f555037c6b65ea93f87c32cc0 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 May 2025 21:12:11 -0400 Subject: [PATCH 2/5] add note --- .../svelte/src/internal/client/dom/elements/attachments.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/svelte/src/internal/client/dom/elements/attachments.js b/packages/svelte/src/internal/client/dom/elements/attachments.js index 5c790dabaedc..0134f5ea9502 100644 --- a/packages/svelte/src/internal/client/dom/elements/attachments.js +++ b/packages/svelte/src/internal/client/dom/elements/attachments.js @@ -1,5 +1,9 @@ /** @import { Effect } from '#client' */ -import { block, branch, destroy_effect, effect } from '../../reactivity/effects.js'; +import { block, branch, destroy_effect } from '../../reactivity/effects.js'; + +// TODO in 6.0 or 7.0, when we remove legacy mode, we can simplify this by +// getting rid of the block/branch stuff and just letting the effect rip. +// see https://github.com/sveltejs/svelte/pull/15962 /** * @param {Element} node From 9f6c4cf84838de8d8cb93e7217de37871839ac8b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 May 2025 21:39:20 -0400 Subject: [PATCH 3/5] one down --- .../3-transform/client/visitors/shared/component.js | 6 +++--- packages/svelte/src/compiler/utils/builders.js | 12 +++++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) 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 6d3d8a68e68e..d8de3fc729e7 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 @@ -261,10 +261,10 @@ export function build_component(node, component_name, context, anchor = context. let expression = /** @type {Expression} */ (context.visit(attribute.expression)); if (attribute.metadata.expression.has_state) { - expression = b.arrow([b.id('$$node')], b.call(expression, b.id('$$node'))); + push_prop(b.get(b.call('$.attachment'), [b.return(expression)], true)); + } else { + push_prop(b.prop('init', b.call('$.attachment'), expression, true)); } - - push_prop(b.prop('get', b.call('$.attachment'), expression, true)); } } diff --git a/packages/svelte/src/compiler/utils/builders.js b/packages/svelte/src/compiler/utils/builders.js index 736738d19f15..05090b1b399c 100644 --- a/packages/svelte/src/compiler/utils/builders.js +++ b/packages/svelte/src/compiler/utils/builders.js @@ -231,12 +231,18 @@ export function function_declaration(id, params, body) { } /** - * @param {string} name + * @param {ESTree.Expression | string} name * @param {ESTree.Statement[]} body + * @param {boolean} computed * @returns {ESTree.Property & { value: ESTree.FunctionExpression}}} */ -export function get(name, body) { - return prop('get', key(name), function_builder(null, [], block(body))); +export function get(name, body, computed = false) { + return prop( + 'get', + typeof name === 'string' ? key(name) : name, + function_builder(null, [], block(body)), + computed + ); } /** From 4e3bd9489732187be222efd9d5abf0f08e8b09bb Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 May 2025 21:52:22 -0400 Subject: [PATCH 4/5] fix --- .../internal/client/dom/elements/attachments.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/elements/attachments.js b/packages/svelte/src/internal/client/dom/elements/attachments.js index 0134f5ea9502..4fc128013888 100644 --- a/packages/svelte/src/internal/client/dom/elements/attachments.js +++ b/packages/svelte/src/internal/client/dom/elements/attachments.js @@ -1,5 +1,5 @@ /** @import { Effect } from '#client' */ -import { block, branch, destroy_effect } from '../../reactivity/effects.js'; +import { block, branch, effect, destroy_effect } from '../../reactivity/effects.js'; // TODO in 6.0 or 7.0, when we remove legacy mode, we can simplify this by // getting rid of the block/branch stuff and just letting the effect rip. @@ -14,17 +14,19 @@ export function attach(node, get_fn) { var fn = undefined; /** @type {Effect | null} */ - var effect; + var e; block(() => { if (fn !== (fn = get_fn())) { - if (effect) { - destroy_effect(effect); - effect = null; + if (e) { + destroy_effect(e); + e = null; } if (fn) { - effect = branch(() => /** @type {(node: Element) => void} */ (fn)(node)); + e = branch(() => { + effect(() => /** @type {(node: Element) => void} */ (fn)(node)); + }); } } }); From 2b13874dae14585b38051f849904d56a81448c2e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 19 May 2025 21:52:33 -0400 Subject: [PATCH 5/5] Revert "one down" This reverts commit 9f6c4cf84838de8d8cb93e7217de37871839ac8b. --- .../3-transform/client/visitors/shared/component.js | 6 +++--- packages/svelte/src/compiler/utils/builders.js | 12 +++--------- 2 files changed, 6 insertions(+), 12 deletions(-) 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 d8de3fc729e7..6d3d8a68e68e 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 @@ -261,10 +261,10 @@ export function build_component(node, component_name, context, anchor = context. let expression = /** @type {Expression} */ (context.visit(attribute.expression)); if (attribute.metadata.expression.has_state) { - push_prop(b.get(b.call('$.attachment'), [b.return(expression)], true)); - } else { - push_prop(b.prop('init', b.call('$.attachment'), expression, true)); + expression = b.arrow([b.id('$$node')], b.call(expression, b.id('$$node'))); } + + push_prop(b.prop('get', b.call('$.attachment'), expression, true)); } } diff --git a/packages/svelte/src/compiler/utils/builders.js b/packages/svelte/src/compiler/utils/builders.js index 05090b1b399c..736738d19f15 100644 --- a/packages/svelte/src/compiler/utils/builders.js +++ b/packages/svelte/src/compiler/utils/builders.js @@ -231,18 +231,12 @@ export function function_declaration(id, params, body) { } /** - * @param {ESTree.Expression | string} name + * @param {string} name * @param {ESTree.Statement[]} body - * @param {boolean} computed * @returns {ESTree.Property & { value: ESTree.FunctionExpression}}} */ -export function get(name, body, computed = false) { - return prop( - 'get', - typeof name === 'string' ? key(name) : name, - function_builder(null, [], block(body)), - computed - ); +export function get(name, body) { + return prop('get', key(name), function_builder(null, [], block(body))); } /**