diff --git a/.changeset/brown-socks-wonder.md b/.changeset/brown-socks-wonder.md new file mode 100644 index 000000000000..2955865432d4 --- /dev/null +++ b/.changeset/brown-socks-wonder.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +Prevent mutating arrays in $effect to cause inifinite loops diff --git a/packages/svelte/src/internal/client/proxy.js b/packages/svelte/src/internal/client/proxy.js index 487050669933..086a97d90c50 100644 --- a/packages/svelte/src/internal/client/proxy.js +++ b/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 | 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} */ + const mutating_methods_cache = /** @type {Map} */ ( + 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); }, diff --git a/packages/svelte/tests/runtime-runes/samples/array-push-in-effect/_config.js b/packages/svelte/tests/runtime-runes/samples/array-push-in-effect/_config.js new file mode 100644 index 000000000000..04798a715e75 --- /dev/null +++ b/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, + ` + +

0

+ ` + ); + + // increment count; effect should append one new entry only + flushSync(() => button?.click()); + + assert.htmlEqual( + target.innerHTML, + ` + +

0

+

1

+ ` + ); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/array-push-in-effect/main.svelte b/packages/svelte/tests/runtime-runes/samples/array-push-in-effect/main.svelte new file mode 100644 index 000000000000..a99567b86f52 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/array-push-in-effect/main.svelte @@ -0,0 +1,17 @@ + + + +{#each log as item} +

{item}

+{/each} \ No newline at end of file