fix: destroy each items after siblings are resumed

.changeset/flat-cars-say.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: correctly reconcile each blocks after outroing branches are resumed

.changeset/great-bikes-listen.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: destroy each items after siblings are resumed

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

@@ -1,4 +1,4 @@
-/** @import { EachItem, EachState, Effect, MaybeSource, Source, TemplateNode, TransitionManager, Value } from '#client' */
+/** @import { EachItem, EachOutroGroup, EachState, Effect, MaybeSource, Source, TemplateNode, TransitionManager, Value } from '#client' */
 /** @import { Batch } from '../../reactivity/batch.js'; */
 import {
 	EACH_INDEX_REACTIVE,
@@ -29,8 +29,6 @@ import {
 	block,
 	branch,
 	destroy_effect,
-	run_out_transitions,
-	pause_children,
 	pause_effect,
 	resume_effect
 } from '../../reactivity/effects.js';
@@ -75,19 +73,41 @@ function pause_effects(state, to_destroy, controlled_anchor) {
 	var transitions = [];
 	var length = to_destroy.length;
 
+	/** @type {EachOutroGroup} */
+	var group;
+	var remaining = to_destroy.length;
+
 	for (var i = 0; i < length; i++) {
-		pause_children(to_destroy[i].e, transitions, true);
+		pause_effect(
+			to_destroy[i].e,
+			() => {
+				if (group) {
+					group.remaining -= 1;
+
+					if (group.remaining === 0) {
+						var groups = /** @type {Set<EachOutroGroup>} */ (state.outrogroups);
+
+						destroy_items(state, Array.from(group.items));
+						groups.delete(group);
+
+						if (groups.size === 0) {
+							state.outrogroups = null;
+						}
+					}
+				} else {
+					remaining -= 1;
+				}
+			},
+			false
+		);
 	}
 
-	run_out_transitions(transitions, () => {
+	if (remaining === 0) {
 		// If we're in a controlled each block (i.e. the block is the only child of an
 		// element), and we are removing all items, _and_ there are no out transitions,
 		// we can use the fast path — emptying the element and replacing the anchor
 		var fast_path = transitions.length === 0 && controlled_anchor !== null;
 
-		// TODO only destroy effects if no pending batch needs them. otherwise,
-		// just set `item.o` back to `false`
-
 		if (fast_path) {
 			var anchor = /** @type {Element} */ (controlled_anchor);
 			var parent_node = /** @type {Element} */ (anchor.parentNode);
@@ -97,23 +117,41 @@ function pause_effects(state, to_destroy, controlled_anchor) {
 
 			state.items.clear();
 			link(state, to_destroy[0].prev, to_destroy[length - 1].next);
-		}
-
-		for (var i = 0; i < length; i++) {
-			var item = to_destroy[i];
 
-			if (!fast_path) {
-				state.items.delete(item.k);
-				link(state, item.prev, item.next);
+			for (i = 0; i < length; i++) {
+				destroy_effect(to_destroy[i].e);
 			}
 
-			destroy_effect(item.e, !fast_path);
+			return;
 		}
 
-		if (state.first === to_destroy[0]) {
-			state.first = to_destroy[0].prev;
-		}
-	});
+		destroy_items(state, to_destroy);
+	} else {
+		group = {
+			remaining,
+			items: new Set(to_destroy)
+		};
+
+		(state.outrogroups ??= new Set()).add(group);
+	}
+}
+
+/**
+ * Pause multiple effects simultaneously, and coordinate their
+ * subsequent destruction. Used in each blocks
+ * @param {EachState} state
+ * @param {EachItem[]} to_destroy
+ */
+function destroy_items(state, to_destroy) {
+	// TODO only destroy effects if no pending batch needs them. otherwise,
+	// just set `item.o` back to `false`
+	for (var i = 0; i < to_destroy.length; i++) {
+		var item = to_destroy[i];
+
+		state.items.delete(item.k);
+		link(state, item.prev, item.next);
+		destroy_effect(item.e);
+	}
 }
 
 /**
@@ -335,7 +373,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f
 	});
 
 	/** @type {EachState} */
-	var state = { effect, flags, items, first };
+	var state = { effect, flags, items, first, outrogroups: null };
 
 	first_run = false;
 
@@ -409,6 +447,15 @@ function reconcile(state, array, anchor, flags, get_key) {
 
 		item = /** @type {EachItem} */ (items.get(key));
 
+		if (state.outrogroups !== null) {
+			for (const group of state.outrogroups) {
+				if (group.items.has(item)) {
+					group.remaining -= 1;
+					group.items.delete(item);
+				}
+			}
+		}
+
 		state.first ??= item;
 
 		if (!item.o) {
@@ -486,11 +533,7 @@ function reconcile(state, array, anchor, flags, get_key) {
 			stashed = [];
 
 			while (current !== null && current.k !== key) {
-				// If the each block isn't inert and an item has an effect that is already inert,
-				// skip over adding it to our seen Set as the item is already being handled
-				if ((current.e.f & INERT) === 0) {
-					(seen ??= new Set()).add(current);
-				}
+				(seen ??= new Set()).add(current);
 				stashed.push(current);
 				current = current.next;
 			}
@@ -509,8 +552,29 @@ function reconcile(state, array, anchor, flags, get_key) {
 
 	let has_offscreen_items = items.size > length;
 
+	if (state.outrogroups !== null) {
+		for (const group of state.outrogroups) {
+			if (group.remaining === 0) {
+				destroy_items(state, Array.from(group.items));
+				state.outrogroups?.delete(group);
+			}
+		}
+
+		if (state.outrogroups.size === 0) {
+			state.outrogroups = null;
+		}
+	}
+
 	if (current !== null || seen !== undefined) {
-		var to_destroy = seen === undefined ? [] : array_from(seen);
+		var to_destroy = [];
+
+		if (seen !== undefined) {
+			for (item of seen) {
+				if ((item.e.f & INERT) === 0) {
+					to_destroy.push(item);
+				}
+			}
+		}
 
 		while (current !== null) {
 			// If the each block isn't inert, then inert effects are currently outroing and will be removed once the transition is finished

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

@@ -593,17 +593,11 @@ export function pause_effect(effect, callback, destroy = true) {
 
 	pause_children(effect, transitions, true);
 
-	run_out_transitions(transitions, () => {
+	var fn = () => {
 		if (destroy) destroy_effect(effect);
 		if (callback) callback();
-	});
-}
+	};
 
-/**
- * @param {TransitionManager[]} transitions
- * @param {() => void} fn
- */
-export function run_out_transitions(transitions, fn) {
 	var remaining = transitions.length;
 	if (remaining > 0) {
 		var check = () => --remaining || fn();
@@ -620,7 +614,7 @@ export function run_out_transitions(transitions, fn) {
  * @param {TransitionManager[]} transitions
  * @param {boolean} local
  */
-export function pause_children(effect, transitions, local) {
+function pause_children(effect, transitions, local) {
 	if ((effect.f & INERT) !== 0) return;
 	effect.f ^= INERT;
 

packages/svelte/src/internal/client/types.d.ts

@@ -72,6 +72,11 @@ export type TemplateNode = Text | Element | Comment;
 
 export type Dom = TemplateNode | TemplateNode[];
 
+export type EachOutroGroup = {
+	remaining: number;
+	items: Set<EachItem>;
+};
+
 export type EachState = {
 	/** the each block effect */
 	effect: Effect;
@@ -81,6 +86,8 @@ export type EachState = {
 	items: Map<any, EachItem>;
 	/** head of the linked list of items */
 	first: EachItem | null;
+	/** all outro groups that this item is a part of */
+	outrogroups: Set<EachOutroGroup> | null;
 };
 
 export type EachItem = {

packages/svelte/tests/runtime-runes/samples/each-updates-12/_config.js

@@ -0,0 +1,33 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target, raf }) {
+		const [clear, push] = target.querySelectorAll('button');
+
+		flushSync(() => clear.click());
+		flushSync(() => push.click());
+		raf.tick(500);
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>clear</button>
+				<button>push</button>
+				<span style="opacity: 1;">1</span>
+				<span style="opacity: 0.5;">2</span>
+			`
+		);
+
+		raf.tick(1000);
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>clear</button>
+				<button>push</button>
+				<span style="opacity: 1;">1</span>
+			`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/each-updates-12/main.svelte

@@ -0,0 +1,19 @@
+<script>
+	function fade(node) {
+		return {
+			duration: 1000,
+			tick(t) {
+				node.style.opacity = t;
+			}
+		}
+	}
+
+	let items = $state([1, 2]);
+</script>
+
+<button onclick={() => items = []}>clear</button>
+<button onclick={() => items.push(items.length + 1)}>push</button>
+
+{#each items as item}
+	<span transition:fade={{duration: 1000}}>{item}</span>
+{/each}

packages/svelte/tests/runtime-runes/samples/each-updates-13/_config.js

@@ -0,0 +1,23 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target, raf }) {
+		const [clear, reverse] = target.querySelectorAll('button');
+
+		flushSync(() => clear.click());
+		flushSync(() => reverse.click());
+		raf.tick(1);
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>clear</button>
+				<button>reverse</button>
+				<span style="opacity: 1;">c</span>
+				<span style="opacity: 1;">b</span>
+				<span style="opacity: 1;">a</span>
+			`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/each-updates-13/main.svelte

@@ -0,0 +1,19 @@
+<script>
+	function fade(node) {
+		return {
+			duration: 1000,
+			tick(t) {
+				node.style.opacity = t;
+			}
+		}
+	}
+
+	let items = $state(['a', 'b', 'c']);
+</script>
+
+<button onclick={() => items = []}>clear</button>
+<button onclick={() => items = ['c', 'b', 'a']}>reverse</button>
+
+{#each items as item (item)}
+	<span transition:fade={{duration: 1000}}>{item}</span>
+{/each}