fix: `{#await await ...}` and async dependencies fixes (#18243)

This started out as an investigation to get rid of the runtime code for
`{#await ...}` that deactivated a batch prior to reading the promise
function. That can result in a new batch being created if the promise
invocation happens to write a source.

Through that I discovered two other bugs:
1. The way we handle `{#await await ...}` was flawed around
SSR/hydration: On the server it would await the expression (which it
should not; `{#await await ...}` is kind of a weird special case here)
and during hydration it did not produce matching nodes, leading to
hydration fails.
2. When reading a dependency after an await expression which we add to
the current reaction, we did not deduplicate those reads. That can lead
to duplicate dependencies, which in turn can lead to bugs when
`remove_reaction` later runs. Conside this: You have `deps = [count,
unrelated, count]`. Now you do `remove_reactions(deps, 1)`, i.e. "remove
all reactions after the first one". That means the "disconnect these
from each other" logic runs for `count`, too, because it's also in the
third position, but that is wrong because it is also in the first
position, i.e. the connection should be kept.

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>

.changeset/clean-ties-start.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: don't unset batch when calling `{#await ...}` promise

.changeset/fair-otters-notice.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: promise-ify `{#await await ...}` expressions on the server and correctly hydrate them on the client

.changeset/loud-phones-return.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: deduplicate dependencies that are added outside the init/update cycle

packages/svelte/src/compiler/phases/3-transform/client/visitors/AwaitBlock.js

@@ -71,14 +71,14 @@ export function AwaitBlock(node, context) {
 		'await'
 	);
 
-	if (node.metadata.expression.has_blockers()) {
+	if (node.metadata.expression.has_blockers() || node.metadata.expression.has_await) {
 		context.state.init.push(
 			b.stmt(
 				b.call(
 					'$.async',
 					context.state.node,
 					node.metadata.expression.blockers(),
-					b.array([]),
+					b.array([]), // {#await await ...} is special insofar that the await should not be waited on
 					b.arrow([context.state.node], b.block([stmt]))
 				)
 			)

packages/svelte/src/compiler/phases/3-transform/client/visitors/VariableDeclaration.js

@@ -386,13 +386,17 @@ export function VariableDeclaration(node, context) {
  * @param {Expression} value
  */
 function create_state_declarators(declarator, context, value) {
+	/**
+	 * @param {Expression} value
+	 * @param {string} name
+	 */
+	const mutable_source = (value, name) => {
+		const call = b.call('$.mutable_source', value, context.state.analysis.immutable && b.true);
+		return dev ? b.call('$.tag', call, b.literal(name)) : call;
+	};
+
 	if (declarator.id.type === 'Identifier') {
-		return [
-			b.declarator(
-				declarator.id,
-				b.call('$.mutable_source', value, context.state.analysis.immutable ? b.true : undefined)
-			)
-		];
+		return [b.declarator(declarator.id, mutable_source(value, declarator.id.name))];
 	}
 
 	const tmp = b.id(context.state.scope.generate('tmp'));
@@ -414,7 +418,7 @@ function create_state_declarators(declarator, context, value) {
 			return b.declarator(
 				path.node,
 				binding?.kind === 'state'
-					? b.call('$.mutable_source', value, context.state.analysis.immutable ? b.true : undefined)
+					? mutable_source(value, /** @type {Identifier} */ (path.node).name)
 					: value
 			);
 		})

packages/svelte/src/compiler/phases/3-transform/server/visitors/AwaitBlock.js

@@ -9,12 +9,19 @@ import { block_close, create_child_block } from './shared/utils.js';
  * @param {ComponentContext} context
  */
 export function AwaitBlock(node, context) {
+	let expression = /** @type {Expression} */ (context.visit(node.expression));
+	if (node.metadata.expression.has_await) {
+		// If this is an await expression, turn it into a IIFE so that the result is a promise.
+		// {#await await ...} is special insofar that the await should not be waited on.
+		expression = b.call(b.arrow([], expression, true));
+	}
+
 	/** @type {Statement} */
 	let statement = b.stmt(
 		b.call(
 			'$.await',
 			b.id('$$renderer'),
-			/** @type {Expression} */ (context.visit(node.expression)),
+			expression,
 			b.thunk(
 				node.pending ? /** @type {BlockStatement} */ (context.visit(node.pending)) : b.block([])
 			),

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

@@ -7,7 +7,8 @@ import {
 	hydrating,
 	skip_nodes,
 	set_hydrate_node,
-	set_hydrating
+	set_hydrating,
+	hydrate_node
 } from '../hydration.js';
 import { queue_micro_task } from '../task.js';
 import { HYDRATION_START_ELSE, UNINITIALIZED } from '../../../../constants.js';
@@ -15,6 +16,7 @@ import { is_runes } from '../../context.js';
 import { Batch, current_batch, flushSync, is_flushing_sync } from '../../reactivity/batch.js';
 import { BranchManager } from './branches.js';
 import { capture, unset_context } from '../../reactivity/async.js';
+import { DEV } from 'esm-env';
 
 const PENDING = 0;
 const THEN = 1;
@@ -42,16 +44,16 @@ export function await_block(node, get_input, pending_fn, then_fn, catch_fn) {
 	var value = runes ? source(v) : mutable_source(v, false, false);
 	var error = runes ? source(v) : mutable_source(v, false, false);
 
+	if (DEV) {
+		value.label = '{#await ...} value';
+		error.label = '{#await ...} error';
+	}
+
 	var branches = new BranchManager(node);
 
 	block(() => {
 		var batch = /** @type {Batch} */ (current_batch);
-
-		// we null out `current_batch` because otherwise `save(...)` will incorrectly restore it —
-		// the batch will already have been committed by the time it resolves
-		batch.deactivate();
 		var input = get_input();
-		batch.activate();
 
 		var destroyed = false;
 
@@ -79,15 +81,17 @@ export function await_block(node, get_input, pending_fn, then_fn, catch_fn) {
 				// We don't want to restore the previous batch here; {#await} blocks don't follow the async logic
 				// we have elsewhere, instead pending/resolve/fail states are each their own batch so to speak.
 				restore(false);
+				// ...but it might still be set here. That means a `save(...)` has restored it — but that batch will
+				// likely already have been committed by the time it resolves, and this resolve should be processed
+				// in a separate batch. We're not using batch.deactivate()/activate() above because get_input()
+				// could write to sources, which would then incorrectly create a new batch or could mess with
+				// async_derived expecting a current_batch to exist.
+				if (current_batch === batch) {
+					batch.deactivate();
+				}
 				// Make sure we have a batch, since the branch manager expects one to exist
 				Batch.ensure();
 
-				if (hydrating) {
-					// `restore()` could set `hydrating` to `true`, which we very much
-					// don't want — we want to restore everything _except_ this
-					set_hydrating(false);
-				}
-
 				try {
 					fn();
 				} finally {

packages/svelte/src/internal/client/runtime.js

@@ -560,9 +560,15 @@ export function get(signal) {
 					}
 				}
 			} else {
-				// we're adding a dependency outside the init/update cycle
-				// (i.e. after an `await`)
-				(active_reaction.deps ??= []).push(signal);
+				// We're adding a dependency outside the init/update cycle (i.e. after an `await`).
+				// We have to deduplicate deps/reactions in this case or remove_reactions could
+				// disconnect deps/reactions that are actually still in use (if skip_deps says
+				// "disconnect all after this index" and some of the signals are also present in
+				// list prior to the cutoff index, i.e. that should be kept).
+				active_reaction.deps ??= [];
+				if (!includes.call(active_reaction.deps, signal)) {
+					active_reaction.deps.push(signal);
+				}
 
 				var reactions = signal.reactions;
 

packages/svelte/tests/runtime-runes/samples/async-await-block-2/_config.js

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

packages/svelte/tests/runtime-runes/samples/async-await-block-2/main.svelte

@@ -0,0 +1,22 @@
+<script>
+	let count = $state(0);
+
+	const queue = [];
+
+	function push(v) {
+		if (v === 0) return v;
+
+		return new Promise((fulfil) => queue.push(() => fulfil(v)));
+	}
+
+	async function request(v) {
+		const result = $derived(await push(v));
+		return result + count;
+	}
+</script>
+
+<button onclick={() => count++}>increment</button>
+<button onclick={() => queue.shift()?.()}>resolve</button>
+{#await request(count) then result}{result}{/await}
+{#await await push(count) + count then result}{result}{/await}
+{#await await 1 then result}{result}{/await}

packages/svelte/tests/runtime-runes/samples/async-duplicate-dependencies/_config.js

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

packages/svelte/tests/runtime-runes/samples/async-duplicate-dependencies/main.svelte

@@ -0,0 +1,20 @@
+<script>
+	let count = $state(0);
+
+	const queue = [];
+
+	function push(v) {
+		if (v === 0) return v;
+
+		return new Promise((fulfil) => queue.push(() => fulfil(v)));
+	}
+
+	async function request(v) {
+		const result = $derived(await push(v));
+		return result + count; // read existing dependency `count` once more
+	}
+</script>
+
+<button onclick={() => count++}>increment</button>
+<button onclick={() => queue.shift()?.()}>resolve</button>
+{await request(count)}