sveltejs / svelte

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

Svelte5: Inconsistent DOM Update Behavior between Svelte 4 and Svelte 5 using sveltekit2 #10332

Closed FoHoOV closed 7 months ago

FoHoOV commented 7 months ago

Describe the bug

Lets try to rewrite this svelte 4 code to svelte 5:

--svelte 4

<script lang="ts">
    import { readable } from 'svelte/store';
    import type { PageData } from './$types';
    import { browser } from '$app/environment';

    export let data: PageData; // data = [{id: 1,name: 'test-server'}, {id: 2, name: 'test-server2'}]

    const clientData = readable([
        {
            id: 1,
            name: 'test-client'
        }
    ]);
</script>

{#each browser ? $clientData : data.serverData as test (test.id)}
    <h1>
        {test.name}
    </h1>
{/each}

--svelte5

<script lang="ts">
    import { readable } from 'svelte/store';
    import { browser } from '$app/environment';

    const { data } = $props(); // data = [{id: 1,name: 'test-server'}, {id: 2, name: 'test-server2'}]

    const clientData = readable([
        {
            id: 1,
            name: 'test-client'
        }
    ]);
</script>

{#each browser ? $clientData : data.serverData as test (test.id)}
    <h1>
        {test.name}
    </h1>
{/each}

Expected Behavior

: The expected outcome is identical to the Svelte 4 version: initially displaying two `h1` elements with 'test-server' and 'test-server2', then dynamically updating the DOM to only show 'test-client' when the JavaScript loads and runs. This behavior is consistent in a Svelte 4 app (refer to tests-svelte4 and run it).

Actual Behavior in Svelte 5

: In the Svelte 5 version, the behavior differs unexpectedly. While it updates 'test-server' to 'test-client', it fails to remove 'test-server2' from the DOM. I've created a [reproduction repo here](https://github.com/FoHoOV/svelte5-bug-report-2). ### Reproduction 1. [clone this repo](https://github.com/FoHoOV/svelte5-bug-report-2) 2. run the tests-svelte4 application to see the expected outcome 3. run the tests-svelte5 application to see the exact same code which uses `props` instead of `export data` to fail * you might have turn on network throttling to see exactly when the update happens ** even using the svelte4 version in an app that uses svelte5, it fails** ### Logs _No response_ ### System Info ```shell System: OS: Windows 11 10.0.22621 CPU: (16) x64 AMD Ryzen 7 4800H with Radeon Graphics Memory: 21.86 GB / 31.42 GB Binaries: Node: 20.4.0 - C:\Program Files\nodejs\node.EXE npm: 9.7.2 - C:\Program Files\nodejs\npm.CMD Browsers: Edge: Chromium (121.0.2277.83) Internet Explorer: 11.0.22621.1 ``` ### Severity blocking an upgrade
FoHoOV commented 7 months ago

After some more testing, I figured out that using the svelte4 version of the code in an app that uses svelte5 (although it should behave the same with no changes) the same bug happens. So I think it is something about svelte5 in general.

FoHoOV commented 7 months ago

After even some more testing, this doesn't work also, this is so weird and so fundamental to svelte usage:

<script lang="ts">
    import { readable } from 'svelte/store';
    import { browser } from '$app/environment';

    const { data } = $props(); // data = [{id: 1,name: 'test-server'}, {id: 2, name: 'test-server2'}]

    const clientData = [
        {
            id: 1,
            name: 'test-client'
        }
    ];
</script>

{#each browser ? clientData : data.serverData as test (test.id)}
    <h1>
        {test.name}
    </h1>
{/each}

server-side rendered test-server2 element stays on the page whilst it should only be showing the test-client element.

dummdidumm commented 7 months ago

Svelte 5 assumes that the array doesn't change between server and client; we should probably add a check during hydration that the length stayed the same.

FoHoOV commented 7 months ago

@dummdidumm So, this means we can't make a new object from the server's array of something and then change it by adding or removing elements on the client side, right? This limit seems like it would stop a lot of uses. Don't you think so? Why is this limit there?

I believe we should be able to modify the initial list of items given by the server on the client side (using localStorage, requests from the client side, user interactions (clicking delete btn etc).

take this as an example:

<script lang="ts">
    import { browser } from '$app/environment';

    const { data } = $props(); // data = [{id: 1,name: 'test-server'}, {id: 2, name: 'test-server2'}]

    const filteredData = $state(data);

    if (browser) {
        // filter based on localStorage, user interaction (pressing delete button), geolocation or whatever
        filterBasedOnSomethingClientKnows(filteredData);
    }

    function filterBasedOnSomethingClientKnows(data: any) {
        data.serverData.pop();
    }
</script>

{#each filteredData.serverData as test (test.id)}
    <h1>
        {test.name}
    </h1>
{/each}

I also want to point out that changing things this way won't alter the original page data from the server. Think of it as making a new object and changing it by adding or removing elements based on information that only the browser knows about which should be allowed and work right?