fix: cleanup old batches, run their still-current commits

.changeset/spicy-ears-join.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: associate batch with boundary

.changeset/tame-ears-invite.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: ensure obsolete batches are removed and its necessary dom changes committed

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

@@ -28,7 +28,7 @@ import { queue_micro_task } from '../task.js';
 import * as e from '../../errors.js';
 import * as w from '../../warnings.js';
 import { DEV } from 'esm-env';
-import { Batch, effect_pending_updates } from '../../reactivity/batch.js';
+import { Batch, current_batch, effect_pending_updates } from '../../reactivity/batch.js';
 import { internal_set, source } from '../../reactivity/sources.js';
 import { tag } from '../../dev/tracing.js';
 import { createSubscriber } from '../../../../reactivity/create-subscriber.js';
@@ -59,6 +59,18 @@ export class Boundary {
 	/** @type {Boundary | null} */
 	parent;
 
+	/**
+	 * The associated batch to this boundary while the boundary pending; set by the one interacting with the boundary when entering pending state.
+	 * Will be `null` once the boundary is no longer pending.
+	 *
+	 * Needed because `current_batch` isn't guaranteed to exist: E.g. when component A has top level await, then renders component B
+	 * which also has top level await, `current_batch` can be null when a flush from component A happens before
+	 * suspend() in component B is called. We hence save it on the boundary instead.
+	 *
+	 * @type {Batch | null}
+	 */
+	#batch = null;
+
 	/** @type {TemplateNode} */
 	#anchor;
 
@@ -188,6 +200,13 @@ export class Boundary {
 		return !!this.#props.pending;
 	}
 
+	get_batch() {
+		if (current_batch) {
+			this.#batch = current_batch;
+		}
+		return /** @type {Batch} */ (this.#batch);
+	}
+
 	/**
 	 * @param {() => Effect | null} fn
 	 */
@@ -231,6 +250,7 @@ export class Boundary {
 
 		if (this.#pending_count === 0) {
 			this.pending = false;
+			this.#batch = null;
 
 			if (this.#pending_effect) {
 				pause_effect(this.#pending_effect, () => {

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

@@ -187,7 +187,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f
 		}
 	}
 
-	block(() => {
+	var b = block(() => {
 		// store a reference to the effect so that we can update the start/end nodes in reconciliation
 		each_effect ??= /** @type {Effect} */ (active_effect);
 
@@ -310,7 +310,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f
 					}
 				}
 
-				batch.add_callback(commit);
+				batch.add_callback(() => b, commit);
 			} else {
 				commit();
 			}

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

@@ -124,7 +124,7 @@ export function if_block(node, fn, elseif = false) {
 			if (active) batch.skipped_effects.delete(active);
 			if (inactive) batch.skipped_effects.add(inactive);
 
-			batch.add_callback(commit);
+			batch.add_callback(() => b, commit);
 		} else {
 			commit();
 		}
@@ -135,7 +135,7 @@ export function if_block(node, fn, elseif = false) {
 		}
 	};
 
-	block(() => {
+	var b = block(() => {
 		has_branch = false;
 		fn(set_branch);
 		if (!has_branch) {

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

@@ -52,7 +52,7 @@ export function key(node, get_key, render_fn) {
 		effect = pending_effect;
 	}
 
-	block(() => {
+	var b = block(() => {
 		if (changed(key, (key = get_key()))) {
 			var target = anchor;
 
@@ -66,7 +66,7 @@ export function key(node, get_key, render_fn) {
 			pending_effect = branch(() => render_fn(target));
 
 			if (defer) {
-				/** @type {Batch} */ (current_batch).add_callback(commit);
+				/** @type {Batch} */ (current_batch).add_callback(() => b, commit);
 			} else {
 				commit();
 			}

packages/svelte/src/internal/client/dom/blocks/svelte-component.js

@@ -51,7 +51,7 @@ export function component(node, get_component, render_fn) {
 		pending_effect = null;
 	}
 
-	block(() => {
+	var b = block(() => {
 		if (component === (component = get_component())) return;
 
 		var defer = should_defer_append();
@@ -70,7 +70,7 @@ export function component(node, get_component, render_fn) {
 		}
 
 		if (defer) {
-			/** @type {Batch} */ (current_batch).add_callback(commit);
+			/** @type {Batch} */ (current_batch).add_callback(() => b, commit);
 		} else {
 			commit();
 		}

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

@@ -95,10 +95,12 @@ export class Batch {
 
 	/**
 	 * 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
-	 * @type {Set<() => void>}
+	 * and append new ones by calling the functions added inside (if/each/key/etc) blocks.
+	 * Key is a function that returns the block effect because #callbacks will be called before
+	 * the block effect reference exists, so we need to capture it in a closure.
+	 * @type {Map<() => Effect, () => void>}
 	 */
-	#callbacks = new Set();
+	#callbacks = new Map();
 
 	/**
 	 * The number of async effects that are currently in flight
@@ -112,12 +114,6 @@ export class Batch {
 	 */
 	#deferred = null;
 
-	/**
-	 * True if an async effect inside this batch resolved and
-	 * its parent branch was already deleted
-	 */
-	#neutered = false;
-
 	/**
 	 * Async effects (created inside `async_derived`) encountered during processing.
 	 * These run after the rest of the batch has updated, since they should
@@ -184,6 +180,14 @@ export class Batch {
 		/** @type {Map<Source, { v: unknown, wv: number }> | null} */
 		var current_values = null;
 
+		/**
+		 * A batch is superseeded if all of its sources are also in the current batch.
+		 * If the current batch commits, we don't need the old batch anymore.
+		 * This also prevents memory leaks since the old batch will never be committed.
+		 * @type {Batch[]}
+		 */
+		var superseeded_batches = [];
+
 		// if there are multiple batches, we are 'time travelling' —
 		// we need to undo the changes belonging to any batch
 		// other than the current one
@@ -196,15 +200,25 @@ export class Batch {
 				source.v = current;
 			}
 
+			let is_prior_batch = true;
+
 			for (const batch of batches) {
-				if (batch === this) continue;
+				if (batch === this) {
+					is_prior_batch = false;
+					continue;
+				}
+
+				let superseeded = is_prior_batch;
 
 				for (const [source, previous] of batch.#previous) {
 					if (!current_values.has(source)) {
+						superseeded = false;
 						current_values.set(source, { v: source.v, wv: source.wv });
 						source.v = previous;
 					}
 				}
+
+				if (superseeded) superseeded_batches.push(batch);
 			}
 		}
 
@@ -215,6 +229,24 @@ export class Batch {
 		// if we didn't start any new async work, and no async work
 		// is outstanding from a previous flush, commit
 		if (this.#async_effects.length === 0 && this.#pending === 0) {
+			if (superseeded_batches.length > 0) {
+				const own = [...this.#callbacks.keys()].map((c) => c());
+				// A superseeded batch could have callbacks for e.g. destroying if blocks
+				// that are not part of the current batch because it already happened in the prior one,
+				// and the corresponding block effect therefore returning early because nothing was changed from its
+				// point of view, therefore not adding a callback to the current batch, so we gotta call them here.
+				// We do it from newest to oldest to ensure the correct callback is applied.
+				for (const batch of superseeded_batches.reverse()) {
+					for (const [effect, cb] of batch.#callbacks) {
+						if (!own.includes(effect())) {
+							cb();
+							own.push(effect());
+						}
+					}
+					batch.remove();
+				}
+			}
+
 			this.#commit();
 
 			var render_effects = this.#render_effects;
@@ -372,8 +404,14 @@ export class Batch {
 		}
 	}
 
-	neuter() {
-		this.#neutered = true;
+	remove() {
+		this.#callbacks.clear();
+		this.#maybe_dirty_effects =
+			this.#dirty_effects =
+			this.#boundary_async_effects =
+			this.#async_effects =
+				[];
+		batches.delete(this);
 	}
 
 	flush() {
@@ -400,10 +438,8 @@ export class Batch {
 	 * Append and remove branches to/from the DOM
 	 */
 	#commit() {
-		if (!this.#neutered) {
-			for (const fn of this.#callbacks) {
-				fn();
-			}
+		for (const fn of this.#callbacks.values()) {
+			fn();
 		}
 
 		this.#callbacks.clear();
@@ -436,9 +472,12 @@ export class Batch {
 		}
 	}
 
-	/** @param {() => void} fn */
-	add_callback(fn) {
-		this.#callbacks.add(fn);
+	/**
+	 * @param {() => Effect} effect
+	 * @param {() => void} fn
+	 */
+	add_callback(effect, fn) {
+		this.#callbacks.set(effect, fn);
 	}
 
 	settled() {
@@ -664,7 +703,7 @@ export function schedule_effect(signal) {
 
 export function suspend() {
 	var boundary = get_pending_boundary();
-	var batch = /** @type {Batch} */ (current_batch);
+	var batch = boundary.get_batch();
 	var pending = boundary.pending;
 
 	boundary.update_pending_count(1);

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_deriveds } from './batch.js';
 import { unset_context } from './async.js';
 
 /** @type {Effect | null} */
@@ -131,7 +130,7 @@ export function async_derived(fn, location) {
 
 		prev = promise;
 
-		var batch = /** @type {Batch} */ (current_batch);
+		var batch = boundary.get_batch();
 		var pending = boundary.pending;
 
 		if (should_suspend) {
@@ -185,12 +184,6 @@ export function async_derived(fn, location) {
 		};
 
 		promise.then(handler, (e) => handler(null, e || 'unknown'));
-
-		if (batch) {
-			return () => {
-				queueMicrotask(() => batch.neuter());
-			};
-		}
 	});
 
 	if (DEV) {

packages/svelte/tests/runtime-runes/samples/async-nested-top-level/Bar.svelte

@@ -0,0 +1,7 @@
+<script lang="ts">
+	import { resolve } from './main.svelte';
+
+	const bar = await new Promise((r) => resolve.push(() => r('bar')));
+</script>
+
+<p>bar: {bar}</p>

packages/svelte/tests/runtime-runes/samples/async-nested-top-level/Foo.svelte

@@ -0,0 +1,10 @@
+<script lang="ts">
+	import { resolve } from './main.svelte';
+	import Bar from './Bar.svelte';
+
+	const foo = await new Promise((r) => resolve.push(() => r('foo')));
+</script>
+
+<p>foo: {foo}</p>
+
+<Bar/>

packages/svelte/tests/runtime-runes/samples/async-nested-top-level/_config.js

@@ -0,0 +1,42 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target }) {
+		const [show, resolve] = target.querySelectorAll('button');
+
+		show.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>show</button>
+				<button>resolve</button>
+				<p>pending...</p>
+			`
+		);
+
+		resolve.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>show</button>
+				<button>resolve</button>
+				<p>pending...</p>
+			`
+		);
+
+		resolve.click();
+		await tick();
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>show</button>
+				<button>resolve</button>
+				<p>foo: foo</p>
+				<p>bar: bar</p>
+			`
+		);
+	}
+});