From 00eefb6c8168f01dccc797db8c54e80553db00f8 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Fri, 5 Jun 2026 23:52:07 +0200 Subject: [PATCH] fix: create batch.eager for forks with eager work (#18362) Replaces the ad-hoc `STATE_EAGER_EFFECT` flag which makes eager effects over-execute with a mechanism that is two-fold: 1. eager versions are sorted by batch. If a batch belongs to a fork, that way it's made sure it doesn't "escape" that fork 2. batches now have an `eager` field, which is set for forks with eager work. It's flushed on commit just before the main commit. This way the eager work happens separately and eagerly just before the main fork commits, which itself may not be ready yet. Additionally in `sources.js` we make sure `active_batch` is set properly when flushing eager effects. --------- Co-authored-by: Rich Harris Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com> Co-authored-by: Vercel Co-authored-by: Rich-Harris --- .../svelte/src/internal/client/constants.js | 1 - .../src/internal/client/reactivity/batch.js | 88 +++++++++++-------- .../src/internal/client/reactivity/sources.js | 7 +- .../samples/async-eager-derived/main.svelte | 1 - 4 files changed, 53 insertions(+), 44 deletions(-) diff --git a/packages/svelte/src/internal/client/constants.js b/packages/svelte/src/internal/client/constants.js index d96cb5204c..bc9f014ce4 100644 --- a/packages/svelte/src/internal/client/constants.js +++ b/packages/svelte/src/internal/client/constants.js @@ -42,7 +42,6 @@ export const HEAD_EFFECT = 1 << 18; export const EFFECT_PRESERVED = 1 << 19; export const USER_EFFECT = 1 << 20; export const EFFECT_OFFSCREEN = 1 << 25; -export const STATE_EAGER_EFFECT = 1 << 27; // Flags exclusive to deriveds /** diff --git a/packages/svelte/src/internal/client/reactivity/batch.js b/packages/svelte/src/internal/client/reactivity/batch.js index 566adc5b03..16d1ba273a 100644 --- a/packages/svelte/src/internal/client/reactivity/batch.js +++ b/packages/svelte/src/internal/client/reactivity/batch.js @@ -15,7 +15,6 @@ import { ERROR_VALUE, MANAGED_EFFECT, REACTION_RAN, - STATE_EAGER_EFFECT, DESTROYING } from '#client/constants'; import { async_mode_flag } from '../../flags/index.js'; @@ -110,6 +109,13 @@ export class Batch { linked = true; + /** + * In case of a fork with eager effects, this is the related eager batch + * that will commit eagerly updating effects just before this fork commits. + * @type {Batch | null} + */ + eager = null; + /** @type {Batch | null} */ #prev = null; @@ -1242,16 +1248,22 @@ function depends_on(reaction, sources, checked) { return false; } -/** @type {Source[]} */ -let eager_versions = []; +/** @type {Map[]>} */ +let eager_map = new Map(); let running_eager_effect = false; -function eager_flush() { +/** @param {Batch} batch */ +function eager_flush(batch) { flushSync(() => { - const eager = eager_versions; - eager_versions = []; - for (const version of eager) { + const versions = /** @type {Source[]} */ (eager_map.get(batch)); + eager_map.delete(batch); + const eager_batch = (current_batch = batch.eager ?? new Batch()); + if (batch.is_fork) { + eager_batch.is_fork = true; + batch.eager = eager_batch; + } + for (const version of versions) { update(version); } }); @@ -1289,7 +1301,7 @@ export function eager(fn) { version.label = ''; } - var effect = eager_effect(() => { + eager_effect(() => { if (initial) { // the first time this runs, we create an eager effect // that will run eagerly whenever the expression changes @@ -1308,23 +1320,18 @@ export function eager(fn) { return; } - if (!current_batch?.is_fork) { - // the second time this effect runs, it's to schedule a - // `version` update. since this will recreate the effect, - // we don't need to evaluate the expression here - if (eager_versions.length === 0) { - queue_micro_task(eager_flush); - } - - eager_versions.push(version); - } else { - fn(); + // the second time this effect runs, it's to schedule a + // `version` update. since this will recreate the effect, + // we don't need to evaluate the expression here + const batch = /** @type {Batch} */ (current_batch); + const versions = eager_map.get(batch) ?? []; + if (versions.length === 0) { + eager_map.set(batch, versions); + queue_micro_task(() => eager_flush(batch)); } - }); - // TODO ideally this wouldn't be necessary. I haven't figured out a way for these - // effects to correctly be marked dirty when `$state.eager(...)` arguments change - effect.f |= STATE_EAGER_EFFECT; + versions.push(version); + }); initial = false; @@ -1430,24 +1437,26 @@ export function fork(fn) { value.wv = snapshot.wv; } - // trigger any `$state.eager(...)` expressions with the new state. - // eager effects don't get scheduled like other effects, so we - // can't just encounter them during traversal, we need to - // proactively flush them - // TODO maybe there's a better implementation? - // e.g. maybe we can just schedule them so that they run - // with everything else during batch.flush? - flushSync(() => { - /** @type {Set} */ - var eager_effects = new Set(); - - for (var source of batch.current.keys()) { - mark_eager_effects(source, eager_effects); + // Flush the eager batch first to update any effects depending + // on `$state.eager/$effect.pending` + if (batch.eager) { + const eager = batch.eager; + eager.is_fork = false; + + for ([reaction, cv] of eager.cvs) { + if (cv > reaction.cv) { + reaction.cv = cv; + } } - set_eager_effects(eager_effects); - flush_eager_effects(); - }); + for ([value, snapshot] of eager.current) { + value.v = snapshot.v; + value.wv = snapshot.wv; + } + + eager.flush(); + batch.eager = null; + } batch.flush(); await settled; @@ -1460,6 +1469,7 @@ export function fork(fn) { } if (!committed && batch.linked) { + batch.eager?.discard(); batch.discard(); } } diff --git a/packages/svelte/src/internal/client/reactivity/sources.js b/packages/svelte/src/internal/client/reactivity/sources.js index 2514163fdd..aa0206a917 100644 --- a/packages/svelte/src/internal/client/reactivity/sources.js +++ b/packages/svelte/src/internal/client/reactivity/sources.js @@ -27,7 +27,6 @@ import { ASYNC, WAS_MARKED, CONNECTED, - STATE_EAGER_EFFECT, REACTION_RAN } from '#client/constants'; import * as e from '../errors.js'; @@ -41,7 +40,8 @@ import { legacy_updates, set_cv, get_cv, - active_batch + active_batch, + current_batch } from './batch.js'; import { proxy } from '../proxy.js'; import { execute_derived } from './deriveds.js'; @@ -245,6 +245,7 @@ export function internal_set(source, value, updated_during_traversal = null) { } if (!batch.is_fork && eager_effects.size > 0 && !eager_effects_deferred) { + current_batch?.apply(); // the current batch is not necessarily the active batch, ensure it is flush_eager_effects(); } } @@ -259,7 +260,7 @@ export function flush_eager_effects() { let dirty; try { - dirty = (effect.f & STATE_EAGER_EFFECT) !== 0 || is_dirty(effect); + dirty = is_dirty(effect); } catch { // Dirty-checking can evaluate derived dependencies and throw in cases where // parent effects are about to destroy this eager effect. Run the effect so diff --git a/packages/svelte/tests/runtime-runes/samples/async-eager-derived/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-eager-derived/main.svelte index d1d979126d..e199386870 100644 --- a/packages/svelte/tests/runtime-runes/samples/async-eager-derived/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/async-eager-derived/main.svelte @@ -1,5 +1,4 @@