sveltejs / svelte

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

Svelte 4/5: Children/slot rerenders DOM when moved #12895

Open Azarattum opened 3 weeks ago

Azarattum commented 3 weeks ago

Describe the bug

When doing a conditional wrapping like so:

{#if wrap}
  <div>
    {@render children()}
  </div>
{:else}
  {@render children()}
{/if}

When the wrap variable changes, all the children are rerendered. Meaning all the DOM nodes inside are recreated, even though they themselves have not changed. This could cause performance issues and inconsistency. For example, it triggers all the transitions again. It would be really nice if when children/slot moves in markup, Svelte would also move the corresponding DOM node instead of creating a new one (which is exactly the same).

I would like to know if there is currently any workaround to achieve the conditional wrapping without rerendering. It would be nice if there is any way to prevent transitions from triggering at least. This is what broke my app and made me discover this behavior.

https://github.com/sveltejs/svelte/issues/7528 related

Reproduction

Svelte 5 REPL

In the REPL it is clear that Math.random() reruns every time wrap is changed. The in:fly transition is triggered every time as well. Also you can check the console to verify that div with id node is different on every instantiation.

Logs

No response

System Info

System:
    OS: macOS 14.2.1
    CPU: (8) arm64 Apple M1
    Memory: 127.20 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 22.3.0 - /opt/homebrew/bin/node
    npm: 10.8.1 - /opt/homebrew/bin/npm
    pnpm: 9.4.0 - ~/Library/pnpm/pnpm
    bun: 1.1.21 - ~/.bun/bin/bun
  Browsers:
    Brave Browser: 127.1.68.137
    Safari: 17.2.1

Severity

annoyance

Azarattum commented 3 weeks ago

Since this would be considered a breaking change, I think Svelte 5 is where this could potentially be fixed

Azarattum commented 3 weeks ago

In each block with a key you wouldn't expect transitions to rerun or components to rerender. Maybe something like a key could be introduced to @render as well? Something like:

{@render children() as "key"}

This syntax could open a lot of possibilities and would avoid making a breaking change.

trueadm commented 3 weeks ago

Unfortunately, we have to do this as Svelte doesn't use a virtual DOM, so it doesn't know that the DOM structures for both are the same/similar. Making this change would be a huge amount of work and we don't want to block the Svelte 5 launch any longer than we can afford to with these sorts of changes.

Azarattum commented 3 weeks ago

so it doesn't know that the DOM structures for both are the same/similar

Couldn't we tell that by comparing the snippet reference children === children and its arguments (both empty in this case)?

Anyway, could this be considered for 5.x then? The keyed approach is opt in, so that wouldn't be a breaking change. This feature opens quite a lot of optimization possibilities and behaviors that weren't possible before.

Btw, do you like the proposed API? Maybe there is a better keyword than as or is it good enough? Are we fine with making the keys global or should they be scoped to a component (more limitations but safer)?