fix: leave stale promises to wait for a later resolution, instead of rejecting (#18180)

This incorporates some of the fixes and insights from #18177, but gets
rid of the `skip` logic.

Instead, we differentiate between _stale_ and _obsolete_ promises. A
promise is stale if it has been overtaken by a subsequent update, and
was rejected with `STALE_REACTION`:

```ts
async function search(query: string) {
  return fetch(`/search?q=${query}`, { signal: getAbortSignal() }).then((r) => r.json());
}
```

In this case, if we start typing `pot`, and then finish typing `potato`,
the first promise will eventually resolve with the results for
`/search?q=potato`, instead of the batch entering a weird limbo/zombie
state.

A promise is obsolete if it belongs to a now-destroyed effect, meaning
that toggling `show` doesn't result in an accumulation of
never-resolving batches:

```svelte
{#if show}
  {await neverResolves()}
{/if}
```

Fixes part of https://github.com/sveltejs/kit/issues/15431

---------

Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>

.changeset/big-webs-sing.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: leave stale promises to wait for a later resolution, instead of rejecting

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

@@ -70,20 +70,23 @@ export function flatten(blockers, sync, async, fn) {
 		unset_context();
 	}
 
+	var decrement_pending = increment_pending();
+
 	// Fast path: blockers but no async expressions
 	if (async.length === 0) {
-		/** @type {Promise<any>} */ (blocker_promise).then(() => finish(sync.map(d)));
+		/** @type {Promise<any>} */ (blocker_promise)
+			.then(() => finish(sync.map(d)))
+			.finally(decrement_pending);
+
 		return;
 	}
 
-	var decrement_pending = increment_pending();
-
 	// Full path: has async expressions
 	function run() {
 		Promise.all(async.map((expression) => async_derived(expression)))
 			.then((result) => finish([...sync.map(d), ...result]))
 			.catch((error) => invoke_error_boundary(error, parent))
-			.finally(() => decrement_pending());
+			.finally(decrement_pending);
 	}
 
 	if (blocker_promise) {
@@ -325,7 +328,7 @@ export function run(thunks) {
 		// wait one more tick, so that template effects are
 		// guaranteed to run before `$effect(...)`
 		.then(() => Promise.resolve())
-		.finally(() => decrement_pending());
+		.finally(decrement_pending);
 
 	return blockers;
 }
@@ -349,8 +352,8 @@ export function increment_pending() {
 	boundary.update_pending_count(1, batch);
 	batch.increment(blocking, effect);
 
-	return (skip = false) => {
+	return () => {
 		boundary.update_pending_count(-1, batch);
-		batch.decrement(blocking, effect, skip);
+		batch.decrement(blocking, effect);
 	};
 }

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

@@ -110,6 +110,13 @@ export class Batch {
 	 */
 	previous = new Map();
 
+	/**
+	 * Async effects which this batch doesn't take into account anymore when calculating blockers,
+	 * as it has a value for it already.
+	 * @type {Set<Effect>}
+	 */
+	unblocked = new Set();
+
 	/**
 	 * When the batch is committed (and the DOM is updated), we need to remove old branches
 	 * and append new ones by calling the functions added inside (if/each/key/etc) blocks
@@ -200,6 +207,8 @@ export class Batch {
 	#is_blocked() {
 		for (const batch of this.#blockers) {
 			for (const effect of batch.#blocking_pending.keys()) {
+				if (this.unblocked.has(effect)) continue;
+
 				var skipped = false;
 				var e = effect;
 
@@ -647,9 +656,8 @@ export class Batch {
 	/**
 	 * @param {boolean} blocking
 	 * @param {Effect} effect
-	 * @param {boolean} skip - whether to skip updates (because this is triggered by a stale reaction)
 	 */
-	decrement(blocking, effect, skip) {
+	decrement(blocking, effect) {
 		this.#pending -= 1;
 
 		if (blocking) {
@@ -662,12 +670,15 @@ export class Batch {
 			}
 		}
 
-		if (this.#decrement_queued || skip) return;
+		if (this.#decrement_queued) return;
 		this.#decrement_queued = true;
 
 		queue_micro_task(() => {
 			this.#decrement_queued = false;
-			this.flush();
+
+			if (batches.has(this)) {
+				this.flush();
+			}
 		});
 	}
 

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

@@ -100,6 +100,8 @@ export function derived(fn) {
 	return signal;
 }
 
+const OBSOLETE = {};
+
 /**
  * @template V
  * @param {() => V | Promise<V>} fn
@@ -118,7 +120,7 @@ export function async_derived(fn, label, location) {
 	var promise = /** @type {Promise<V>} */ (/** @type {unknown} */ (undefined));
 	var signal = source(/** @type {V} */ (UNINITIALIZED));
 
-	if (DEV) signal.label = label;
+	if (DEV) signal.label = label ?? fn.toString();
 
 	// only suspend in async deriveds created on initialisation
 	var should_suspend = !active_reaction;
@@ -141,7 +143,13 @@ export function async_derived(fn, label, location) {
 			// If this code is changed at some point, make sure to still access the then property
 			// of fn() to read any signals it might access, so that we track them as dependencies.
 			// We call `unset_context` to undo any `save` calls that happen inside `fn()`
-			Promise.resolve(fn()).then(d.resolve, d.reject).finally(unset_context);
+			Promise.resolve(fn())
+				.then(d.resolve, (e) => {
+					// if the promise was rejected by the user, via `getAbortSignal`, then
+					// wait for a subsequent resolution instead of flushing the batch
+					if (e !== STALE_REACTION) d.reject(e);
+				})
+				.finally(unset_context);
 		} catch (error) {
 			d.reject(error);
 			unset_context();
@@ -180,15 +188,13 @@ export function async_derived(fn, label, location) {
 			}
 
 			if (/** @type {Boundary} */ (parent.b).is_rendered()) {
-				deferreds.get(batch)?.reject(STALE_REACTION);
+				deferreds.get(batch)?.reject(OBSOLETE);
-				deferreds.delete(batch); // delete to ensure correct order in Map iteration below
 			} else {
 				// While the boundary is still showing pending, a new run supersedes all older in-flight runs
 				// for this async expression. Cancel eagerly so resolution cannot commit stale values.
 				for (const d of deferreds.values()) {
-					d.reject(STALE_REACTION);
+					d.reject(OBSOLETE);
 				}
-				deferreds.clear();
 			}
 
 			deferreds.set(batch, d);
@@ -203,16 +209,10 @@ export function async_derived(fn, label, location) {
 				reactivity_loss_tracker = null;
 			}
 
-			if (decrement_pending) {
+			decrement_pending?.();
-				// don't trigger an update if we're only here because
+			deferreds.delete(batch);
-				// the promise was superseded before it could resolve
-				var skip = error === STALE_REACTION;
-				decrement_pending(skip);
-			}
 
-			if (error === STALE_REACTION || (effect.f & DESTROYED) !== 0) {
+			if (error === OBSOLETE) return;
-				return;
-			}
 
 			batch.activate();
 
@@ -230,9 +230,14 @@ export function async_derived(fn, label, location) {
 
 				// All prior async derived runs are now stale
 				for (const [b, d] of deferreds) {
-					deferreds.delete(b);
+					if (b.id < batch.id) {
-					if (b === batch) break;
+						// Don't delete + resolve directly, instead only do that once
-					d.resolve(value);
+						// the current batch commits. This way we avoid tearing when
+						// `b` is rendering through the early resolve while `batch` is
+						// still pending.
+						batch.unblocked.add(effect);
+						batch.oncommit(() => d.resolve(value));
+					}
 				}
 
 				if (DEV && location !== undefined) {
@@ -255,7 +260,7 @@ export function async_derived(fn, label, location) {
 
 	teardown(() => {
 		for (const d of deferreds.values()) {
-			d.reject(STALE_REACTION);
+			d.reject(OBSOLETE);
 		}
 	});
 

packages/svelte/tests/runtime-runes/samples/async-batch-order/_config.js

@@ -0,0 +1,30 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target }) {
+		await tick();
+		const [increment, shift, middle] = target.querySelectorAll('button');
+		const [div] = target.querySelectorAll('div');
+
+		increment.click();
+		await tick();
+		increment.click();
+		await tick();
+		increment.click();
+		await tick();
+		middle.click(); // resolve the second increment which will make the if block go away and the first batch discarded
+		await tick();
+		assert.htmlEqual(div.innerHTML, '2 2');
+
+		shift.click();
+		await tick();
+		shift.click();
+		await tick();
+		shift.click();
+		await tick();
+		shift.click();
+		await tick();
+		assert.htmlEqual(div.innerHTML, '3 3');
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-batch-order/main.svelte

@@ -0,0 +1,21 @@
+<script>
+	let a = $state(0);
+
+	const deferred = [];
+
+	function delay(value) {
+		if (!value) return value;
+		return new Promise((resolve) => deferred.push(() => resolve(value)));
+	}
+</script>
+
+<div>
+	{a} {await delay(a)}
+	{#if a < 2}
+		{await delay(a)}
+	{/if}
+</div>
+
+<button onclick={() => {a++;}}>a++</button>
+<button onclick={() => deferred.shift()?.()}>shift</button>
+<button onclick={() => deferred[2]()}>middle</button>

packages/svelte/tests/runtime-runes/samples/async-stale-derived-4/_config.js

@@ -0,0 +1,28 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target }) {
+		await tick();
+
+		const [increment, hide, pop] = target.querySelectorAll('button');
+
+		increment.click();
+		await tick();
+		pop.click();
+		await tick();
+		hide.click(); // hides the if block, which cancels the pending async inside, which means the batch can complete
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`<button>increment</button> <button>hide</button> <button>pop</button> 1`
+		);
+
+		pop.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`<button>increment</button> <button>hide</button> <button>pop</button> 1`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-stale-derived-4/main.svelte

@@ -0,0 +1,21 @@
+<script>
+	let show = $state(true);
+	let count = $state(0);
+
+	const queue = [];
+
+	function push(value) {
+		if (!value) return value;
+		return new Promise(r => queue.push(() => r(value)));
+	}
+</script>
+
+<button onclick={() => count += 1}>increment</button>
+<button onclick={() => show = false}>hide</button>
+<!-- pop() so that the outer one resolves first, not the one inside the if block -->
+<button onclick={() => queue.pop()?.()}>pop</button>
+
+{await push(count)}
+{#if show}
+	{await push(count)}
+{/if}

packages/svelte/tests/runtime-runes/samples/async-stale-derived-5/_config.js

@@ -0,0 +1,33 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target }) {
+		await tick();
+
+		const [increment, shift] = target.querySelectorAll('button');
+
+		increment.click();
+		await tick();
+		increment.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`<button>3</button><button>shift</button><p>1 = 1</p><p>fizz: true</p><p>buzz: true</p>`
+		);
+
+		shift.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`<button>3</button><button>shift</button><p>1 = 1</p><p>fizz: true</p><p>buzz: true</p>`
+		);
+
+		shift.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`<button>3</button><button>shift</button><p>3 = 3</p><p>fizz: true</p><p>buzz: false</p>`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-stale-derived-5/main.svelte

@@ -0,0 +1,44 @@
+<script>
+	import { getAbortSignal } from 'svelte';
+
+	const queue = [];
+
+	let n = $state(1);
+	let fizz = $state(true);
+	let buzz = $state(true);
+
+	function increment() {
+		n++;
+
+		fizz = n % 3 === 0;
+		buzz = n % 5 === 0;
+	}
+
+	function push(value) {
+		if (value === 1) return 1;
+		const d = Promise.withResolvers();
+
+		queue.push(() => d.resolve(value));
+
+		const signal = getAbortSignal();
+		signal.onabort = () => d.reject(signal.reason);
+
+		return d.promise;
+	}
+</script>
+
+<button onclick={increment}>
+	{$state.eager(n)}
+</button>
+
+<button onclick={() => queue.shift()?.()}>shift</button>
+
+<p>{n} = {await push(n)}</p>
+
+{#if true}
+	<p>fizz: {fizz}</p>
+{/if}
+
+{#if true}
+	<p>buzz: {buzz}</p>
+{/if}

packages/svelte/tests/runtime-runes/samples/async-stale-derived-6/_config.js

@@ -0,0 +1,34 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target }) {
+		await tick();
+		const [button1, button2, pop, shift] = target.querySelectorAll('button');
+		const [p] = target.querySelectorAll('p');
+
+		button1.click();
+		await tick();
+		button2.click();
+		await tick();
+		assert.htmlEqual(p.innerHTML, `0 + 0 = 0 | 0 0`);
+
+		shift.click();
+		await tick();
+		assert.htmlEqual(p.innerHTML, `0 + 0 = 0 | 0 0`);
+
+		pop.click();
+		await tick();
+		assert.htmlEqual(p.innerHTML, `0 + 0 = 0 | 0 0`);
+
+		pop.click();
+		await tick();
+		assert.htmlEqual(p.innerHTML, `1 + 0 = 1 | 1 0`);
+
+		shift.click();
+		await tick();
+		pop.click();
+		await tick();
+		assert.htmlEqual(p.innerHTML, `1 + 2 = 3 | 1 1`);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-stale-derived-6/main.svelte

@@ -0,0 +1,21 @@
+<script>
+	const queue1 = [];
+	const queue2 = [];
+
+	let a = $state(0);
+	let b = $state(0);
+	let c = $state(0);
+	let d = $state(0)
+
+	function push(value, where = 1) {
+		if (!value) return value;
+		return new Promise(r => (where === 1 ? queue1 : queue2).push(() => r(value)));
+	}
+</script>
+
+<button onclick={() => {a++;c++}}>a / c</button>
+<button onclick={() => {b+=2;d++}}>b / d</button>
+<button onclick={() => queue1.pop()?.()}>pop 1</button>
+<button onclick={() => queue2.shift()?.()}>shift 2</button>
+
+<p>{a} + {b} = {await push(a + b)} | {await push(c, 2)} {await push(d, 2)}</p>

packages/svelte/tests/runtime-runes/samples/async-stale-derived-7/_config.js

@@ -0,0 +1,34 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target }) {
+		await tick();
+		const [button1, button2, shift_1, pop_1, shift_2] = target.querySelectorAll('button');
+		const [p] = target.querySelectorAll('p');
+
+		button1.click();
+		await tick();
+		button2.click();
+		await tick();
+		assert.htmlEqual(p.innerHTML, `0 + 0 = 0 | 0 0`);
+
+		pop_1.click();
+		await tick();
+		shift_2.click();
+		await tick();
+		assert.htmlEqual(p.innerHTML, `0 + 0 = 0 | 0 0`);
+
+		// Check that the first batch can still resolve before the second even if one of its async values
+		// is already superseeded (but the subsequent batch as a whole is still pending).
+		shift_1.click();
+		await tick();
+		assert.htmlEqual(p.innerHTML, `1 + 0 = 1 | 1 0`);
+
+		shift_1.click();
+		await tick();
+		shift_2.click();
+		await tick();
+		assert.htmlEqual(p.innerHTML, `1 + 2 = 3 | 1 1`);
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-stale-derived-7/main.svelte

@@ -0,0 +1,22 @@
+<script>
+	const queue1 = [];
+	const queue2 = [];
+
+	let a = $state(0);
+	let b = $state(0);
+	let c = $state(0);
+	let d = $state(0)
+
+	function push(value, where = 1) {
+		if (!value) return value;
+		return new Promise(r => (where === 1 ? queue1 : queue2).push(() => r(value)));
+	}
+</script>
+
+<button onclick={() => {a++;c++}}>a / c</button>
+<button onclick={() => {b+=2;d++}}>b / d</button>
+<button onclick={() => queue1.shift()?.()}>shift 1</button>
+<button onclick={() => queue1.pop()?.()}>pop 1</button>
+<button onclick={() => queue2.shift()?.()}>shift 2</button>
+
+<p>{a} + {b} = {await push(a + b)} | {await push(c, 2)} {await push(d, 2)}</p>