From 51bd24ea5c82af4def9ee8e4824e23604db9079c Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 7 Nov 2025 22:45:58 +0100 Subject: [PATCH] split effect collection and execution when rebasing - prevents each loops breaking in async when certain race conditions occur --- .../src/internal/client/reactivity/batch.js | 60 +++++++++++++------ 1 file changed, 42 insertions(+), 18 deletions(-) diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 57aa185a31..ad147a992b 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -348,15 +348,13 @@ export class Batch { var previous_batch_values = batch_values; var is_earlier = true; - /** @type {EffectTarget} */ - var dummy_target = { - parent: null, - effect: null, - effects: [], - render_effects: [], - block_effects: [] - }; + /** @type {Map>} */ + const batch_effects = new Map(); + // First loop: collect effects to run for each batch + // Do this before running them because rerunning an effect + // might change its dependencies and so other batches could + // run effects when they shouldn't or not when they should. for (const batch of batches) { if (batch === this) { is_earlier = false; @@ -388,18 +386,43 @@ export class Batch { // Re-run async/block effects that depend on distinct values changed in both batches const others = [...batch.current.keys()].filter((s) => !this.current.has(s)); if (others.length > 0) { + /** @type {Set} */ + const effects = new Set(); /** @type {Set} */ const marked = new Set(); /** @type {Map} */ const checked = new Map(); for (const source of sources) { - mark_effects(source, others, marked, checked); + mark_effects(source, others, effects, marked, checked); } - if (queued_root_effects.length > 0) { - current_batch = batch; - batch.apply(); + if (effects.size > 0) { + batch_effects.set(batch, effects); + } + } + } + // Second loop: schedule effects and traverse effect trees + if (batch_effects.size > 0) { + /** @type {EffectTarget} */ + var dummy_target = { + parent: null, + effect: null, + effects: [], + render_effects: [], + block_effects: [] + }; + + for (const [batch, effects] of batch_effects) { + current_batch = batch; + batch.apply(); + + for (const effect of effects) { + set_signal_status(effect, DIRTY); + schedule_effect(effect); + } + + if (queued_root_effects.length > 0) { for (const root of queued_root_effects) { batch.#traverse_effect_tree(root, dummy_target); } @@ -407,8 +430,9 @@ export class Batch { // TODO do we need to do anything with `target`? defer block effects? queued_root_effects = []; - batch.deactivate(); } + + batch.deactivate(); } } @@ -710,10 +734,11 @@ function flush_queued_effects(effects) { * these effects can re-run after another batch has been committed * @param {Value} value * @param {Source[]} sources + * @param {Set} effects * @param {Set} marked * @param {Map} checked */ -function mark_effects(value, sources, marked, checked) { +function mark_effects(value, sources, effects, marked, checked) { if (marked.has(value)) return; marked.add(value); @@ -722,14 +747,13 @@ function mark_effects(value, sources, marked, checked) { const flags = reaction.f; if ((flags & DERIVED) !== 0) { - mark_effects(/** @type {Derived} */ (reaction), sources, marked, checked); + mark_effects(/** @type {Derived} */ (reaction), sources, effects, marked, checked); } else if ( (flags & (ASYNC | BLOCK_EFFECT)) !== 0 && - (flags & DIRTY) === 0 && // we may have scheduled this one already + !effects.has(/** @type {Effect} */ (reaction)) && depends_on(reaction, sources, checked) ) { - set_signal_status(reaction, DIRTY); - schedule_effect(/** @type {Effect} */ (reaction)); + effects.add(/** @type {Effect} */ (reaction)); } } }