fix: correct first processing of boundary with pending snippet

.changeset/old-taxis-relate.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: run effects created in the template after async work has resolved

packages/svelte/src/internal/client/dom/task.js

@@ -1,4 +1,5 @@
 import { run_all } from '../../shared/utils.js';
+import { is_flushing_sync } from '../reactivity/batch.js';
 
 // Fallback for when requestIdleCallback is not available
 const request_idle_callback =
@@ -24,11 +25,15 @@ function run_idle_tasks() {
 	run_all(tasks);
 }
 
+export function has_pending_tasks() {
+	return micro_tasks.length > 0 || idle_tasks.length > 0;
+}
+
 /**
  * @param {() => void} fn
  */
 export function queue_micro_task(fn) {
-	if (micro_tasks.length === 0) {
+	if (micro_tasks.length === 0 && !is_flushing_sync) {
 		queueMicrotask(run_micro_tasks);
 	}
 

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

@@ -25,7 +25,7 @@ import {
 	update_effect
 } from '../runtime.js';
 import * as e from '../errors.js';
-import { flush_tasks } from '../dom/task.js';
+import { flush_tasks, has_pending_tasks, queue_micro_task } from '../dom/task.js';
 import { DEV } from 'esm-env';
 import { invoke_error_boundary } from '../error-handling.js';
 import { old_values } from './sources.js';
@@ -56,27 +56,14 @@ export let batch_deriveds = null;
 /** @type {Set<() => void>} */
 export let effect_pending_updates = new Set();
 
-/** @type {Array<() => void>} */
-let tasks = [];
-
-function dequeue() {
-	const task = /** @type {() => void} */ (tasks.shift());
-
-	if (tasks.length > 0) {
-		queueMicrotask(dequeue);
-	}
-
-	task();
-}
-
 /** @type {Effect[]} */
 let queued_root_effects = [];
 
 /** @type {Effect | null} */
 let last_scheduled_effect = null;
 
 let is_flushing = false;
-let is_flushing_sync = false;
+export let is_flushing_sync = false;
 
 export class Batch {
 	/**
@@ -357,6 +344,48 @@ export class Batch {
 		current_batch = this;
 	}
 
+	/**
+	 * Check if the branch this effect is in is obsolete in a later batch.
+	 * That is, if the branch exists in this batch but will be destroyed in a later batch.
+	 * @param {Effect} effect
+	 */
+	branch_obsolete(effect) {
+		/** @type {Effect[]} */
+		let alive = [];
+		/** @type {Effect[]} */
+		let skipped = [];
+		/** @type {Effect | null} */
+		let current = effect;
+
+		while (current !== null) {
+			if ((current.f & (BRANCH_EFFECT | ROOT_EFFECT)) !== 0) {
+				alive.push(current);
+				if (this.skipped_effects.has(current)) {
+					skipped.push(...alive);
+					alive = [];
+				}
+			}
+			current = current.parent;
+		}
+
+		let check = false;
+		for (const b of batches) {
+			if (b === this) {
+				check = true;
+			} else if (check) {
+				if (
+					alive.some((branch) => b.skipped_effects.has(branch)) ||
+					// TODO do we even have to check skipped here? how would an async_derived run for a branch that was already skipped?
+					(skipped.length > 0 && !skipped.some((branch) => b.skipped_effects.has(branch)))
+				) {
+					return true;
+				}
+			}
+		}
+
+		return false;
+	}
+
 	deactivate() {
 		current_batch = null;
 		previous_batch = null;
@@ -379,7 +408,7 @@ export class Batch {
 	flush() {
 		if (queued_root_effects.length > 0) {
 			flush_effects();
-		} else {
+		} else if (this.#pending === 0) {
 			this.#commit();
 		}
 
@@ -417,20 +446,24 @@ export class Batch {
 		this.#pending -= 1;
 
 		if (this.#pending === 0) {
-			for (const e of this.#dirty_effects) {
-				set_signal_status(e, DIRTY);
-				schedule_effect(e);
-			}
+			Batch.enqueue(() => {
+				this.activate();
 
-			for (const e of this.#maybe_dirty_effects) {
-				set_signal_status(e, MAYBE_DIRTY);
-				schedule_effect(e);
-			}
+				for (const e of this.#dirty_effects) {
+					set_signal_status(e, DIRTY);
+					schedule_effect(e);
+				}
 
-			this.#render_effects = [];
-			this.#effects = [];
+				for (const e of this.#maybe_dirty_effects) {
+					set_signal_status(e, MAYBE_DIRTY);
+					schedule_effect(e);
+				}
+
+				this.#render_effects = [];
+				this.#effects = [];
 
-			this.flush();
+				this.flush();
+			});
 		} else {
 			this.deactivate();
 		}
@@ -467,11 +500,7 @@ export class Batch {
 
 	/** @param {() => void} task */
 	static enqueue(task) {
-		if (tasks.length === 0) {
-			queueMicrotask(dequeue);
-		}
-
-		tasks.unshift(task);
+		queue_micro_task(task);
 	}
 }
 
@@ -502,7 +531,7 @@ export function flushSync(fn) {
 		while (true) {
 			flush_tasks();
 
-			if (queued_root_effects.length === 0) {
+			if (queued_root_effects.length === 0 && !has_pending_tasks()) {
 				current_batch?.flush();
 
 				// we need to check again, in case we just updated an `$effect.pending()`
@@ -670,19 +699,24 @@ export function schedule_effect(signal) {
 export function suspend() {
 	var boundary = get_pending_boundary();
 	var batch = /** @type {Batch} */ (current_batch);
-	var pending = boundary.pending;
+	// In case the pending snippet is shown, we want to update the UI immediately
+	// and not have the batch be blocked on async work,
+	// since the async work is happening "hidden" behind the pending snippet.
+	var ignore_async = boundary.pending;
 
 	boundary.update_pending_count(1);
-	if (!pending) batch.increment();
+	if (!ignore_async) batch.increment();
 
 	return function unsuspend() {
 		boundary.update_pending_count(-1);
-
-		if (!pending) {
-			batch.activate();
-			batch.decrement();
+		batch.activate();
+		if (ignore_async) {
+			// Template could have created new effects (for example via attachments) which need to be flushed
+			Batch.enqueue(() => {
+				batch.flush();
+			});
 		} else {
-			batch.deactivate();
+			batch.decrement();
 		}
 
 		unset_context();

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

@@ -1,5 +1,4 @@
 /** @import { Derived, Effect, Source } from '#client' */
-/** @import { Batch } from './batch.js'; */
 import { DEV } from 'esm-env';
 import {
 	ERROR_VALUE,
@@ -33,7 +32,7 @@ import { tracing_mode_flag } from '../../flags/index.js';
 import { Boundary } from '../dom/blocks/boundary.js';
 import { component_context } from '../context.js';
 import { UNINITIALIZED } from '../../../constants.js';
-import { batch_deriveds, current_batch } from './batch.js';
+import { Batch, batch_deriveds, current_batch } from './batch.js';
 import { unset_context } from './async.js';
 
 /** @type {Effect | null} */
@@ -115,7 +114,7 @@ export function async_derived(fn, location) {
 	// only suspend in async deriveds created on initialisation
 	var should_suspend = !active_reaction;
 
-	async_effect(() => {
+	var effect = async_effect(() => {
 		if (DEV) current_async_effect = active_effect;
 
 		try {
@@ -135,11 +134,14 @@ export function async_derived(fn, location) {
 		prev = promise;
 
 		var batch = /** @type {Batch} */ (current_batch);
-		var pending = boundary.pending;
+		// In case the pending snippet is shown, we want to update the UI immediately
+		// and not have the batch be blocked on async work,
+		// since the async work is happening "hidden" behind the pending snippet.
+		var ignore_async = boundary.pending;
 
 		if (should_suspend) {
 			boundary.update_pending_count(1);
-			if (!pending) batch.increment();
+			if (!ignore_async) batch.increment();
 		}
 
 		/**
@@ -151,10 +153,10 @@ export function async_derived(fn, location) {
 
 			current_async_effect = null;
 
-			if (!pending) batch.activate();
+			batch.activate();
 
 			if (error) {
-				if (error !== STALE_REACTION) {
+				if (error !== STALE_REACTION && !batch.branch_obsolete(effect)) {
 					signal.f |= ERROR_VALUE;
 
 					// @ts-expect-error the error is the wrong type, but we don't care
@@ -181,7 +183,14 @@ export function async_derived(fn, location) {
 
 			if (should_suspend) {
 				boundary.update_pending_count(-1);
-				if (!pending) batch.decrement();
+				if (ignore_async) {
+					// Template could have created new effects (for example via attachments) which need to be flushed
+					Batch.enqueue(() => {
+						batch.flush();
+					});
+				} else {
+					batch.decrement();
+				}
 			}
 
 			unset_context();

packages/svelte/tests/runtime-runes/samples/async-attachment/Inner.svelte

@@ -0,0 +1,10 @@
+<script>
+	function renderContent(node) {
+		node.textContent = 'foo';
+	}
+
+	const test = await Promise.resolve('foo');
+</script>
+
+<p>{test}</p>
+<div {@attach renderContent}></div>

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

@@ -0,0 +1,18 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target }) {
+		await tick();
+		assert.htmlEqual(target.innerHTML, '<button>toggle</button> <p>foo</p><div>foo</div>');
+
+		const [toggle] = target.querySelectorAll('button');
+		toggle.click();
+		await tick();
+		assert.htmlEqual(target.innerHTML, '<button>toggle</button>');
+
+		toggle.click();
+		await tick();
+		assert.htmlEqual(target.innerHTML, '<button>toggle</button> <p>foo</p><div>foo</div>');
+	}
+});

packages/svelte/tests/runtime-runes/samples/async-attachment/main.svelte

@@ -0,0 +1,16 @@
+<script>
+	import Inner from './Inner.svelte';
+
+	let show = $state(true);
+</script>
+
+<svelte:boundary>
+	<button onclick={() => show = !show}>toggle</button>
+	{#if show}
+		<Inner />
+	{/if}
+
+	{#snippet pending()}
+		<p>pending</p>
+	{/snippet}
+</svelte:boundary>

packages/svelte/tests/runtime-runes/samples/async-block-reject-each-during-init/_config.js

@@ -8,9 +8,11 @@ export default test({
 		increment.click();
 		await tick();
 
+		reject.click();
 		reject.click();
 		await tick();
 
+		resolve.click();
 		resolve.click();
 		await tick();
 
@@ -22,6 +24,35 @@ export default test({
 				<button>reject</button>
 				<p>false</p>
 				<p>1</p>
+				<p>false</p>
+				<p>1</p>
+			`
+		);
+
+		increment.click();
+		await tick();
+
+		increment.click();
+		await tick();
+
+		reject.click();
+		reject.click();
+		await tick();
+
+		resolve.click();
+		resolve.click();
+		await tick();
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>increment</button>
+				<button>resolve</button>
+				<button>reject</button>
+				<p>false</p>
+				<p>3</p>
+				<p>false</p>
+				<p>3</p>
 			`
 		);
 	}