fix: prevent mutating arrays in $effect to cause inifinite loops

.changeset/brown-socks-wonder.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+Prevent mutating arrays in $effect to cause inifinite loops

packages/svelte/src/internal/client/proxy.js

@@ -1,15 +1,16 @@
 /** @import { Source } from '#client' */
 import { DEV } from 'esm-env';
-import { get, active_effect, active_reaction, set_active_reaction } from './runtime.js';
+import { get, active_effect, active_reaction, set_active_reaction, untrack } from './runtime.js';
 import {
 	array_prototype,
 	get_descriptor,
 	get_prototype_of,
 	is_array,
-	object_prototype
+	object_prototype,
+	define_property
 } from '../shared/utils.js';
 import { state as source, set } from './reactivity/sources.js';
-import { PROXY_PATH_SYMBOL, STATE_SYMBOL } from '#client/constants';
+import { PROXY_PATH_SYMBOL, STATE_SYMBOL, DERIVED, BLOCK_EFFECT } from '#client/constants';
 import { UNINITIALIZED } from '../../constants.js';
 import * as e from './errors.js';
 import { get_stack, tag } from './dev/tracing.js';
@@ -18,6 +19,19 @@ import { tracing_mode_flag } from '../flags/index.js';
 // TODO move all regexes into shared module?
 const regex_is_valid_identifier = /^[a-zA-Z_$][a-zA-Z_$0-9]*$/;
 
+// Array methods that mutate the array, according to the ECMAScript specification
+const MUTATING_ARRAY_METHODS = new Set([
+	'push',
+	'pop',
+	'shift',
+	'unshift',
+	'splice',
+	'sort',
+	'reverse',
+	'copyWithin',
+	'fill'
+]);
+
 /**
  * @template T
  * @param {T} value
@@ -43,6 +57,9 @@ export function proxy(value) {
 	var stack = DEV && tracing_mode_flag ? get_stack('CreatedAt') : null;
 	var reaction = active_reaction;
 
+	/** @type {Map<string, Function> | null} */
+	var proxied_array_mutating_methods_cache = is_proxied_array ? new Map() : null;
+
 	/**
 	 * @template T
 	 * @param {() => T} fn
@@ -176,6 +193,55 @@ export function proxy(value) {
 				return v === UNINITIALIZED ? undefined : v;
 			}
 
+			// if this is a proxied array and the property is a mutating method, return a
+			// wrapper that executes the native method inside `untrack`, preventing the
+			// current reaction from accidentally depending on `length` (or other
+			// internals) that the algorithm reads.
+			if (is_proxied_array && typeof prop === 'string' && MUTATING_ARRAY_METHODS.has(prop)) {
+				/** @type {Map<string, Function>} */
+				const mutating_methods_cache = /** @type {Map<string, Function>} */ (
+					proxied_array_mutating_methods_cache
+				);
+
+				var cached_method = mutating_methods_cache.get(prop);
+
+				if (cached_method === undefined) {
+					/**
+					 * wrapper executes the native mutating method inside `untrack` so that
+					 * any implicit `length` reads are ignored.
+					 * @this any
+					 * @param {...any} args
+					 */
+					cached_method = function (...args) {
+						// preserve correct `this` binding and forward result.
+						const fn = /** @type {any} */ (array_prototype)[prop];
+
+						// if we are inside a template expression/derived or block effect,
+						// keep tracking so the runtime can still detect unsafe mutations.
+						if (active_reaction !== null && (active_reaction.f & (DERIVED | BLOCK_EFFECT)) !== 0) {
+							return fn.apply(this, args);
+						}
+
+						// otherwise (ordinary user effect etc.) suppress dependency tracking
+						// so we avoid the self-invalidating loop.
+						return untrack(() => fn.apply(this, args));
+					};
+
+					// give the wrapper a meaningful name for better debugging
+					try {
+						define_property(cached_method, 'name', {
+							value: `proxied_array_untracked_${/** @type {string} */ (prop)}`
+						});
+					} catch {
+						// property might be non-configurable in some engines
+					}
+
+					mutating_methods_cache.set(prop, cached_method);
+				}
+
+				return cached_method;
+			}
+
 			return Reflect.get(target, prop, receiver);
 		},
 

packages/svelte/tests/runtime-runes/samples/array-push-in-effect/_config.js

@@ -0,0 +1,33 @@
+import { flushSync } from 'svelte';
+import { test } from '../../test';
+
+export default test({
+	/**
+	 * Ensure that mutating an array with push inside an $effect
+	 * does not cause an infinite re-execution loop.
+	 */
+	test({ assert, target }) {
+		const button = target.querySelector('button');
+
+		// initial render — effect should have run once
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>inc</button>
+				<p>0</p>
+			`
+		);
+
+		// increment count; effect should append one new entry only
+		flushSync(() => button?.click());
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<button>inc</button>
+				<p>0</p>
+				<p>1</p>
+			`
+		);
+	}
+});

packages/svelte/tests/runtime-runes/samples/array-push-in-effect/main.svelte

@@ -0,0 +1,17 @@
+<script>
+	let count = $state(0);
+	let log = $state([]);
+
+	$effect(() => {
+		log.push(count);
+	});
+
+	function inc() {
+		count += 1;
+	}
+</script>
+
+<button on:click={inc}>inc</button>
+{#each log as item}
+	<p>{item}</p>
+{/each}