wip

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

@@ -183,6 +183,14 @@ export class Batch {
 	 */
 	#maybe_dirty_effects = new Set();
 
+	/**
+	 * Async/block effects that were scheduled in this batch before it was
+	 * started, and stashed so that their tree markers could be reset (see
+	 * #stash). Rescheduled (once) when the batch is processed
+	 * @type {Effect[]}
+	 */
+	#stashed = [];
+
 	/**
 	 * A map of branches that still exist, but will be destroyed when this batch
 	 * is committed — we skip over these during `process`.
@@ -273,6 +281,52 @@ export class Batch {
 		this.#unskipped_branches.add(effect);
 	}
 
+	/**
+	 * Stash any work that has been scheduled in this (not yet processed) batch as
+	 * deferred work, resetting the 'already scheduled' markers (cleared CLEAN flags
+	 * on branches) on the effect tree. This makes the scheduling state batch-local,
+	 * so that concurrent scheduling into _other_ batches (during a rebase) isn't
+	 * confused by this batch's tree markers. The work is rescheduled when this
+	 * batch is processed.
+	 */
+	#stash() {
+		var roots = this.#roots;
+		this.#roots = [];
+
+		for (const root of roots) {
+			this.#reset(root);
+		}
+	}
+
+	/**
+	 * @param {Effect} effect
+	 */
+	#reset(effect) {
+		// clean branch = nothing scheduled inside, no need to traverse further
+		if ((effect.f & BRANCH_EFFECT) !== 0 && (effect.f & CLEAN) !== 0) {
+			return;
+		}
+
+		if ((effect.f & (DIRTY | MAYBE_DIRTY)) !== 0) {
+			if ((effect.f & (ASYNC | BLOCK_EFFECT)) !== 0) {
+				// async/block effects run during traversal, so they must not go into
+				// #dirty_effects (whose contents are re-marked on every process) —
+				// stash them for a one-time reschedule instead, keeping their status
+				this.#stashed.push(effect);
+			} else {
+				defer_effect(effect, this.#dirty_effects, this.#maybe_dirty_effects);
+			}
+		} else {
+			set_signal_status(effect, CLEAN);
+		}
+
+		var e = effect.first;
+		while (e !== null) {
+			this.#reset(e);
+			e = e.next;
+		}
+	}
+
 	#process() {
 		this.#started = true;
 
@@ -304,6 +358,15 @@ export class Batch {
 			this.schedule(e);
 		}
 
+		if (this.#stashed.length > 0) {
+			const stashed = this.#stashed;
+			this.#stashed = [];
+
+			for (const e of stashed) {
+				this.schedule(e);
+			}
+		}
+
 		const roots = this.#roots;
 		this.#roots = [];
 
@@ -401,6 +464,14 @@ export class Batch {
 			// In sync mode flushSync can cause #commit to wrongfully think that there needs to be a rebase, so we only do it in async mode
 			// TODO fix the underlying cause, otherwise this will likely regress when non-async mode is removed
 			if (async_mode_flag) {
+				// If effects that ran during this flush wrote state, that work was scheduled
+				// in a new batch, leaving 'already scheduled' markers (cleared CLEAN flags)
+				// on the shared effect tree. Stash it as the new batch's deferred work (which
+				// resets those markers) before rebasing other in-flight batches in #commit —
+				// otherwise scheduling effects in those batches would wrongly bail, assuming
+				// the effects are already covered by a root in _their_ queue
+				if (next_batch !== null) next_batch.#stash();
+
 				this.#commit();
 				// Rebases can activate other batches or null it out, therefore restore the new one here
 				current_batch = next_batch;

packages/svelte/tests/runtime-runes/samples/async-commit-effect-overlap/_config.js

@@ -0,0 +1,41 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+const buttons = '<button>a</button> <button>b</button> <button>shift</button> <button>pop</button>';
+
+export default test({
+	async test({ assert, target }) {
+		await tick();
+
+		const [a, b, , pop] = target.querySelectorAll('button');
+		const shift = target.querySelectorAll('button')[2];
+
+		assert.htmlEqual(target.innerHTML, `${buttons} <p>a</p><p>a</p><p>aa</p><p>1</p>`);
+
+		// start two independent batches, both blocked on their awaited expression
+		a.click();
+		await tick();
+		assert.htmlEqual(target.innerHTML, `${buttons} <p>a</p><p>a</p><p>aa</p><p>1</p>`);
+
+		b.click();
+		await tick();
+		assert.htmlEqual(target.innerHTML, `${buttons} <p>a</p><p>a</p><p>aa</p><p>1</p>`);
+
+		// resolve the newer (b) batch first. Committing it must not commit the
+		// still-pending `a` batch, whose async work has not completed — `a` must
+		// still read 'a', and the unrelated `c` update must not be blocked
+		pop.click();
+		await tick();
+		assert.htmlEqual(target.innerHTML, `${buttons} <p>a</p><p>b</p><p>ab</p><p>2</p>`);
+
+		// stale promise from the `a` batch's first run — resolving it does nothing
+		shift.click();
+		await tick();
+		assert.htmlEqual(target.innerHTML, `${buttons} <p>a</p><p>b</p><p>ab</p><p>2</p>`);
+
+		// the `a` batch's re-run await ('bb') resolves — everything is committed
+		shift.click();
+		await tick();
+		assert.htmlEqual(target.innerHTML, `${buttons} <p>b</p><p>b</p><p>bb</p><p>2</p>`);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-commit-effect-overlap/main.svelte

@@ -0,0 +1,38 @@
+<script>
+	let a = $state('a');
+	let b = $state('a');
+	let c = $state(0);
+
+	let n = 0;
+
+	let queued = [];
+	let first = true;
+
+	function push(v) {
+		if (first) {
+			first = false;
+			return v;
+		}
+
+		return new Promise((resolve) => {
+			queued.push(() => resolve(v));
+		});
+	}
+
+	// when a batch changing `b` commits, this writes `c` during that batch's
+	// flush phase — the write lands in a freshly-created batch
+	$effect(() => {
+		b;
+		c = ++n;
+	});
+</script>
+
+<button onclick={() => a = 'b'}>a</button>
+<button onclick={() => b = 'b'}>b</button>
+<button onclick={() => queued.shift()?.()}>shift</button>
+<button onclick={() => queued.pop()?.()}>pop</button>
+
+<p>{a}</p>
+<p>{b}</p>
+<p>{await push(a + b)}</p>
+<p>{c}</p>