Infinite loop when binding to an item in a for loop

[kyngs]

Describe the bug

The page freezes when I attempt to iterate through a map containing a class and bind a freshly created element using data from that class into a field of that class. This appears to be related to #6921
Considering that one of my cores spikes up to 100%, there is probably some infinite loop going on.
I am a beginner, so I am sorry if I am making some rookie mistake

Reproduction

I have this example:

<script lang="ts">
    import "../app.css";
    import {NavbarRoute} from "../lib/NavbarRoute.ts";
    import {onMount} from "svelte";

    let beforeShop = new Map<string, NavbarRoute>;

    beforeShop.set("/", new NavbarRoute("Home"));
    beforeShop.set("/tos", new NavbarRoute("Pravidla a GDPR"));

    let afterShop = new Map<string, NavbarRoute>;

    afterShop.set("/team", new NavbarRoute("Tým"));
    afterShop.set("/hire", new NavbarRoute("Nábory"))
    afterShop.set("/contact", new NavbarRoute("Kontakt"));

    onMount(function () {
        let route;

        route = beforeShop.get(window.location.pathname);
        if (route == undefined) route = afterShop.get(window.location.pathname);

        if (route) {
            let div = route.div;

            div.classList.add("tab-active");
        }
    })
</script>

<header>
    <div class="bg-base-100 w-screen inline-flex items-center pt-2 pb-2">
        <div class="w-1/2 justify-start"></div>
        <div class="flex-shrink-0 inline-flex">
            <div>
                {#each [...beforeShop] as [path, route]}
                    <nav-el bind:this={route.div}>{route.name}</nav-el>
                {/each}
                <nav-el>Wiki</nav-el>
            </div>
            <div>
                <nav-el class="bg-primary text-base-100 rounded-box">Shop</nav-el>
            </div>
            <div>
                {#each [...afterShop] as [path, route]}
                    <nav-el bind:this={route.div}>{route.name}</nav-el>
                {/each}
            </div>
        </div>
        <div class="w-1/2 justify-end flex">
            <nav-el class="">Přihlásit se</nav-el>
        </div>
    </div>
</header>

NavbarRoute:

export class NavbarRoute {

    public div: any | undefined;

    constructor(public name: string) {
        this.div = undefined;
    }
}

By commenting out some parts of the code, I've found out, that it hangs due to: bind:this={route.div}

Logs

Nothing showed up in the server and browser logs.

System Info

System:
    OS: Linux 5.18 Arch Linux
    CPU: (24) x64 AMD Ryzen 9 5900X 12-Core Processor
    Memory: 44.10 GB / 62.71 GB
    Container: Yes
    Shell: 5.1.16 - /bin/bash
  Binaries:
    Node: 16.16.0 - /usr/bin/node
    Yarn: 1.22.19 - /usr/bin/yarn
    npm: 8.15.1 - /usr/bin/npm
  Browsers:
    Chromium: 104.0.5112.79
    Firefox: 103.0.1
  npmPackages:
    svelte: ^3.44.0 => 3.49.0

Severity

blocking all usage of svelte

[Prinzhorn]

Could you put this into a REPL (https://svelte.dev/repl/hello-world)? This is a great way to share small Svelte applications to demonstrate bugs like this. If it actually causes 100% CPU, maybe comment out the part you've identified as being the problem and give instructions in the REPL à la "Uncomment this part to cause 100% CPU".

Edit: fwiw, you entire onMount code can literally be:

<nav-el bind:this={route.div} class:tab-active={path === window.location.pathname}>{route.name}</nav-el>

Which would also later make it much easier to use a client side router. Because it would be reactive. Right now onMount happens once.
In Svelte it's extremely rare to manually touch the DOM like you do.

[kyngs]

I, unfortunately, cannot put it into a REPL, because of #6900 which doesn't allow to declare TypeScript classes inside a <script> block. Thanks for the idea of squashing it into one line, I probably won't need the bind:this={route.div} at all, but it would still be nice to figure it out.

EDIT: Your one-liner doesn't appear to work, because window is not defined.

[Prinzhorn]

I, unfortunately, cannot put it into a REPL, because of #6900 which doesn't allow to declare TypeScript classes inside a <script> block.

Does it not repro with JavaScript? I'm sure you can turn 10 lines of TypeScript into JavaScript?

EDIT: Your one-liner doesn't appear to work, because window is not defined.

Please provide a REPL if possible. Works for me https://svelte.dev/repl/f400f9f6692e4193b81a0c6e7b6f5244?version=3.49.0

[kyngs]

I, unfortunately, cannot put it into a REPL, because of #6900 which doesn't allow to declare TypeScript classes inside a <script> block.

Does it not repro with JavaScript? I'm sure you can turn 10 lines of TypeScript into JavaScript?

EDIT: Your one-liner doesn't appear to work, because window is not defined.

Please provide a REPL if possible. Works for me https://svelte.dev/repl/f400f9f6692e4193b81a0c6e7b6f5244?version=3.49.0

After some trial and error (I've never touched JavaScript, only TS) I've converted the original to this REPL:
https://svelte.dev/repl/303c5db2eaae4350a5ee6534e9daaf4a?version=3
You can see, that the moment you attempt to open it, your browser window freezes.

As far as your one-liner goes, you are right that it works in REPL: https://svelte.dev/repl/7c3ac2ac4e4546be8e9d3a10f6d8f768?version=3

However, it does not work in my SvelteKit application when I access it via npm run dev. It throws the following error:

window is not defined
ReferenceError: window is not defined
    at __layout.svelte:25:55
    at Module.each (/node_modules/svelte/internal/index.mjs:1736:16)
    at eval (/src/routes/__layout.svelte:26:82)
    at Object.$$render (/node_modules/svelte/internal/index.mjs:1770:22)
    at root.svelte:37:37
    at $$render (/node_modules/svelte/internal/index.mjs:1770:22)
    at Object.render (/node_modules/svelte/internal/index.mjs:1778:26)
    at render_response (file:///home/kyngs/Documents/Code/GitHub/ProjectZet-Plugins/Web-Frontend/.svelte-kit/runtime/server/index.js:1461:27)
    at async respond$1 (file:///home/kyngs/Documents/Code/GitHub/ProjectZet-Plugins/Web-Frontend/.svelte-kit/runtime/server/index.js:3098:4)
    at async resolve (file:///home/kyngs/Documents/Code/GitHub/ProjectZet-Plugins/Web-Frontend/.svelte-kit/runtime/server/index.js:3456:11)
window is not defined
ReferenceError: window is not defined
    at __layout.svelte:25:55
    at Module.each (/node_modules/svelte/internal/index.mjs:1736:16)
    at eval (/src/routes/__layout.svelte:26:82)
    at Object.$$render (/node_modules/svelte/internal/index.mjs:1770:22)
    at root.svelte:37:37
    at $$render (/node_modules/svelte/internal/index.mjs:1770:22)
    at Object.render (/node_modules/svelte/internal/index.mjs:1778:26)
    at render_response (file:///home/kyngs/Documents/Code/GitHub/ProjectZet-Plugins/Web-Frontend/.svelte-kit/runtime/server/index.js:1461:27)
    at async respond_with_error (file:///home/kyngs/Documents/Code/GitHub/ProjectZet-Plugins/Web-Frontend/.svelte-kit/runtime/server/index.js:2859:10)
    at async respond$1 (file:///home/kyngs/Documents/Code/GitHub/ProjectZet-Plugins/Web-Frontend/.svelte-kit/runtime/server/index.js:3115:4)
window is not defined
ReferenceError: window is not defined
    at __layout.svelte:25:55
    at Module.each (/node_modules/svelte/internal/index.mjs:1736:16)
    at eval (/src/routes/__layout.svelte:26:82)
    at Object.$$render (/node_modules/svelte/internal/index.mjs:1770:22)
    at root.svelte:37:37window is not defined
ReferenceError: window is not defined
    at __layout.svelte:25:55
    at Module.each (/node_modules/svelte/internal/index.mjs:1736:16)
    at eval (/src/routes/__layout.svelte:26:82)
    at Object.$$render (/node_modules/svelte/internal/index.mjs:1770:22)
    at root.svelte:37:37
    at $$render (/node_modules/svelte/internal/index.mjs:1770:22)
    at Object.render (/node_modules/svelte/internal/index.mjs:1778:26)
    at render_response (file:///home/kyngs/Documents/Code/GitHub/ProjectZet-Plugins/Web-Frontend/.svelte-kit/runtime/server/index.js:1461:27)
    at async respond$1 (file:///home/kyngs/Documents/Code/GitHub/ProjectZet-Plugins/Web-Frontend/.svelte-kit/runtime/server/index.js:3098:4)
    at async resolve (file:///home/kyngs/Documents/Code/GitHub/ProjectZet-Plugins/Web-Frontend/.svelte-kit/runtime/server/index.js:3456:11)
window is not defined
ReferenceError: window is not defined
    at __layout.svelte:25:55
    at Module.each (/node_modules/svelte/internal/index.mjs:1736:16)
    at eval (/src/routes/__layout.svelte:26:82)
    at Object.$$render (/node_modules/svelte/internal/index.mjs:1770:22)
    at root.svelte:37:37
    at $$render (/node_modules/svelte/internal/index.mjs:1770:22)
    at Object.render (/node_modules/svelte/internal/index.mjs:1778:26)
    at render_response (file:///home/kyngs/Documents/Code/GitHub/ProjectZet-Plugins/Web-Frontend/.svelte-kit/runtime/server/index.js:1461:27)
    at async respond_with_error (file:///home/kyngs/Documents/Code/GitHub/ProjectZet-Plugins/Web-Frontend/.svelte-kit/runtime/server/index.js:2859:10)
    at async respond$1 (file:///home/kyngs/Documents/Code/GitHub/ProjectZet-Plugins/Web-Frontend/.svelte-kit/runtime/server/index.js:3115:4)
window is not defined
ReferenceError: window is not defined
    at __layout.svelte:25:55
    at Module.each (/node_modules/svelte/internal/index.mjs:1736:16)
    at eval (/src/routes/__layout.svelte:26:82)
    at Object.$$render (/node_modules/svelte/internal/index.mjs:1770:22)
    at root.svelte:37:37
    at $$render (/node_modules/svelte/internal/index.mjs:1770:22)
    at Object.render (/node_modules/svelte/internal/index.mjs:1778:26)
    at render_response (file:///home/kyngs/Documents/Code/GitHub/ProjectZet-Plugins/Web-Frontend/.svelte-kit/runtime/server/index.js:1461:27)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async respond_with_error (file:///home/kyngs/Documents/Code/GitHub/ProjectZet-Plugins/Web-Frontend/.svelte-kit/runtime/server/index.js:2859:10)


    at $$render (/node_modules/svelte/internal/index.mjs:1770:22)
    at Object.render (/node_modules/svelte/internal/index.mjs:1778:26)
    at render_response (file:///home/kyngs/Documents/Code/GitHub/ProjectZet-Plugins/Web-Frontend/.svelte-kit/runtime/server/index.js:1461:27)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async respond_with_error (file:///home/kyngs/Documents/Code/GitHub/ProjectZet-Plugins/Web-Frontend/.svelte-kit/runtime/server/index.js:2859:10)

[Prinzhorn]

After some trial and error (I've never touched JavaScript, only TS) I've converted the original to this REPL:
https://svelte.dev/repl/303c5db2eaae4350a5ee6534e9daaf4a?version=3
You can see, that the moment you attempt to open it, your browser window freezes.

Thanks! I'm sure someone can use that to further debug the issue.

However, it does not work in my SvelteKit application when I access it via npm run dev. It throws the following error:

You never mentioned SvelteKit, window is not defined on the server, so this naturally cannot work.

[kyngs]

After some trial and error (I've never touched JavaScript, only TS) I've converted the original to this REPL:
https://svelte.dev/repl/303c5db2eaae4350a5ee6534e9daaf4a?version=3
You can see, that the moment you attempt to open it, your browser window freezes.

Thanks! I'm sure someone can use that do further debug the issue.

However, it does not work in my SvelteKit application when I access it via npm run dev. It throws the following error:

You never mentioned SvelteKit, window is not defined on the server, so this naturally cannot work.

Sorry, I thought that it was unrelated to the original issue, do you have any idea how to transform this one-liner to sveltekit?

[Prinzhorn]

Sorry, I thought that it was unrelated to the original issue, do you have any idea how to transform this one-liner to sveltekit?

SvelteKit offers a whole host of information for this precise purpose. E.g. https://kit.svelte.dev/docs/modules#$app-stores-page allows you to access the current URL on both server and client.

If you follow the Getting Started https://kit.svelte.dev/docs/introduction#getting-started you can see that in the demo app:

	<nav>
		<svg viewBox="0 0 2 3" aria-hidden="true">
			<path d="M0,0 L1,2 C1.5,3 1.5,3 2,3 L2,0 Z" />
		</svg>
		<ul>
			<li class:active={$page.url.pathname === '/'}><a sveltekit:prefetch href="/">Home</a></li>
			<li class:active={$page.url.pathname === '/about'}>
				<a sveltekit:prefetch href="/about">About</a>
			</li>
			<li class:active={$page.url.pathname === '/todos'}>
				<a sveltekit:prefetch href="/todos">Todos</a>
			</li>
		</ul>
		<svg viewBox="0 0 2 3" aria-hidden="true">
			<path d="M0,0 L0,3 C0.5,3 0.5,3 1,2 L2,0 Z" />
		</svg>
	</nav>

https://github.com/sveltejs/kit/blob/5cd8385391e30ecf525d11d24c268d3b08284dfb/packages/create-svelte/templates/default/src/lib/header/Header.svelte

Edit: I recommend checking out the demo app before working with an empty project.

[Prinzhorn]

I was able to track this down to the use of the spread operator. Or more specifically cloning the array, slice works too. Here's the absolute minimal REPL

https://svelte.dev/repl/d8f6f4532ab04a1fa028d1835d55839e?version=3.49.0

The first two each cause infinite loops, the third does not.

[Prinzhorn]

Here's the only difference between the generated Svelte code. This would be the time someone who knows about Svelte internals should take over.

--- good.js	2022-08-05 16:16:24.071516677 +0200
+++ bad.js	2022-08-05 16:15:57.823234520 +0200
@@ -20,7 +20,7 @@
   return child_ctx;
 }
 
-// (19:0) {#each bindings as binding}
+// (7:0) {#each [...bindings] as binding}
 function create_each_block(ctx) {
   let div;
   let each_value = /*each_value*/ ctx[3];
@@ -56,7 +56,7 @@
 
 function create_fragment(ctx) {
   let each_1_anchor;
-  let each_value = /*bindings*/ ctx[0];
+  let each_value = [.../*bindings*/ ctx[0]];
   let each_blocks = [];
 
   for (let i = 0; i < each_value.length; i += 1) {
@@ -80,7 +80,7 @@
     },
     p(ctx, [dirty]) {
       if (dirty & /*bindings*/ 1) {
-        each_value = /*bindings*/ ctx[0];
+        each_value = [.../*bindings*/ ctx[0]];
         let i;
 
         for (i = 0; i < each_value.length; i += 1) {

[Prinzhorn]

I think this could potentially be yet another version of #5689. Where the first iteration will correctly invalidate the entire bindings array, causing another each loop. However, there is no reason for Svelte to invalidate it again when you bind the same object to the same div. But due to #5689 the mere existence of bind:this={binding.div} causes the entire binding object and in turn the bindings array to be invalidated. I assume without copying the array Svelte already has some safeguard to prevent this from causing an endless loop by comparing references of the iterable.

[kyngs]

I think this could potentially be yet another version of #5689. Where the first iteration will correctly invalidate the entire bindings array, causing another each loop. However, there is no reason for Svelte to invalidate it again when you bind the same object to the same div. But due to #5689 the mere existence of bind:this={binding.div} causes the entire binding object and in turn the bindings array to be invalidated. I assume without copying the array Svelte already has some safeguard to prevent this from causing an endless loop by comparing references of the iterable.

However, if I understand correctly, #5689 is a duplicate to #4447, which should be fixed.

[Prinzhorn]

However, if I understand correctly, #5689 is a duplicate to #4447, which should be fixed.

I've been triaging Svelte issues for well over a year now and seen every single issue and this comes up regularly, it is definitely not fixed (at least not every variation of it). I even ran into it myself over year ago #6298

To make this very clear, here is a REPL https://svelte.dev/repl/5f436e05809346a38baf906184321761?version=3.49.0
It logs obj twice, for no reason.

[dummdidumm]

Looked at this a bit closer and the cause for the loop is:

1. loop starts with a fresh array
2. bind this is called, which invalidates the bindings array because the div is added to it
3. loop is iterated another time because of this
4. check if the bind this logic should be called again is done by looking if the array instance has changed, which it has due to the spread/slice
5. goto 2

I'm not sure how this can be solved in a way that breaks the loop and at the same time plays it safe in other use cases.