fix: wrap each block expression in derived to encapsulte effects (#14967)

* fix: wrap each block expression in derived to encapsulte effects

* add test

* Update .changeset/tender-apples-scream.md

---------

Co-authored-by: Rich Harris <[email protected]>

Author: trueadm

.changeset/tender-apples-scream.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: wrap each block expression in derived to encapsulate effects

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

@@ -35,8 +35,9 @@ import { source, mutable_source, internal_set } from '../../reactivity/sources.j
 import { array_from, is_array } from '../../../shared/utils.js';
 import { INERT } from '../../constants.js';
 import { queue_micro_task } from '../task.js';
-import { active_effect, active_reaction } from '../../runtime.js';
+import { active_effect, active_reaction, get } from '../../runtime.js';
 import { DEV } from 'esm-env';
+import { derived_safe_equal } from '../../reactivity/deriveds.js';
 
 /**
  * The row of a keyed each block that is currently updating. We track this
@@ -135,15 +136,17 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f
 
 	var was_empty = false;
 
-	block(() => {
+	// TODO: ideally we could use derived for runes mode but because of the ability
+	// to use a store which can be mutated, we can't do that here as mutating a store
+	// will still result in the collection array being the same from the store
+	var each_array = derived_safe_equal(() => {
 		var collection = get_collection();
 
-		var array = is_array(collection)
-			? collection
-			: collection == null
-				? []
-				: array_from(collection);
+		return is_array(collection) ? collection : collection == null ? [] : array_from(collection);
+	});
 
+	block(() => {
+		var array = get(each_array);
 		var length = array.length;
 
 		if (was_empty && length === 0) {
@@ -254,7 +257,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f
 		// that a mutation occurred and it's made the collection MAYBE_DIRTY, so reading the
 		// collection again can provide consistency to the reactive graph again as the deriveds
 		// will now be `CLEAN`.
-		get_collection();
+		get(each_array);
 	});
 
 	if (hydrating) {

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

@@ -0,0 +1,16 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	async test({ assert, target, logs }) {
+		const [btn1] = target.querySelectorAll('button');
+
+		btn1.click();
+		flushSync();
+
+		await Promise.resolve();
+		await Promise.resolve();
+
+		assert.deepEqual(logs, ['cleanup']);
+	}
+});

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

@@ -0,0 +1,46 @@
+<script>
+	import { createSubscriber } from 'svelte/reactivity';
+
+	class MyStore {
+		#subscribe;
+		#data = $state([
+			['a', [1, 2]],
+			['b', [3, 4]]
+		]);
+		#id;
+
+		constructor(options) {
+			options?.someBoolean;
+			this.#id = options?.id;
+			this.#subscribe = createSubscriber(() => {
+				debugger
+				return () => {
+					console.log('cleanup');
+				};
+			});
+		}
+
+		get data() {
+			this.#subscribe();
+			return this.#data;
+		}
+		set data(v) {
+			this.#data = v;
+		}
+	}
+
+	let storeOptions = $state({
+		someBoolean: false,
+		id: 0
+	});
+
+	let myStore = $derived(new MyStore(storeOptions));
+</script>
+
+<button
+	onclick={() => {
+		storeOptions.someBoolean = !storeOptions.someBoolean;
+	}}>+</button
+>
+
+{#each myStore.data as _}{/each}