fix: unskip branches of earlier batches after commit (#18048)

Fixes #17571 where the situation is the following:

A derived creates a new query. That query initializes loading with true.
This means the if block is marked for destruction (therefore effects
inside branch are skipped), but it's not doing that yet because the
query promise is pending. Then query resolves and loading is set back to
false right before resolving, but it's not the same tick so
`loading=false` is a separate thing. Because that later batch doesn't
see any overlap with an earlier batch (the earlier batch did set loading
to true but not via set but indirectly via recreating the query) it
doesn't wait on it and flushes right away. Now the if block is marked as
visible again but the earlier batch doesn't know that if noone unskips
its branch. If we don't do that the render effect that is now dirty as
part of that batch will not run.

.changeset/petite-signs-flash.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: unskip branches of earlier batches after commit

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

@@ -172,6 +172,12 @@ export class Batch {
 	 */
 	#skipped_branches = new Map();
 
+	/**
+	 * Inverse of #skipped_branches which we need to tell prior batches to unskip them when committing
+	 * @type {Set<Effect>}
+	 */
+	#unskipped_branches = new Set();
+
 	is_fork = false;
 
 	#decrement_queued = false;
@@ -215,28 +221,31 @@ export class Batch {
 		if (!this.#skipped_branches.has(effect)) {
 			this.#skipped_branches.set(effect, { d: [], m: [] });
 		}
+		this.#unskipped_branches.delete(effect);
 	}
 
 	/**
 	 * Remove an effect from the #skipped_branches map and reschedule
 	 * any tracked dirty/maybe_dirty child effects
 	 * @param {Effect} effect
+	 * @param {(e: Effect) => void} callback
 	 */
-	unskip_effect(effect) {
+	unskip_effect(effect, callback = (e) => this.schedule(e)) {
 		var tracked = this.#skipped_branches.get(effect);
 		if (tracked) {
 			this.#skipped_branches.delete(effect);
 
 			for (var e of tracked.d) {
 				set_signal_status(e, DIRTY);
-				this.schedule(e);
+				callback(e);
 			}
 
 			for (e of tracked.m) {
 				set_signal_status(e, MAYBE_DIRTY);
-				this.schedule(e);
+				callback(e);
 			}
 		}
+		this.#unskipped_branches.add(effect);
 	}
 
 	#process() {
@@ -532,6 +541,19 @@ export class Batch {
 					invariant(batch.#roots.length === 0, 'Batch has scheduled roots');
 				}
 
+				// A batch was unskipped in a later batch -> tell prior batches to unskip it, too
+				if (is_earlier) {
+					for (const unskipped of this.#unskipped_branches) {
+						batch.unskip_effect(unskipped, (e) => {
+							if ((e.f & (BLOCK_EFFECT | ASYNC)) !== 0) {
+								batch.schedule(e);
+							} else {
+								batch.#defer_effects([e]);
+							}
+						});
+					}
+				}
+
 				batch.activate();
 
 				/** @type {Set<Value>} */

packages/svelte/tests/runtime-runes/samples/async-if-block-unskip/_config.js

@@ -0,0 +1,30 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target }) {
+		await tick();
+		const [load, resolve] = target.querySelectorAll('button');
+
+		load.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<button>load</button>
+			<button>resolve</button>
+		`
+		);
+
+		resolve.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			search search search search
+			<button>load</button>
+			<button>resolve</button>
+		`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-if-block-unskip/main.svelte

@@ -0,0 +1,40 @@
+<script>
+	let query = $state('');
+	// changing the query results in a new promise with loading initialized to true
+	const promise = $derived(push(query));
+
+	const resolvers = [];
+	function push(value) {
+		if (!value) return Promise.resolve(value);
+
+		const { promise, resolve } = Promise.withResolvers();
+
+		resolvers.push(() => {
+			// before resolving, set loading to false - this makes it run in a different batch
+			loading = false;
+			resolve(value);
+		});
+
+		let loading = $state(true);
+		Object.defineProperty(promise, 'loading', {
+			get() {
+				return loading;
+			}
+		});
+
+		return promise
+	}
+</script>
+
+{query} {await promise}
+
+{#if !promise.loading}
+	{query}
+{/if}
+
+{#if !promise.loading}
+	{await query}
+{/if}
+
+<button onclick={() => query = 'search'}>load</button>
+<button onclick={() => resolvers.shift()?.()}>resolve</button>