From 66396f391ee109af8420882dd89be658ac6ce9d2 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 14 Aug 2025 14:28:16 -0400 Subject: [PATCH 1/6] fix: only abort effect flushing if it causes an existing effect to be scheduled --- .changeset/clever-months-clap.md | 5 +++++ .../svelte/src/internal/client/reactivity/batch.js | 14 +++++--------- .../src/internal/client/reactivity/sources.js | 6 ++++++ 3 files changed, 16 insertions(+), 9 deletions(-) create mode 100644 .changeset/clever-months-clap.md diff --git a/.changeset/clever-months-clap.md b/.changeset/clever-months-clap.md new file mode 100644 index 000000000000..9566dcf54d66 --- /dev/null +++ b/.changeset/clever-months-clap.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: only abort effect flushing if it causes an existing effect to be scheduled diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 123bc95d163a..f4ba4b54b61f 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -28,7 +28,7 @@ import * as e from '../errors.js'; import { flush_tasks } from '../dom/task.js'; import { DEV } from 'esm-env'; import { invoke_error_boundary } from '../error-handling.js'; -import { old_values } from './sources.js'; +import { old_values, schedule_version } from './sources.js'; import { unlink_effect } from './effects.js'; import { unset_context } from './async.js'; @@ -598,7 +598,7 @@ function flush_queued_effects(effects) { var effect = effects[i++]; if ((effect.f & (DESTROYED | INERT)) === 0 && is_dirty(effect)) { - var n = current_batch ? current_batch.current.size : 0; + var sv = schedule_version; update_effect(effect); @@ -619,13 +619,9 @@ function flush_queued_effects(effects) { } } - // if state is written in a user effect, abort and re-schedule, lest we run - // effects that should be removed as a result of the state change - if ( - current_batch !== null && - current_batch.current.size > n && - (effect.f & USER_EFFECT) !== 0 - ) { + // if an effect is invalidated by a user effect, abort and re-schedule, lest we + // run effects that should be removed as a result of the state change + if (schedule_version > sv && (effect.f & USER_EFFECT) !== 0) { break; } } diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 7fb3135708d3..d2a57e1acfe3 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -299,6 +299,11 @@ export function increment(source) { set(source, source.v + 1); } +/** + * We increment this value when an effect is scheduled as a result of a state change + */ +export let schedule_version = 0; + /** * @param {Value} signal * @param {number} status should be DIRTY or MAYBE_DIRTY @@ -334,6 +339,7 @@ function mark_reactions(signal, status) { if ((flags & DERIVED) !== 0) { mark_reactions(/** @type {Derived} */ (reaction), MAYBE_DIRTY); } else if (not_dirty) { + schedule_version += 1; schedule_effect(/** @type {Effect} */ (reaction)); } } From 2048f5dae0168ff53391d66f4a229b523fda23dd Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 14 Aug 2025 14:35:51 -0400 Subject: [PATCH 2/6] tweak comment --- packages/svelte/src/internal/client/reactivity/batch.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index f4ba4b54b61f..8bdb12007df5 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -619,8 +619,8 @@ function flush_queued_effects(effects) { } } - // if an effect is invalidated by a user effect, abort and re-schedule, lest we - // run effects that should be removed as a result of the state change + // if a state change in a user effect invalidates a _different_ effect, + // abort and reschedule in case that effect now needs to be destroyed if (schedule_version > sv && (effect.f & USER_EFFECT) !== 0) { break; } From acc7bb3f7f203b10a09968119e6d8c0159b21d24 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 14 Aug 2025 14:48:17 -0400 Subject: [PATCH 3/6] add test --- .../samples/effect-loop-3/Child.svelte | 11 +++++++++++ .../samples/effect-loop-3/_config.js | 12 ++++++++++++ .../samples/effect-loop-3/main.svelte | 15 +++++++++++++++ 3 files changed, 38 insertions(+) create mode 100644 packages/svelte/tests/runtime-runes/samples/effect-loop-3/Child.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/effect-loop-3/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/effect-loop-3/main.svelte diff --git a/packages/svelte/tests/runtime-runes/samples/effect-loop-3/Child.svelte b/packages/svelte/tests/runtime-runes/samples/effect-loop-3/Child.svelte new file mode 100644 index 000000000000..71588c6ee8c3 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-loop-3/Child.svelte @@ -0,0 +1,11 @@ + + + {@render children()} diff --git a/packages/svelte/tests/runtime-runes/samples/effect-loop-3/_config.js b/packages/svelte/tests/runtime-runes/samples/effect-loop-3/_config.js new file mode 100644 index 000000000000..046c1904322a --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-loop-3/_config.js @@ -0,0 +1,12 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target }) { + const [button] = target.querySelectorAll('button'); + + assert.doesNotThrow(() => { + flushSync(() => button.click()); + }); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/effect-loop-3/main.svelte b/packages/svelte/tests/runtime-runes/samples/effect-loop-3/main.svelte new file mode 100644 index 000000000000..2b3a17179806 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/effect-loop-3/main.svelte @@ -0,0 +1,15 @@ + + + + +{#if show} + {#each { length: 1234 } as i} + {i} + {/each} +{/if} From f89e3557eef4a2d3feb849c0e8cb39e9e8c08b7e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 14 Aug 2025 22:54:01 -0400 Subject: [PATCH 4/6] restrict to block effects --- packages/svelte/src/internal/client/reactivity/sources.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index d2a57e1acfe3..93f4cdaddbe0 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -300,7 +300,8 @@ export function increment(source) { } /** - * We increment this value when an effect is scheduled as a result of a state change + * We increment this value when a block effect is scheduled as a result of a state change, + * as its currently-scheduled child effects may need to be destroyed */ export let schedule_version = 0; @@ -339,7 +340,7 @@ function mark_reactions(signal, status) { if ((flags & DERIVED) !== 0) { mark_reactions(/** @type {Derived} */ (reaction), MAYBE_DIRTY); } else if (not_dirty) { - schedule_version += 1; + if ((flags & BLOCK_EFFECT) !== 0) schedule_version += 1; schedule_effect(/** @type {Effect} */ (reaction)); } } From 12ceae113d3ce9fe05e876dc1773cf53135fbc32 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 15 Aug 2025 20:17:25 -0400 Subject: [PATCH 5/6] make test fail --- .../tests/runtime-runes/samples/effect-loop-3/Child.svelte | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/svelte/tests/runtime-runes/samples/effect-loop-3/Child.svelte b/packages/svelte/tests/runtime-runes/samples/effect-loop-3/Child.svelte index 71588c6ee8c3..9bf4db52d6b4 100644 --- a/packages/svelte/tests/runtime-runes/samples/effect-loop-3/Child.svelte +++ b/packages/svelte/tests/runtime-runes/samples/effect-loop-3/Child.svelte @@ -8,4 +8,6 @@ }); - {@render children()} +{#if inited} + {@render children()} +{/if} From d5ba21f77cae226de3ceb0073e208b8c796ed32b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 15 Aug 2025 20:32:37 -0400 Subject: [PATCH 6/6] collect all effects --- .../svelte/src/internal/client/reactivity/batch.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 8bdb12007df5..b109c792e8b0 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -292,12 +292,12 @@ export class Batch { if (!skip && effect.fn !== null) { if (is_branch) { effect.f ^= CLEAN; + } else if ((flags & EFFECT) !== 0) { + this.#effects.push(effect); + } else if (async_mode_flag && (flags & RENDER_EFFECT) !== 0) { + this.#render_effects.push(effect); } else if ((flags & CLEAN) === 0) { - if ((flags & EFFECT) !== 0) { - this.#effects.push(effect); - } else if (async_mode_flag && (flags & RENDER_EFFECT) !== 0) { - this.#render_effects.push(effect); - } else if ((flags & ASYNC) !== 0) { + if ((flags & ASYNC) !== 0) { var effects = effect.b?.pending ? this.#boundary_async_effects : this.#async_effects; effects.push(effect); } else if (is_dirty(effect)) {