fix: $effect.pending sends updates to incorrect boundary

Author: elliott-with-the-longest-name-on-github

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

@@ -1,15 +1,15 @@
 /** @import { TemplateNode, Value } from '#client' */
 import { flatten } from '../../reactivity/async.js';
 import { get } from '../../runtime.js';
-import { get_pending_boundary } from './boundary.js';
+import { get_boundary } from './boundary.js';
 
 /**
  * @param {TemplateNode} node
  * @param {Array<() => Promise<any>>} expressions
  * @param {(anchor: TemplateNode, ...deriveds: Value[]) => void} fn
  */
 export function async(node, expressions, fn) {
-	var boundary = get_pending_boundary();
+	var boundary = get_boundary();
 
 	boundary.update_pending_count(1);
 

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

@@ -49,11 +49,34 @@ export function boundary(node, props, children) {
 }
 
 export class Boundary {
-	pending = false;
-
 	/** @type {Boundary | null} */
 	parent;
 
+	/**
+	 * Whether this boundary is inside a boundary (including this one) that's showing a pending snippet.
+	 * @type {boolean}
+	 */
+	get pending() {
+		if (this.has_pending_snippet()) {
+			return this.#pending;
+		}
+
+		// intentionally not throwing here, as the answer to "am I in a pending snippet" is false when
+		// there's no pending snippet at all
+		return this.parent?.pending ?? false;
+	}
+
+	set pending(value) {
+		if (this.has_pending_snippet()) {
+			this.#pending = value;
+		} else if (this.parent) {
+			this.parent.pending = value;
+		} else if (value) {
+			e.await_outside_boundary();
+		}
+		// if we're trying to set it to `false` and yeeting that into the void, it's fine
+	}
+
 	/** @type {TemplateNode} */
 	#anchor;
 
@@ -81,7 +104,28 @@ export class Boundary {
 	/** @type {DocumentFragment | null} */
 	#offscreen_fragment = null;
 
+	/**
+	 * Whether this boundary is inside a boundary (including this one) that's showing a pending snippet.
+	 * Derived from {@link props.pending} and {@link cascading_pending_count}.
+	 */
+	#pending = false;
+
+	/**
+	 * The number of pending async deriveds/expressions within this boundary, not counting any parent or child boundaries.
+	 * This controls `$effect.pending` for this boundary.
+	 *
+	 * Don't ever set this directly; use {@link update_pending_count} instead.
+	 */
 	#pending_count = 0;
+
+	/**
+	 * Like {@link #pending_count}, but treats boundaries with no `pending` snippet as porous.
+	 * This controls the pending snippet for this boundary.
+	 *
+	 * Don't ever set this directly; use {@link update_pending_count} instead.
+	 */
+	#cascading_pending_count = 0;
+
 	#is_creating_fallback = false;
 
 	/**
@@ -149,7 +193,7 @@ export class Boundary {
 						return branch(() => this.#children(this.#anchor));
 					});
 
-					if (this.#pending_count > 0) {
+					if (this.#cascading_pending_count > 0) {
 						this.#show_pending_snippet();
 					} else {
 						pause_effect(/** @type {Effect} */ (this.#pending_effect), () => {
@@ -166,7 +210,7 @@ export class Boundary {
 					this.error(error);
 				}
 
-				if (this.#pending_count > 0) {
+				if (this.#cascading_pending_count > 0) {
 					this.#show_pending_snippet();
 				} else {
 					this.pending = false;
@@ -220,11 +264,11 @@ export class Boundary {
 		}
 	}
 
-	/** @param {1 | -1} d */
-	#update_pending_count(d) {
-		this.#pending_count += d;
+	/** @param {number} d */
+	#update_cascading_pending_count(d) {
+		this.#cascading_pending_count = Math.max(this.#cascading_pending_count + d, 0);
 
-		if (this.#pending_count === 0) {
+		if (this.#cascading_pending_count === 0) {
 			this.pending = false;
 
 			if (this.#pending_effect) {
@@ -240,12 +284,21 @@ export class Boundary {
 		}
 	}
 
-	/** @param {1 | -1} d */
-	update_pending_count(d) {
+	/**
+	 * @param {number} d
+	 * @param {boolean} safe
+	 */
+	update_pending_count(d, safe = false, first = true) {
+		if (first) {
+			this.#pending_count = Math.max(this.#pending_count + d, 0);
+		}
+
 		if (this.has_pending_snippet()) {
-			this.#update_pending_count(d);
+			this.#update_cascading_pending_count(d);
 		} else if (this.parent) {
-			this.parent.#update_pending_count(d);
+			this.parent.update_pending_count(d, safe, false);
+		} else if (this.parent === null && !safe) {
+			e.await_outside_boundary();
 		}
 
 		effect_pending_updates.add(this.#effect_pending_update);
@@ -300,22 +353,26 @@ export class Boundary {
 			// If the failure happened while flushing effects, current_batch can be null
 			Batch.ensure();
 
-			this.#pending_count = 0;
+			// this ensures we modify the cascading_pending_count of the correct parent
+			// by the number we're decreasing this boundary by
+			this.update_pending_count(-this.#pending_count, true);
 
 			if (this.#failed_effect !== null) {
 				pause_effect(this.#failed_effect, () => {
 					this.#failed_effect = null;
 				});
 			}
 
-			this.pending = true;
+			// we intentionally do not try to find the nearest pending boundary. If this boundary has one, we'll render it on reset
+			// but it would be really weird to show the parent's boundary on a child reset.
+			this.pending = this.has_pending_snippet();
 
 			this.#main_effect = this.#run(() => {
 				this.#is_creating_fallback = false;
 				return branch(() => this.#children(this.#anchor));
 			});
 
-			if (this.#pending_count > 0) {
+			if (this.#cascading_pending_count > 0) {
 				this.#show_pending_snippet();
 			} else {
 				this.pending = false;
@@ -384,13 +441,9 @@ function move_effect(effect, fragment) {
 	}
 }
 
-export function get_pending_boundary() {
+export function get_boundary() {
 	var boundary = /** @type {Effect} */ (active_effect).b;
 
-	while (boundary !== null && !boundary.has_pending_snippet()) {
-		boundary = boundary.parent;
-	}
-
 	if (boundary === null) {
 		e.await_outside_boundary();
 	}

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

@@ -3,7 +3,6 @@
 import { DESTROYED } from '#client/constants';
 import { DEV } from 'esm-env';
 import { component_context, is_runes, set_component_context } from '../context.js';
-import { get_pending_boundary } from '../dom/blocks/boundary.js';
 import { invoke_error_boundary } from '../error-handling.js';
 import {
 	active_effect,
@@ -39,7 +38,6 @@ export function flatten(sync, async, fn) {
 	var parent = /** @type {Effect} */ (active_effect);
 
 	var restore = capture();
-	var boundary = get_pending_boundary();
 
 	Promise.all(async.map((expression) => async_derived(expression)))
 		.then((result) => {
@@ -60,7 +58,7 @@ export function flatten(sync, async, fn) {
 			unset_context();
 		})
 		.catch((error) => {
-			boundary.error(error);
+			invoke_error_boundary(error, parent);
 		});
 }
 

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

@@ -10,12 +10,11 @@ import {
 	INERT,
 	RENDER_EFFECT,
 	ROOT_EFFECT,
-	USER_EFFECT,
 	MAYBE_DIRTY
 } from '#client/constants';
 import { async_mode_flag } from '../../flags/index.js';
 import { deferred, define_property } from '../../shared/utils.js';
-import { get_pending_boundary } from '../dom/blocks/boundary.js';
+import { get_boundary } from '../dom/blocks/boundary.js';
 import {
 	active_effect,
 	is_dirty,
@@ -668,7 +667,7 @@ export function schedule_effect(signal) {
 }
 
 export function suspend() {
-	var boundary = get_pending_boundary();
+	var boundary = get_boundary();
 	var batch = /** @type {Batch} */ (current_batch);
 	var pending = boundary.pending;
 

packages/svelte/tests/runtime-runes/samples/async-effect-pending-nested/_config.js

@@ -0,0 +1,95 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target }) {
+		const [increment, shift] = target.querySelectorAll('button');
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>increment</button>
+				<button>shift</button>
+				<p>loading...</p>
+			`
+		);
+
+		shift.click();
+		shift.click();
+		shift.click();
+
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>increment</button>
+				<button>shift</button>
+				<p>0</p>
+				<p>0</p>
+				<p>0</p>
+				<p>inner pending: 0</p>
+				<p>outer pending: 0</p>
+			`
+		);
+
+		increment.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>increment</button>
+				<button>shift</button>
+				<p>0</p>
+				<p>0</p>
+				<p>0</p>
+				<p>inner pending: 3</p>
+				<p>outer pending: 0</p>
+			`
+		);
+
+		shift.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>increment</button>
+				<button>shift</button>
+				<p>0</p>
+				<p>0</p>
+				<p>0</p>
+				<p>inner pending: 2</p>
+				<p>outer pending: 0</p>
+			`
+		);
+
+		shift.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>increment</button>
+				<button>shift</button>
+				<p>0</p>
+				<p>0</p>
+				<p>0</p>
+				<p>inner pending: 1</p>
+				<p>outer pending: 0</p>
+			`
+		);
+
+		shift.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>increment</button>
+				<button>shift</button>
+				<p>1</p>
+				<p>1</p>
+				<p>1</p>
+				<p>inner pending: 0</p>
+				<p>outer pending: 0</p>
+			`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-effect-pending-nested/main.svelte

@@ -0,0 +1,34 @@
+<script>
+	let value = $state(0);
+	let deferreds = [];
+
+	function push(value) {
+		const deferred = Promise.withResolvers();
+		deferreds.push({ value, deferred });
+		return deferred.promise;
+	}
+
+	function shift() {
+		const d = deferreds.shift();
+		d?.deferred.resolve(d.value);
+	}
+</script>
+
+<button onclick={() => value++}>increment</button>
+<button onclick={() => shift()}>shift</button>
+
+<svelte:boundary>
+	<svelte:boundary>
+		<p>{await push(value)}</p>
+		<p>{await push(value)}</p>
+		<p>{await push(value)}</p>
+		<p>inner pending: {$effect.pending()}</p>
+	</svelte:boundary>
+	<p>outer pending: {$effect.pending()}</p>
+
+	{#snippet pending()}
+		<p>loading...</p>
+	{/snippet}
+</svelte:boundary>
+
+