From cd9f093b74845e89c1e014da4c7605ae8b4b7896 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Mon, 3 Nov 2025 14:15:05 +0100 Subject: [PATCH] fix: ensure async batches get noticed of new effects If you have a value X that causes a pending batch, and while that batch is pending cause creation of another batch which creates a new branch and therefore new effects which also read that value X, those template effects are not correctly rescheduled once the pending batch resolves because the pending batch does not know about them. As a result, the shown values of X get out of sync (once shows the old, once the new value). This partially fixes that by telling the pending batch about the newly created effects. Partially fixes #17099, but not completely because effects that are only indirectly relying on the source that change update, but those that directly rely on the source do not. In this example that means if you do `{x}` in the template it does not work, because it directly invokes the source x, but if I do e.g. `{JSON.stringify(x)}` it does work because the intermediate derived is marked as outdated and therefore reruns, reinvoking the source getter in the process. I assume it's because `MAYBE_DIRTY` for the effect isn't enough, since the version check logic determines that the effect's dependencies aren't newer. I believe we can only fix that by changing the strategy and going back to rerunning `mark_reactions` on changed sources in a batch similar to how it was before #16487 but without running into the infinite loop problem, possibly by keeping track of which of these reactions are async effects that have already run. --- .changeset/many-jobs-speak.md | 5 + .../src/internal/client/reactivity/batch.js | 110 +++++++++++++++++- .../async-state-new-branch/Child.svelte | 7 ++ .../samples/async-state-new-branch/_config.js | 46 ++++++++ .../async-state-new-branch/main.svelte | 28 +++++ 5 files changed, 190 insertions(+), 6 deletions(-) create mode 100644 .changeset/many-jobs-speak.md create mode 100644 packages/svelte/tests/runtime-runes/samples/async-state-new-branch/Child.svelte create mode 100644 packages/svelte/tests/runtime-runes/samples/async-state-new-branch/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/async-state-new-branch/main.svelte diff --git a/.changeset/many-jobs-speak.md b/.changeset/many-jobs-speak.md new file mode 100644 index 000000000000..717e7ebd0468 --- /dev/null +++ b/.changeset/many-jobs-speak.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: ensure async batches get noticed of new effects diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 27c90d770843..afcd2ea1177f 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -228,8 +228,65 @@ export class Batch { } else if (async_mode_flag && (flags & RENDER_EFFECT) !== 0) { target.render_effects.push(effect); } else if (is_dirty(effect)) { - if ((effect.f & BLOCK_EFFECT) !== 0) target.block_effects.push(effect); - update_effect(effect); + if ((effect.f & BLOCK_EFFECT) !== 0) { + target.block_effects.push(effect); + + let branches; + let has_multiple_batches = batches.size > 1; + if (has_multiple_batches) { + branches = new Set(); + let b = effect.first; + + while (b !== null) { + if ((b.f & BRANCH_EFFECT) !== 0) { + branches.add(b); + } + b = b.next; + } + } + + update_effect(effect); + + let new_branches; + if (has_multiple_batches) { + new_branches = new Set(); + let b = effect.first; + + while (b !== null) { + const next = b.next; + if (!(/** @type {Set} */ (branches).has(b))) { + new_branches.add(b); + } + b = next; + } + + const new_target = { + parent: null, + effect, + effects: [], + render_effects: [], + block_effects: [] + }; + + // Traverse any new branches added due to running the block effect and collect their effects... + if (new_branches.size > 0) { + for (const b of new_branches) { + this.#traverse_new_effects(b, new_target); + } + } + + // ...then defer those effects in other batches, as they could have changed values that these effects depend on + for (const b of batches) { + if (b !== this) { + b.#defer_effects(new_target.render_effects, false); + b.#defer_effects(new_target.effects, false); + b.#defer_effects(new_target.block_effects, false); + } + } + } + } else { + update_effect(effect); + } } var child = effect.first; @@ -261,16 +318,57 @@ export class Batch { } } + /** + * Traverse the newly created effect tree, adding effects to the appropriate lists + * @param {Effect} root + * @param {EffectTarget} target + */ + #traverse_new_effects(root, target) { + var effect = root.first; + + while (effect !== null) { + var flags = effect.f; + if (effect.fn !== null) { + if ((flags & EFFECT) !== 0) { + target.effects.push(effect); + } else if ((flags & RENDER_EFFECT) !== 0) { + target.render_effects.push(effect); + } else if ((effect.f & BLOCK_EFFECT) !== 0) { + target.block_effects.push(effect); + } + + var child = effect.first; + + if (child !== null) { + effect = child; + continue; + } + } + + var parent = effect.parent; + effect = effect.next; + + while (effect === null && parent !== null && parent !== root) { + effect = parent.next; + parent = parent.parent; + } + } + } + /** * @param {Effect[]} effects + * @param {boolean} use_status */ - #defer_effects(effects) { + #defer_effects(effects, use_status = true) { for (const e of effects) { - const target = (e.f & DIRTY) !== 0 ? this.#dirty_effects : this.#maybe_dirty_effects; + const target = + (e.f & DIRTY) !== 0 && use_status ? this.#dirty_effects : this.#maybe_dirty_effects; target.push(e); - // mark as clean so they get scheduled if they depend on pending async state - set_signal_status(e, CLEAN); + if (use_status) { + // mark as clean so they get scheduled if they depend on pending async state + set_signal_status(e, CLEAN); + } } } diff --git a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch/Child.svelte b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch/Child.svelte new file mode 100644 index 000000000000..6b765526c82f --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch/Child.svelte @@ -0,0 +1,7 @@ + + +{x} 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 new file mode 100644 index 000000000000..f4b6cc777bde --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch/_config.js @@ -0,0 +1,46 @@ +import { tick } from 'svelte'; +import { test } from '../../test'; + +export default test({ + async test({ assert, target, logs }) { + const [x, y, resolve] = target.querySelectorAll('button'); + + x.click(); + await tick(); + assert.deepEqual(logs, ['universe']); + + y.click(); + await tick(); + assert.deepEqual(logs, ['universe', 'world', '$effect: world']); + assert.htmlEqual( + target.innerHTML, + ` + + + + world + ` + ); + + resolve.click(); + await tick(); + assert.deepEqual(logs, [ + 'universe', + 'world', + '$effect: world', + '$effect: universe', + '$effect: universe' + ]); + assert.htmlEqual( + target.innerHTML, + ` + + + + universe + universe + universe + ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-state-new-branch/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch/main.svelte new file mode 100644 index 000000000000..8edc718de2ae --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-state-new-branch/main.svelte @@ -0,0 +1,28 @@ + + + + + + + + +{#if x === 'universe'} + {await delay(x)} + +{/if} + +{#if y > 0} + +{/if}