fix rescheduling after commit

packages/svelte/src/internal/client/reactivity/batch.js

@@ -263,6 +263,13 @@ export class Batch {
 	 */
 	fork_values = null;
 
+	/**
+	 * Async and block effects that ran or were proven clean inside this fork.
+	 * When the fork commits, only these effects can retain their speculative results
+	 * @type {Set<Effect> | null}
+	 */
+	fork_effects = null;
+
 	/**
 	 * Reactions that observed the pre-write world of this batch (via
 	 * `batch_values`) while it was pending. When this batch commits, they
@@ -519,6 +526,13 @@ export class Batch {
 		return false;
 	}
 
+	/** @param {Effect} effect */
+	record_fork_effect(effect) {
+		if (this.is_fork && (effect.f & (ASYNC | BLOCK_EFFECT)) !== 0) {
+			(this.fork_effects ??= new Set()).add(effect);
+		}
+	}
+
 	/**
 	 * Absorb another batch into this one — their reactivity graphs overlap,
 	 * so they describe a single 'world'. All state is transferred, and stale
@@ -898,11 +912,17 @@ export class Batch {
 					effects.push(effect);
 				} else if (async_mode_flag && (flags & (RENDER_EFFECT | MANAGED_EFFECT)) !== 0) {
 					render_effects.push(effect);
-				} else if (is_dirty(effect)) {
-					if ((flags & BLOCK_EFFECT) !== 0) {
-						(this.#maybe_dirty_effects ??= new Set()).add(effect);
+				} else {
+					var dirty = is_dirty(effect);
+
+					if (dirty) {
+						if ((flags & BLOCK_EFFECT) !== 0) {
+							(this.#maybe_dirty_effects ??= new Set()).add(effect);
+						}
+						update_effect(effect);
+					} else if ((flags & MAYBE_DIRTY) !== 0) {
+						this.record_fork_effect(effect);
 					}
-					update_effect(effect);
 				}
 
 				var child = effect.first;
@@ -1190,6 +1210,7 @@ export class Batch {
 	 */
 	schedule(effect) {
 		last_scheduled_effect = effect;
+		if (this.is_fork) this.fork_effects?.delete(effect);
 
 		if (this.claim(effect)) {
 			this.#dirty_effects ??= new Set();
@@ -1488,17 +1509,18 @@ export function eager(fn) {
 
 /**
  * When a fork is committed, deriveds affected by its writes must recompute with
- * the now-committed values, and template/user effects must re-run. Async and
- * block effects are skipped — they already ran inside the fork with these
- * values (their results were captured by the fork's batch), and eager effects
- * are collected so that `$state.eager(...)` expressions update immediately
+ * the now-committed values, and affected effects must be checked. Async and
+ * block effects that were validated inside the fork are skipped, and eager
+ * effects are collected so that `$state.eager(...)` expressions update immediately
  * @param {Value} value
  * @param {Batch} batch
- * @param {Set<Value>} marked
+ * @param {Map<Value, number>} marked
+ * @param {number} status
  */
-function mark_committed_reactions(value, batch, marked) {
-	if (marked.has(value)) return;
-	marked.add(value);
+function mark_committed_reactions(value, batch, marked, status) {
+	var previous_status = marked.get(value);
+	if (previous_status === DIRTY || previous_status === status) return;
+	marked.set(value, status);
 
 	var reactions = value.reactions;
 	if (reactions === null) return;
@@ -1507,34 +1529,39 @@ function mark_committed_reactions(value, batch, marked) {
 		var flags = reaction.f;
 
 		if ((flags & EAGER_EFFECT) !== 0) {
-			set_signal_status(reaction, DIRTY);
+			if ((flags & DIRTY) === 0) {
+				set_signal_status(reaction, status);
+			}
 			eager_effects.add(/** @type {Effect} */ (reaction));
 		} else if ((flags & DERIVED) !== 0) {
 			batch.claim(reaction);
 
 			if ((flags & DIRTY) === 0) {
-				set_signal_status(reaction, MAYBE_DIRTY);
+				set_signal_status(reaction, status);
 			}
 
-			mark_committed_reactions(/** @type {Derived} */ (reaction), batch, marked);
+			mark_committed_reactions(/** @type {Derived} */ (reaction), batch, marked, MAYBE_DIRTY);
 		} else if ((flags & (ASYNC | BLOCK_EFFECT)) !== 0) {
-			// async and block effects that the fork itself ran are left alone —
-			// they already ran with these exact values. But if such an effect
-			// belongs to another live batch's world, it must re-run with the
-			// committed values (which also entangles us with that batch, as
-			// with any other write)
+			// async and block effects validated inside the fork are left alone —
+			// they already ran with these exact values or were proven unaffected.
+			// But if such an effect belongs to another live batch's world, it must
+			// re-run with the committed values (which also entangles us with that
+			// batch, as with any other write)
 			var owner = /** @type {Effect} */ (reaction).batch;
 			while (owner !== null && owner.merged_into !== null) owner = owner.merged_into;
 
-			if (owner !== null && owner !== batch && owner.linked && !owner.is_fork) {
+			if (
+				!batch.fork_effects?.has(/** @type {Effect} */ (reaction)) ||
+				(owner !== null && owner !== batch && owner.linked && !owner.is_fork)
+			) {
 				if ((reaction.f & DIRTY) === 0) {
-					set_signal_status(reaction, DIRTY);
+					set_signal_status(reaction, status);
 				}
 
 				batch.schedule(/** @type {Effect} */ (reaction));
 			}
 		} else if ((flags & DIRTY) === 0) {
-			set_signal_status(reaction, DIRTY);
+			set_signal_status(reaction, status);
 			batch.schedule(/** @type {Effect} */ (reaction));
 		}
 	}
@@ -1643,13 +1670,13 @@ export function fork(fn) {
 			// entangles us with any overlapping pending batches
 			batch.activate();
 
-			/** @type {Set<Value>} */
-			var marked = new Set();
+			/** @type {Map<Value, number>} */
+			var marked = new Map();
 
 			for (var [source, value] of batch.current) {
 				source.v = value;
 				source.wv = increment_write_version();
-				mark_committed_reactions(source, batch, marked);
+				mark_committed_reactions(source, batch, marked, DIRTY);
 			}
 
 			batch.deactivate();

packages/svelte/src/internal/client/runtime.js

@@ -454,6 +454,7 @@ export function remove_reactions(signal, start_index) {
  */
 export function update_effect(effect) {
 	var flags = effect.f;
+	var batch = current_batch;
 
 	if ((flags & DESTROYED) !== 0) {
 		return;
@@ -486,6 +487,7 @@ export function update_effect(effect) {
 		var teardown = update_reaction(effect);
 		effect.teardown = typeof teardown === 'function' ? teardown : null;
 		effect.wv = write_version;
+		batch?.record_fork_effect(effect);
 
 		// In DEV, increment versions of any sources that were written to during the effect,
 		// so that they are correctly marked as dirty when the effect re-runs

packages/svelte/tests/runtime-runes/samples/async-fork-no-rerun/_config.js

@@ -0,0 +1,18 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target, logs }) {
+		const [change, commit] = target.querySelectorAll('button');
+
+		assert.deepEqual(logs, ['hi']);
+
+		change.click();
+		await tick();
+		assert.deepEqual(logs, ['hi', 'hi']);
+
+		commit.click();
+		await tick();
+		assert.deepEqual(logs, ['hi', 'hi']);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-fork-no-rerun/main.svelte

@@ -0,0 +1,12 @@
+<script>
+	import { fork } from 'svelte';
+
+	let x = $state(1);
+	let y = $derived.by(() => {console.log('hi'); return x % 2});
+	let f;
+</script>
+
+<button onclick={() => { f = fork(() => x = 3)}}>change</button>
+<button onclick={() => f.commit()}>commit</button>
+
+{#if y}hi{/if}

packages/svelte/tests/runtime-runes/samples/async-state-new-branch-fork-4/_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');