From a9d8439ad1eecde5c4a91dca1eb1f034abfe7d81 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Sat, 28 Mar 2026 00:02:04 +0100 Subject: [PATCH] fix: reschedule new effects in prior batches (#18021) If a batch creates a new branch (e.g. through an if block becoming true) the previous batches so far do not know about the new effects created through that. This can lead to stale values being shown. We therefore schedule those new effects on prior batches if they are touched by a `current` value of that batch Fixes #17099 extracted from #17971 --- .changeset/yellow-hairs-laugh.md | 5 +++ .../src/internal/client/reactivity/batch.js | 32 +++++++++++++++++++ .../src/internal/client/reactivity/effects.js | 4 ++- .../async-state-new-branch-1/_config.js | 13 ++++++-- .../async-state-new-branch-2/_config.js | 9 ++++-- .../async-state-new-branch-3/_config.js | 11 +++++-- .../async-state-new-branch-3/main.svelte | 1 - .../async-state-new-branch-fork-2/_config.js | 3 +- .../async-state-new-branch-fork-5/_config.js | 1 - .../samples/async-state-new-branch/_config.js | 1 - 10 files changed, 68 insertions(+), 12 deletions(-) create mode 100644 .changeset/yellow-hairs-laugh.md diff --git a/.changeset/yellow-hairs-laugh.md b/.changeset/yellow-hairs-laugh.md new file mode 100644 index 0000000000..1c1ede8dc6 --- /dev/null +++ b/.changeset/yellow-hairs-laugh.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: reschedule new effects in prior batches diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 3b10d6ebe6..d53d824d03 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -145,6 +145,12 @@ export class Batch { */ #roots = []; + /** + * Effects created while this batch was active. + * @type {Effect[]} + */ + #new_effects = []; + /** * Deferred effects (which run after async work has completed) that are DIRTY * @type {Set} @@ -472,6 +478,13 @@ export class Batch { batches.delete(this); } + /** + * @param {Effect} effect + */ + register_created_effect(effect) { + this.#new_effects.push(effect); + } + #commit() { // If there are other pending batches, they now need to be 'rebased' — // in other words, we re-run block/async effects with the newly @@ -525,6 +538,25 @@ export class Batch { mark_effects(source, others, marked, checked); } + checked = new Map(); + var current_unequal = [...batch.current.keys()].filter((c) => + this.current.has(c) ? /** @type {[any, boolean]} */ (this.current.get(c))[0] !== c : true + ); + + for (const effect of this.#new_effects) { + if ( + (effect.f & (DESTROYED | INERT | EAGER_EFFECT)) === 0 && + depends_on(effect, current_unequal, checked) + ) { + if ((effect.f & (ASYNC | BLOCK_EFFECT)) !== 0) { + set_signal_status(effect, DIRTY); + batch.schedule(effect); + } else { + batch.#dirty_effects.add(effect); + } + } + } + // Only apply and traverse when we know we triggered async work with marking the effects if (batch.#roots.length > 0) { batch.apply(); diff --git a/packages/svelte/src/internal/client/reactivity/effects.js b/packages/svelte/src/internal/client/reactivity/effects.js index 54c8a17d79..ea8a4b645e 100644 --- a/packages/svelte/src/internal/client/reactivity/effects.js +++ b/packages/svelte/src/internal/client/reactivity/effects.js @@ -42,7 +42,7 @@ import { DEV } from 'esm-env'; import { define_property } from '../../shared/utils.js'; import { get_next_sibling } from '../dom/operations.js'; import { component_context, dev_current_component_function, dev_stack } from '../context.js'; -import { Batch, collected_effects } from './batch.js'; +import { Batch, collected_effects, current_batch } from './batch.js'; import { flatten, increment_pending } from './async.js'; import { without_reactive_context } from '../dom/elements/bindings/shared.js'; import { set_signal_status } from './status.js'; @@ -120,6 +120,8 @@ function create_effect(type, fn) { effect.component_function = dev_current_component_function; } + current_batch?.register_created_effect(effect); + /** @type {Effect | null} */ var e = effect; diff --git a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-1/_config.js b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-1/_config.js index 0af275009c..dee8af2446 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-1/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-1/_config.js @@ -2,7 +2,6 @@ import { tick } from 'svelte'; import { test } from '../../test'; export default test({ - skip: true, // TODO works on https://github.com/sveltejs/svelte/pull/17971 async test({ assert, target, logs }) { const [x, y, resolve] = target.querySelectorAll('button'); @@ -17,12 +16,20 @@ export default test({ - ` // if this shows world world - that would also be ok + world + ` // if this does not show world - that would also be ok ); resolve.click(); await tick(); - assert.deepEqual(logs, ['universe', 'universe', '$effect: universe', '$effect: universe']); + assert.deepEqual(logs, [ + 'universe', + 'world', + '$effect: world', + '$effect: universe', + '$effect: universe' + ]); + // assert.deepEqual(logs, ['universe', 'universe', '$effect: universe', '$effect: universe']); // this would also be ok assert.htmlEqual( target.innerHTML, ` diff --git a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-2/_config.js b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-2/_config.js index 035616dfb6..d99f0df731 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-2/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-2/_config.js @@ -2,7 +2,6 @@ import { tick } from 'svelte'; import { test } from '../../test'; export default test({ - skip: true, // TODO works on https://github.com/sveltejs/svelte/pull/17971 async test({ assert, target }) { const [x, y, resolve] = target.querySelectorAll('button'); @@ -18,7 +17,13 @@ export default test({
- ` // if this shows world world "world" world world world "world" - then this would also be ok + world + "world" + world + world + world + "world" + ` // if this does not show world "world" world world world "world" - then this would also be ok ); resolve.click(); diff --git a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-3/_config.js b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-3/_config.js index a2d615b6e5..eb4485e8a6 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-3/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-3/_config.js @@ -2,7 +2,6 @@ import { tick } from 'svelte'; import { test } from '../../test'; export default test({ - skip: true, // TODO works on https://github.com/sveltejs/svelte/pull/17971 async test({ assert, target }) { const [x, y, resolve] = target.querySelectorAll('button'); @@ -30,9 +29,17 @@ export default test({
- ` // if this shows world world "world" world world world "world" - then this would also be ok + world + "world" + world + world + world + "world" + ` // if this does not show world "world" world world world "world" - then this would also be ok ); + resolve.click(); + await tick(); resolve.click(); await tick(); assert.htmlEqual( diff --git a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-3/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-3/main.svelte index b02ab20995..c8a4ca587f 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-3/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-3/main.svelte @@ -31,4 +31,3 @@ {#if y > 0} {/if} - \ No newline at end of file diff --git a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-fork-2/_config.js b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-fork-2/_config.js index a712e70630..74df968d82 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-fork-2/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-fork-2/_config.js @@ -2,7 +2,6 @@ import { tick } from 'svelte'; import { test } from '../../test'; export default test({ - skip: true, // TODO works on https://github.com/sveltejs/svelte/pull/17971 async test({ assert, target }) { const [x, y, shift, pop, commit] = target.querySelectorAll('button'); @@ -43,6 +42,8 @@ export default test({ await tick(); shift.click(); await tick(); + shift.click(); // would be ok to not need this one + await tick(); assert.htmlEqual( target.innerHTML, ` diff --git a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-fork-5/_config.js b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-fork-5/_config.js index 9221a96c2e..e8f16ade3c 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-fork-5/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch-fork-5/_config.js @@ -2,7 +2,6 @@ import { tick } from 'svelte'; import { test } from '../../test'; export default test({ - skip: true, // TODO works on https://github.com/sveltejs/svelte/pull/17971 async test({ assert, target }) { const [x, y, resolve, commit] = target.querySelectorAll('button'); diff --git a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch/_config.js b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch/_config.js index f2091eb6ab..f4b6cc777b 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch/_config.js @@ -2,7 +2,6 @@ import { tick } from 'svelte'; import { test } from '../../test'; export default test({ - skip: true, // this fails on main, too; skip for now async test({ assert, target, logs }) { const [x, y, resolve] = target.querySelectorAll('button');