HMR reorders content

[danawoodman]

Describe the bug
When saving a component in the latest version of Kit.

To Reproduce

Repro repo:
https://github.com/danawoodman/sveltekit-content-jump-bug/tree/main

npm init svelte@next foo
cd foo
npm i
npm run dev

Then create a file with a few components, change a component and watch it jump to the bottom.

Expected behavior
Should reload without content jumping around.

Information about your SvelteKit Installation:

Diagnostics

  System:
    OS: macOS 11.2
    CPU: (8) arm64 Apple M1
    Memory: 98.31 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.3.0 - /var/folders/j4/3_1h9_mn6w5g4p438nym2vl40000gn/T/fnm_multishells/74212_1625170227411/bin/node
    npm: 7.15.1 - /var/folders/j4/3_1h9_mn6w5g4p438nym2vl40000gn/T/fnm_multishells/74212_1625170227411/bin/npm
  Browsers:
    Brave Browser: 91.1.25.73
    Chrome: 91.0.4472.114
    Safari: 14.0.3
  npmPackages:
    @sveltejs/kit: next => 1.0.0-next.119 
    svelte: ^3.34.0 => 3.38.3 

• Browser: Brave, Safari (not browser related AFAIK)

Severity
This really messes up local development since you'll need to reload every time to get the proper layout to show up.

Additional context
Add any other context about the problem here.

[dummdidumm]

Could you try with Svelte version 3.38.2? It's just a rough guess, but maybe this is a result of the new hydration optimization in 3.38.3

updated, thanks @Mlocik97

[danawoodman]

@dummdidumm that fixed it! v3.38.2 works, so looks like v3.38.3 introduced the bug

[hbirler]

@benmccann I believe that the issue might lie in svelte-hmr here https://github.com/rixo/svelte-hmr/blob/master/runtime/proxy-adapter-dom.js#L53 . I believe that svelte's insert_dev (or insert) function should be called rather than the DOM's native insertBefore. svelte-hmr likely assumed that all nodes will be re-appended regardless of whether they are in the right positions (which is not the case anymore)

[hbirler]

(Since the issue is not caused by a bug in hydration, I don't think reverting the hydration PR would be ideal. If svelte-hmr cannot be fixed in time, a quick fix might be to disable hydration in dev mode where I believe HMR is used, since it is the interaction between the two that seems to cause the issue)

[benmccann]

Ah, interesting. Thanks for the investigation! CC'ing @rixo and @dominikg who would know the most about svelte-hmr

[dominikg]

Thanks for the deep dive @hbirler . I'll take a closer look.
One possible problem i see here is that this is not the same as initial hydration. Existing elements already have state attached and i'm not sure if they can be kept safely.

[rixo]

The fix suggested by @hbirler seems correct to me, and local tests seems to confirm that it works for me.

I'm not sure whether insert_dev or insert should be used. Since we're adding a comment element to keep track of positions in the DOM, that is not an element that is really "owned" by a Svelte component but purely a HMR artifact, my feeling is that insert should better respect expectations of dev tools. @RedHatter Any thought on this? Should manipulating HMR-only elements trigger dev events?

This change should normally have no effect when used with Svelte <3.38.3 so it should probably be a patch version for svelte-hmr. I'm thinking of publishing a pre-release with this fix, so that everyone here who's interested can test the fix more in-depth before publishing as latest on npm. @benmccann @dominikg Seems OK to proceed like this?