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

[raythurnvoid]

Fixes #16092

Analysis

The bug

1. The user puts log.push(x) inside an $effect.
2. Array.prototype.push first reads length and then writes length.
3. Svelte’s proxy makes “length” reactive, so
   • the read adds the effect as a dependency of length;
   • the write marks length dirty and re-queues the same effect.
4. The effect keeps re-scheduling itself ⇒ “Maximum update depth exceeded”.

The fix

proxy.js is where every $state object/array is wrapped with a Proxy.

1. Recognise mutating array methods – push, pop, splice, sort, …
   (constant MUTATING_ARRAY_METHODS).

2. When such a method is accessed (get trap):
   • Return a cached wrapper instead of the original function.
   • The wrapper does one of two things:
   a. Inside template/derived code (active_reaction has DERIVED or BLOCK_EFFECT)
   → call the array method directly so reads are still tracked and the
   compiler can flag illegal “state mutation in template” (state_unsafe_mutation).
   b. Everywhere else (ordinary effects, user code)
   → call the array method inside untrack(...) so internal reads
   (length, indices) are not registered as dependencies, breaking the
   self-invalidating loop.

Result

• $effect(() => log.push(x)) now runs once per real update and never loops.
• Mutating an array inside templates (items.sort()) still throws
state_unsafe_mutation, so existing safety checks/tests remain intact.

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• Prefix your PR title with feat:, fix:, chore:, or docs:.
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.
• If this PR changes code within packages/svelte/src, add a changeset (npx changeset).

Tests and linting

• Run the tests with pnpm test and lint the project with pnpm lint

[changeset-bot]

🦋 Changeset detected

Latest commit: ada9c02

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
svelte	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[svelte-docs-bot]

Preview: https://svelte-dev-git-preview-svelte-16161-svelte.vercel.app/

[github-actions]

Playground

pnpm add https://pkg.pr.new/svelte@16161

[7nik]

In the case of sort, won't it break the existing code like

// keep array sorted
$effect(() => array.sort(comparator));

This method should not cause infinite loops, REPL

[raythurnvoid]

Thanks @7nik for this test case, right we should do an exception only for oeprations that actually do add/remove items. however some are tricky like what about splice if you replace 3 items with other 3 items, I will add these cases to the tests and make them work ^^

[raythurnvoid]

I was thinking what about adding a write_version of the array to the sources? It would work the same way as length and it's increased everytime an item is directly assigned and passed the not equality check or the length has changed.

This will allow to not have to track each item and the length, instead the effects can rely on the write_version only.

However this is becoming a significant change, I will close this PR for now because i don't see a way out of this that doesn't involve trating arrays significantly differently than now.

After thinking a lot about this I realized that using push or any other mutating method on the array is effectively like doing +=, you are writing and reading at the same time from a variable so the infinite loop makes sense. It's counter intuitive that people (including myself until now) would consider push a write only operation like x = y but it's not.