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.

Author: dummdidumm

.changeset/many-jobs-speak.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: ensure async batches get noticed of new effects

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<Effect>} */ (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);
+			}
 		}
 	}
 

packages/svelte/tests/runtime-runes/samples/async-state-new-branch/Child.svelte

@@ -0,0 +1,7 @@
+<script>
+	let { x } = $props();
+	console.log(x);
+	$effect(() => console.log('$effect: '+ x))
+</script>
+
+{x}

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,
+			`
+			<button>x</button>
+			<button>y++</button>
+			<button>resolve</button>
+			world
+		`
+		);
+
+		resolve.click();
+		await tick();
+		assert.deepEqual(logs, [
+			'universe',
+			'world',
+			'$effect: world',
+			'$effect: universe',
+			'$effect: universe'
+		]);
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<button>x</button>
+			<button>y++</button>
+			<button>resolve</button>
+			universe
+			universe
+			universe
+		`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-state-new-branch/main.svelte

@@ -0,0 +1,28 @@
+<script>
+	import Child from './Child.svelte';
+
+	let x = $state('world');
+	let y = $state(0);
+	let deferred = [];
+
+	function delay(s) {
+		const d = Promise.withResolvers();
+		deferred.push(() => d.resolve(s))
+		return d.promise;
+	}
+</script>
+
+<button onclick={() => x = 'universe'}>x</button>
+
+<button onclick={() => y++}>y++</button>
+
+<button onclick={() => deferred.shift()()}>resolve</button>
+
+{#if x === 'universe'}
+	{await delay(x)}
+	<Child {x} />
+{/if}
+
+{#if y > 0}
+	<Child {x} />
+{/if}