fix: run batch until complete

packages/svelte/src/internal/client/dom/blocks/async.js

@@ -1,5 +1,6 @@
 /** @import { TemplateNode, Value } from '#client' */
 import { flatten } from '../../reactivity/async.js';
+import { Batch, current_batch } from '../../reactivity/batch.js';
 import { get } from '../../runtime.js';
 import {
 	hydrate_next,
@@ -18,8 +19,12 @@ import { get_boundary } from './boundary.js';
  */
 export function async(node, expressions, fn) {
 	var boundary = get_boundary();
+	var batch = /** @type {Batch} */ (current_batch);
+
+	var blocking = !boundary.is_pending();
 
 	boundary.update_pending_count(1);
+	batch.increment(blocking);
 
 	var was_hydrating = hydrating;
 
@@ -44,6 +49,7 @@ export function async(node, expressions, fn) {
 			fn(node, ...values);
 		} finally {
 			boundary.update_pending_count(-1);
+			batch.decrement(blocking);
 		}
 
 		if (was_hydrating) {

packages/svelte/src/internal/client/dom/blocks/boundary.js

@@ -285,13 +285,6 @@ export class Boundary {
 				this.#anchor.before(this.#offscreen_fragment);
 				this.#offscreen_fragment = null;
 			}
-
-			// TODO this feels like a little bit of a kludge, but until we
-			// overhaul the boundary/batch relationship it's probably
-			// the most pragmatic solution available to us
-			queue_micro_task(() => {
-				Batch.ensure().flush();
-			});
 		}
 	}
 

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

@@ -202,10 +202,11 @@ export function unset_context() {
 export async function async_body(fn) {
 	var boundary = get_boundary();
 	var batch = /** @type {Batch} */ (current_batch);
-	var pending = boundary.is_pending();
+
+	var blocking = !boundary.is_pending();
 
 	boundary.update_pending_count(1);
-	if (!pending) batch.increment();
+	batch.increment(blocking);
 
 	var active = /** @type {Effect} */ (active_effect);
 
@@ -238,12 +239,7 @@ export async function async_body(fn) {
 		}
 
 		boundary.update_pending_count(-1);
-
-		if (pending) {
-			batch.flush();
-		} else {
-			batch.decrement();
-		}
+		batch.decrement(blocking);
 
 		unset_context();
 	}

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

@@ -11,7 +11,8 @@ import {
 	RENDER_EFFECT,
 	ROOT_EFFECT,
 	MAYBE_DIRTY,
-	DERIVED
+	DERIVED,
+	BOUNDARY_EFFECT
 } from '#client/constants';
 import { async_mode_flag } from '../../flags/index.js';
 import { deferred, define_property } from '../../shared/utils.js';
@@ -30,6 +31,16 @@ import { invoke_error_boundary } from '../error-handling.js';
 import { old_values } from './sources.js';
 import { unlink_effect } from './effects.js';
 
+/**
+ * @typedef {{
+ *   parent: EffectTarget | null;
+ *   effect: Effect | null;
+ *   effects: Effect[];
+ *   render_effects: Effect[];
+ *   block_effects: Effect[];
+ * }} EffectTarget
+ */
+
 /** @type {Set<Batch>} */
 const batches = new Set();
 
@@ -90,33 +101,18 @@ export class Batch {
 	 */
 	#pending = 0;
 
+	/**
+	 * The number of async effects that are currently in flight, _not_ inside a pending boundary
+	 */
+	#blocking_pending = 0;
+
 	/**
 	 * A deferred that resolves when the batch is committed, used with `settled()`
 	 * TODO replace with Promise.withResolvers once supported widely enough
 	 * @type {{ promise: Promise<void>, resolve: (value?: any) => void, reject: (reason: unknown) => void } | null}
 	 */
 	#deferred = null;
 
-	/**
-	 * Template effects and `$effect.pre` effects, which run when
-	 * a batch is committed
-	 * @type {Effect[]}
-	 */
-	#render_effects = [];
-
-	/**
-	 * The same as `#render_effects`, but for `$effect` (which runs after)
-	 * @type {Effect[]}
-	 */
-	#effects = [];
-
-	/**
-	 * Block effects, which may need to re-run on subsequent flushes
-	 * in order to update internal sources (e.g. each block items)
-	 * @type {Effect[]}
-	 */
-	#block_effects = [];
-
 	/**
 	 * Deferred effects (which run after async work has completed) that are DIRTY
 	 * @type {Effect[]}
@@ -147,41 +143,42 @@ export class Batch {
 
 		this.apply();
 
+		/** @type {EffectTarget} */
+		var target = {
+			parent: null,
+			effect: null,
+			effects: [],
+			render_effects: [],
+			block_effects: []
+		};
+
 		for (const root of root_effects) {
-			this.#traverse_effect_tree(root);
+			this.#traverse_effect_tree(root, target);
 		}
 
 		// if there is no outstanding async work, commit
 		if (this.#pending === 0) {
-			// TODO we need this because we commit _then_ flush effects...
-			// maybe there's a way we can reverse the order?
-			var previous_batch_sources = batch_values;
-
+			// commit before flushing effects, since that may result in
+			// another batch being created
 			this.#commit();
+		}
 
-			var render_effects = this.#render_effects;
-			var effects = this.#effects;
-
-			this.#render_effects = [];
-			this.#effects = [];
-			this.#block_effects = [];
+		if (this.#blocking_pending > 0) {
+			this.#defer_effects(target.effects);
+			this.#defer_effects(target.render_effects);
+			this.#defer_effects(target.block_effects);
+		} else {
+			// TODO append/detach blocks here, not in #commit
 
 			// If sources are written to, then work needs to happen in a separate batch, else prior sources would be mixed with
 			// newly updated sources, which could lead to infinite loops when effects run over and over again.
 			previous_batch = this;
 			current_batch = null;
 
-			batch_values = previous_batch_sources;
-			flush_queued_effects(render_effects);
-			flush_queued_effects(effects);
+			flush_queued_effects(target.render_effects);
+			flush_queued_effects(target.effects);
 
 			previous_batch = null;
-
-			this.#deferred?.resolve();
-		} else {
-			this.#defer_effects(this.#render_effects);
-			this.#defer_effects(this.#effects);
-			this.#defer_effects(this.#block_effects);
 		}
 
 		batch_values = null;
@@ -191,8 +188,9 @@ export class Batch {
 	 * Traverse the effect tree, executing effects or stashing
 	 * them for later execution as appropriate
 	 * @param {Effect} root
+	 * @param {EffectTarget} target
 	 */
-	#traverse_effect_tree(root) {
+	#traverse_effect_tree(root, target) {
 		root.f ^= CLEAN;
 
 		var effect = root.first;
@@ -204,15 +202,25 @@ export class Batch {
 
 			var skip = is_skippable_branch || (flags & INERT) !== 0 || this.skipped_effects.has(effect);
 
+			if ((effect.f & BOUNDARY_EFFECT) !== 0 && effect.b?.is_pending()) {
+				target = {
+					parent: target,
+					effect,
+					effects: [],
+					render_effects: [],
+					block_effects: []
+				};
+			}
+
 			if (!skip && effect.fn !== null) {
 				if (is_branch) {
 					effect.f ^= CLEAN;
 				} else if ((flags & EFFECT) !== 0) {
-					this.#effects.push(effect);
+					target.effects.push(effect);
 				} else if (async_mode_flag && (flags & RENDER_EFFECT) !== 0) {
-					this.#render_effects.push(effect);
+					target.render_effects.push(effect);
 				} else if (is_dirty(effect)) {
-					if ((effect.f & BLOCK_EFFECT) !== 0) this.#block_effects.push(effect);
+					if ((effect.f & BLOCK_EFFECT) !== 0) target.block_effects.push(effect);
 					update_effect(effect);
 				}
 
@@ -228,6 +236,17 @@ export class Batch {
 			effect = effect.next;
 
 			while (effect === null && parent !== null) {
+				if (parent === target.effect) {
+					// TODO rather than traversing into pending boundaries and deferring the effects,
+					// could we just attach the effects _to_ the pending boundary and schedule them
+					// once the boundary is ready?
+					this.#defer_effects(target.effects);
+					this.#defer_effects(target.render_effects);
+					this.#defer_effects(target.block_effects);
+
+					target = /** @type {EffectTarget} */ (target.parent);
+				}
+
 				effect = parent.next;
 				parent = parent.parent;
 			}
@@ -245,8 +264,6 @@ export class Batch {
 			// mark as clean so they get scheduled if they depend on pending async state
 			set_signal_status(e, CLEAN);
 		}
-
-		effects.length = 0;
 	}
 
 	/**
@@ -316,7 +333,17 @@ export class Batch {
 		if (batches.size > 1) {
 			this.#previous.clear();
 
-			let is_earlier = true;
+			var previous_batch_values = batch_values;
+			var is_earlier = true;
+
+			/** @type {EffectTarget} */
+			var dummy_target = {
+				parent: null,
+				effect: null,
+				effects: [],
+				render_effects: [],
+				block_effects: []
+			};
 
 			for (const batch of batches) {
 				if (batch === this) {
@@ -358,27 +385,42 @@ export class Batch {
 						batch.apply();
 
 						for (const root of queued_root_effects) {
-							batch.#traverse_effect_tree(root);
+							batch.#traverse_effect_tree(root, dummy_target);
 						}
 
+						// TODO do we need to do anything with `target`? defer block effects?
+
 						queued_root_effects = [];
 						batch.deactivate();
 					}
 				}
 			}
 
 			current_batch = null;
+			batch_values = previous_batch_values;
 		}
 
 		batches.delete(this);
+
+		this.#deferred?.resolve();
 	}
 
-	increment() {
+	/**
+	 *
+	 * @param {boolean} blocking
+	 */
+	increment(blocking) {
 		this.#pending += 1;
+		if (blocking) this.#blocking_pending += 1;
 	}
 
-	decrement() {
+	/**
+	 *
+	 * @param {boolean} blocking
+	 */
+	decrement(blocking) {
 		this.#pending -= 1;
+		if (blocking) this.#blocking_pending -= 1;
 
 		for (const e of this.#dirty_effects) {
 			set_signal_status(e, DIRTY);

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

@@ -136,17 +136,16 @@ export function async_derived(fn, location) {
 		if (DEV) current_async_effect = null;
 
 		var batch = /** @type {Batch} */ (current_batch);
-		var pending = boundary.is_pending();
 
 		if (should_suspend) {
+			var blocking = !boundary.is_pending();
+
 			boundary.update_pending_count(1);
-			if (!pending) {
-				batch.increment();
+			batch.increment(blocking);
 
-				deferreds.get(batch)?.reject(STALE_REACTION);
-				deferreds.delete(batch); // delete to ensure correct order in Map iteration below
-				deferreds.set(batch, d);
-			}
+			deferreds.get(batch)?.reject(STALE_REACTION);
+			deferreds.delete(batch); // delete to ensure correct order in Map iteration below
+			deferreds.set(batch, d);
 		}
 
 		/**
@@ -156,7 +155,7 @@ export function async_derived(fn, location) {
 		const handler = (value, error = undefined) => {
 			current_async_effect = null;
 
-			if (!pending) batch.activate();
+			batch.activate();
 
 			if (error) {
 				if (error !== STALE_REACTION) {
@@ -193,7 +192,7 @@ export function async_derived(fn, location) {
 
 			if (should_suspend) {
 				boundary.update_pending_count(-1);
-				if (!pending) batch.decrement();
+				batch.decrement(blocking);
 			}
 		};
 

packages/svelte/tests/runtime-runes/samples/async-abort-signal/_config.js

@@ -6,7 +6,7 @@ export default test({
 		const [reset, resolve] = target.querySelectorAll('button');
 
 		reset.click();
-		await settled();
+		await tick();
 		assert.deepEqual(logs, ['aborted']);
 
 		resolve.click();