fix: each block breaking with effects interspersed among items (#17550)

* fix: each block breaking with effects interspersed among items

* test sample with interspersed non-branch effects

* Apply suggestion from @Rich-Harris

* use $effect.pre in sample instead

---------

Co-authored-by: Rich Harris <hello@rich-harris.dev>

.changeset/thirty-carpets-attack.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: each block breaking with effects interspersed among items

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

@@ -34,7 +34,7 @@ import {
 } from '../../reactivity/effects.js';
 import { source, mutable_source, internal_set } from '../../reactivity/sources.js';
 import { array_from, is_array } from '../../../shared/utils.js';
-import { COMMENT_NODE, EFFECT_OFFSCREEN, INERT } from '#client/constants';
+import { BRANCH_EFFECT, COMMENT_NODE, EFFECT_OFFSCREEN, INERT } from '#client/constants';
 import { queue_micro_task } from '../task.js';
 import { get } from '../../runtime.js';
 import { DEV } from 'esm-env';
@@ -336,6 +336,18 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f
 	}
 }
 
+/**
+ * Skip past any non-branch effects (which could be created with `createSubscriber`, for example) to find the next branch effect
+ * @param {Effect | null} effect
+ * @returns {Effect | null}
+ */
+function skip_to_branch(effect) {
+	while (effect !== null && (effect.f & BRANCH_EFFECT) === 0) {
+		effect = effect.next;
+	}
+	return effect;
+}
+
 /**
  * Add, remove, or reorder items output by an each block as its input changes
  * @template V
@@ -351,7 +363,7 @@ function reconcile(state, array, anchor, flags, get_key) {
 
 	var length = array.length;
 	var items = state.items;
-	var current = state.effect.first;
+	var current = skip_to_branch(state.effect.first);
 
 	/** @type {undefined | Set<Effect>} */
 	var seen;
@@ -431,7 +443,7 @@ function reconcile(state, array, anchor, flags, get_key) {
 				matched = [];
 				stashed = [];
 
-				current = prev.next;
+				current = skip_to_branch(prev.next);
 				continue;
 			}
 		}
@@ -495,7 +507,7 @@ function reconcile(state, array, anchor, flags, get_key) {
 			while (current !== null && current !== effect) {
 				(seen ??= new Set()).add(current);
 				stashed.push(current);
-				current = current.next;
+				current = skip_to_branch(current.next);
 			}
 
 			if (current === null) {
@@ -508,7 +520,7 @@ function reconcile(state, array, anchor, flags, get_key) {
 		}
 
 		prev = effect;
-		current = effect.next;
+		current = skip_to_branch(effect.next);
 	}
 
 	if (state.outrogroups !== null) {
@@ -542,7 +554,7 @@ function reconcile(state, array, anchor, flags, get_key) {
 				to_destroy.push(current);
 			}
 
-			current = current.next;
+			current = skip_to_branch(current.next);
 		}
 
 		var destroy_length = to_destroy.length;

packages/svelte/tests/runtime-runes/samples/each-non-branch-effects/_config.js

@@ -0,0 +1,43 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target }) {
+		const addBtn = /** @type {HTMLElement} */ (target.querySelector('button.add'));
+		const removeBtn = /** @type {HTMLElement} */ (target.querySelector('button.remove'));
+
+		const btnHtml = '<button class="add">add</button><button class="remove">remove</button>';
+
+		assert.htmlEqual(target.innerHTML, btnHtml);
+
+		addBtn.click();
+		flushSync();
+
+		assert.htmlEqual(target.innerHTML, `<span>1</span>${btnHtml}`);
+
+		addBtn.click();
+		flushSync();
+
+		assert.htmlEqual(target.innerHTML, `<span>1</span><span>2</span>${btnHtml}`);
+
+		addBtn.click();
+		flushSync();
+
+		assert.htmlEqual(target.innerHTML, `<span>1</span><span>2</span><span>3</span>${btnHtml}`);
+
+		removeBtn.click();
+		flushSync();
+
+		assert.htmlEqual(target.innerHTML, `<span>1</span><span>2</span>${btnHtml}`);
+
+		removeBtn.click();
+		flushSync();
+
+		assert.htmlEqual(target.innerHTML, `<span>1</span>${btnHtml}`);
+
+		addBtn.click();
+		flushSync();
+
+		assert.htmlEqual(target.innerHTML, `<span>1</span><span>2</span>${btnHtml}`);
+	}
+});

packages/svelte/tests/runtime-runes/samples/each-non-branch-effects/main.svelte

@@ -0,0 +1,30 @@
+<script>
+	let items = $state([]);
+
+	const proxy = new Proxy(items, {
+		get: (target, prop) => {
+			try {
+				$effect.pre(() => {
+					return () => {};
+				});
+			} catch {}
+
+			return Reflect.get(target, prop);
+		}
+	});
+
+	function add() {
+		items.push(items.length + 1);
+	}
+
+	function remove() {
+		items.pop();
+	}
+</script>
+
+{#each proxy as item}
+	<span>{item}</span>
+{/each}
+
+<button class="add" onclick={add}>add</button>
+<button class="remove" onclick={remove}>remove</button>