fix memory leak, fix branch commit bug

Author: dummdidumm

.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/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
@@ -233,8 +229,20 @@ 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) {
-			for (const batch of superseeded_batches) {
-				batch.remove();
+			if (superseeded_batches.length > 0) {
+				const own = [...this.#callbacks.keys()].map((c) => c());
+				for (const batch of superseeded_batches) {
+					// 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.
+					for (const [effect, cb] of batch.#callbacks) {
+						if (!own.includes(effect())) {
+							cb();
+						}
+					}
+					batch.remove();
+				}
 			}
 
 			this.#commit();
@@ -394,12 +402,13 @@ export class Batch {
 		}
 	}
 
-	neuter() {
-		this.#neutered = true;
-	}
-
 	remove() {
-		this.neuter();
+		this.#callbacks.clear();
+		this.#maybe_dirty_effects =
+			this.#dirty_effects =
+			this.#boundary_async_effects =
+			this.#async_effects =
+				[];
 		batches.delete(this);
 	}
 
@@ -427,10 +436,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();
@@ -463,9 +470,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

@@ -184,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-redirect/_config.js

@@ -29,6 +29,7 @@ export default test({
 				<button>c</button>
 				<button>ok</button>
 				<p>c</p>
+				<p>b or c</p>
 			`
 		);
 
@@ -46,6 +47,7 @@ export default test({
 				<button>c</button>
 				<button>ok</button>
 				<p>b</p>
+				<p>b or c</p>
 			`
 		);
 	}

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

@@ -33,6 +33,10 @@
 		<p>c</p>
 	{/if}
 
+	{#if route === 'b' || route === 'c'}
+		<p>b or c</p>
+	{/if}
+
 	{#snippet pending()}
 		<p>pending...</p>
 	{/snippet}