sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
77.39k stars 4.03k forks source link

Svelte5: snippets components invoked multiple times #12187

Open fsoft72 opened 3 days ago

fsoft72 commented 3 days ago

Describe the bug

Look at the following code:

{#snippet comp(component, name)}
    <svelte:component 
        this={component.component}
        name={component.name}
        />
{/snippet}

Each time <svelte:component> access the component attributes, internally it invokes a new component() instance. If component() is heavy, it results in a slowdown of the whole page render.

This is the code generated by Svelte compiler:

$.component(() => component().component, ($$component) => {
            $$component(node, {
                get name() {
                    return component().name;
                },
                $$legacy: true
            });
        });

Since I am already passing a valid Svelte component as parameter, I'd expect no new instance to be created.

Reproduction

I have created a small REPL here

This example renders 3 components, but console log shows them instantiated 6 times in total.

Logs

No response

System Info

System:
    OS: Linux 6.8 Ubuntu 24.04 LTS 24.04 LTS (Noble Numbat)
    CPU: (24) x64 AMD Ryzen 9 7900X 12-Core Processor
    Memory: 50.76 GB / 62.01 GB
    Container: Yes
    Shell: 5.2.21 - /bin/bash
  Binaries:
    Node: 20.11.1 - ~/.nvm/versions/node/v20.11.1/bin/node
    Yarn: 1.22.19 - ~/.yarn/bin/yarn
    npm: 10.2.4 - ~/.nvm/versions/node/v20.11.1/bin/npm
    pnpm: 9.4.0 - ~/.local/share/pnpm/pnpm
    bun: 1.0.0 - ~/.bun/bin/bun
  Browsers:
    Brave Browser: 125.1.66.115
    Chrome: 126.0.6478.114

Severity

blocking an upgrade

dummdidumm commented 3 days ago

You can avoid the reinvocation by using a @const tag:

{#each list as el }
    {@const component = getComponent(el)}
    {@render comp(component, el)}
{/each}

I'm not sure if this warrants a fix - i.e. use a derived to deduplicate accesses - or if we can keep this as-is.

Neptunium1129 commented 3 days ago

You can avoid the reinvocation by using a @const tag:

{#each list as el }
  {@const component = getComponent(el)}
  {@render comp(component, el)}
{/each}

I'm not sure if this warrants a fix - i.e. use a derived to deduplicate accesses - or if we can keep this as-is.

This is my first time seeing this. Thank you.

fsoft72 commented 3 days ago

This solves my problem, but I think it is worth investigating more, because I discovered it only by placing some console.log() inside my function: it could lead to very strange / unexpected behaviors.

You can avoid the reinvocation by using a @const tag:

dummdidumm commented 3 days ago

Giving this the 5.0 label in the sense of "we should decide what to do here (if anything) soon"