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

.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

@@ -1,10 +1,5 @@
 /** @import { Effect, Source, TemplateNode, } from '#client' */
-import {
-	BOUNDARY_EFFECT,
-	EFFECT_PRESERVED,
-	EFFECT_RAN,
-	EFFECT_TRANSPARENT
-} from '#client/constants';
+import { BOUNDARY_EFFECT, EFFECT_PRESERVED, EFFECT_TRANSPARENT } from '#client/constants';
 import { component_context, set_component_context } from '../../context.js';
 import { handle_error, invoke_error_boundary } from '../../error-handling.js';
 import { block, branch, destroy_effect, pause_effect } from '../../reactivity/effects.js';

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

@@ -10,7 +10,6 @@ import {
 	INERT,
 	RENDER_EFFECT,
 	ROOT_EFFECT,
-	USER_EFFECT,
 	MAYBE_DIRTY
 } from '#client/constants';
 import { async_mode_flag } from '../../flags/index.js';
@@ -95,10 +94,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 +113,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 +179,14 @@ export class Batch {
 		/** @type {Map<Source, { v: unknown, wv: number }> | null} */
 		var current_values = null;
 
+		/**
+		 * A batch is superseded 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 superseded_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 +199,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 superseded = is_prior_batch;
 
 				for (const [source, previous] of batch.#previous) {
 					if (!current_values.has(source)) {
+						superseded = false;
 						current_values.set(source, { v: source.v, wv: source.wv });
 						source.v = previous;
 					}
 				}
+
+				if (superseded) superseded_batches.push(batch);
 			}
 		}
 
@@ -215,6 +228,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 (superseded_batches.length > 0) {
+				const own = [...this.#callbacks.keys()].map((c) => c());
+				// A superseded 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 superseded_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 +403,17 @@ export class Batch {
 		}
 	}
 
-	neuter() {
-		this.#neutered = true;
+	remove() {
+		// Cleanup to
+		// - prevent memory leaks which could happen if a batch is tied to a never-ending promise
+		// - prevent effects from rerunning for outdated-and-now-no-longer-pending batches
+		this.#callbacks.clear();
+		this.#maybe_dirty_effects =
+			this.#dirty_effects =
+			this.#boundary_async_effects =
+			this.#async_effects =
+				[];
+		batches.delete(this);
 	}
 
 	flush() {
@@ -400,10 +440,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 +474,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() {

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

@@ -185,12 +185,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-redirect-2/_config.js

@@ -0,0 +1,36 @@
+import { tick } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, logs, target }) {
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<h1>a</h1>
+				<button>a</button>
+				<button>b</button>
+				<button>c</button>
+				<p>a</p>
+			`
+		);
+
+		const [a, b] = target.querySelectorAll('button');
+
+		b.click();
+		await tick();
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<h1>c</h1>
+				<button>a</button>
+				<button>b</button>
+				<button>c</button>
+				<p>c</p>
+				<p>b or c</p>
+			`
+		);
+
+		assert.deepEqual(logs, ['route a', 'route c']);
+	}
+});

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

@@ -0,0 +1,41 @@
+<script lang=ts>
+	let route = $state('a');
+
+	function goto(r) {
+		return Promise.resolve().then(async () => {
+			route = r;
+			await Promise.resolve();
+		});
+	}
+
+	$effect(() => {
+		console.log('route ' + route);
+	});
+</script>
+
+<h1>{route}</h1>
+<button onclick={() => route = 'a'}>a</button>
+<button onclick={() => route = 'b'}>b</button>
+<button onclick={() => route = 'c'}>c</button>
+
+<svelte:boundary>
+	{#if route === 'a'}
+		<p>a</p>
+	{/if}
+
+	{#if route === 'b'} 
+		{await goto('c')}
+	{/if}
+
+	{#if route === 'c'}
+		<p>c</p>
+	{/if}
+
+	{#if route === 'b' || route === 'c'}
+		<p>b or c</p>
+	{/if}
+
+	{#snippet pending()}
+		<p>pending...</p>
+	{/snippet}
+</svelte:boundary>