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 <rich.harris@vercel.com>
Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com>
Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com>
Co-authored-by: Rich-Harris <hello@rich-harris.dev>

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
 /**

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<number>[]} */
-let eager_versions = [];
+/** @type {Map<Batch | null, Source<number>[]>} */
+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<number>[]} */ (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 = '<eager>';
 	}
 
-	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<Effect>} */
-				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();
 			}
 		}

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

packages/svelte/tests/runtime-runes/samples/async-eager-derived/main.svelte

@@ -1,5 +1,4 @@
 <script>
-	
 	let count = $state(0);
 	const delayedCount = $derived(await push(count));
 	const derivedCount = $derived(count);