svelte
svelte copied to clipboard
fix: infinite loop caused by having bind:this in an each loop
Fixes #4317
I did some digging and the infinite loop occurs when the array or iterable in the each-loop is a literal array, a (pure or impure, it depends) array from a regular function or generator function (iterable), containing objects or more arrays. The exact internal reasons on how the infinite loop came to be is ~confusing and hard to describe, even for me digging this 😁, so, uh, yeah...~
Here's my attempt to explain what's going on with the bug. I'll use this https://svelte.dev/repl/c0a3494b41f14ebd84a5f2f3f9747fa1?version=3.59.1, courtesy of the original bug report, as an example.
When the component is created and mounted, the binding functions are called (from the example, it is the div_binding
functions), which add callbacks to the binding_callbacks
list. After the component is mounted, the internal flush()
function is called. The callbacks in bindings_callbacks
are called, which invalidates the working_array
. When it invalidates the working_array
, it causes the component to update. When updating, we compare each array element with the previous array element, decide that they're different, which causes us to call the div_binding
functions again, adding callbacks to the binding_callbacks
list, again, and we're back to where we started, causing an infinite loop. (Thanks browser debugger!)
To get the infinite loop, at the stage where we compare the array elements, they are different. This could be caused by having a function returning an array of objects/arrays, or an impure function returning different things every time you call it, or a literal array of objects, or whatever. We comparing two objects (different in memory but same in what it contains), or two different values because the function is impure and we erroneously call the function that returns the array but the function is not invalidated. We don't need to do this, because we are not invalidating the actual array to be iterated, and requiring us to get the new array. Therefore, this PR adds a dirty
check around arrays, or if the array is literal, no need to get a new literal array because we already defined it already.
Hope this comment is clear enough!
HEADS UP: BIG RESTRUCTURING UNDERWAY
The Svelte repo is currently in the process of heavy restructuring for Svelte 4. After that, work on Svelte 5 will likely change a lot on the compiler aswell. For that reason, please don't open PRs that are large in scope, touch more than a couple of files etc. In other words, bug fixes are fine, but feature PRs will likely not be merged.
Before submitting the PR, please make sure you do the following
- [x] 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
- [x] Prefix your PR title with
feat:
,fix:
,chore:
, ordocs:
. - [x] This message body should clearly illustrate what problems it solves.
- [x] Ideally, include a test that fails without this PR but passes with it.
Tests
- [x] Run the tests with
pnpm test
and lint the project withpnpm lint
@ngtr6788 is attempting to deploy a commit to the Svelte Team on Vercel.
A member of the Team first needs to authorize it.
Thanks for adding support for XYZ. That was exactly what I needed!
Deployment failed with the following error:
There is no GitHub account connected to this Vercel account.
⚠️ No Changeset found
Latest commit: 4855c27ab29c45bcef616d0202bdcb6a3bca1adc
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
Will close this PR because of Svelte 5