Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplication of elements on hydration in Svelte 3.38.0 #6274

Closed
intrikate opened this issue May 1, 2021 · 16 comments · Fixed by #6275
Closed

Duplication of elements on hydration in Svelte 3.38.0 #6274

intrikate opened this issue May 1, 2021 · 16 comments · Fixed by #6275
Labels

Comments

@intrikate
Copy link

Describe the bug
Under certain conditions, hydration appends elements to the DOM alongside prerendered parts, leading to element duplication. This seems to specifically affect routes/components using svelte:head and depends heavily on components’ structure. There is no duplication after navigating away and coming back.

This behaviour is almost certainly introduced by a recent change in Svelte, but it doesn’t seem to affect much beyond SvelteKit (obviously not REPL, but also not Sapper), so may warrant a look at this side.

To Reproduce
Clone and run the demo: https://github.com/intrikate/sveltekit-duplication-issue.

Expected behavior
CSR should overwrite, not duplicate, SSR.

Information about your SvelteKit Installation:

Diagnostics

System:
OS: macOS 11.2.3
CPU: (4) x64 Intel(R) Core(TM) i5-4288U CPU @ 2.60GHz
Memory: 130.07 MB / 8.00 GB
Shell: 5.8 - /bin/zsh
Binaries:
Node: 14.16.0 - /usr/local/bin/node
Yarn: 1.22.10 - /usr/local/bin/yarn
npm: 6.14.11 - /usr/local/bin/npm
Browsers:
Chrome: 90.0.4430.93
Firefox Developer Edition: 89.0
Safari: 14.0.3
npmPackages:
@sveltejs/kit: next => 1.0.0-next.94
svelte: ^3.38.0 => 3.38.0
vite: ^2.2.3 => 2.2.3

Severity
High.

@Kansuler
Copy link

Kansuler commented May 1, 2021

I just upgraded my project from version 71 to 94 and met the same issue. I can confirm that it does no longer occur when I remove the <svelte:head> from my page components.

edit: downgrading to [email protected] seems to fix the issue as well.

@DhyeyMoliya

This comment has been minimized.

@DhyeyMoliya

This comment has been minimized.

@intrikate

This comment has been minimized.

@stalkerg
Copy link
Contributor

stalkerg commented May 1, 2021

@intrikate it's broken, just not so many people using a hydration and looks like we skip it during tests

@Conduitry Conduitry transferred this issue from sveltejs/kit May 1, 2021
@Conduitry
Copy link
Member

Thank you for the excellent set of reproductions! I am moving this to the Svelte repro, as this is almost certainly a bug caused by the hydration changes in 3.38.0.

@lukasIO
Copy link

lukasIO commented May 1, 2021

I was seeing the same thing. I set up a reproduction repository with minimal modification from the demo app:
lukasIO/kit-svelte-3.38@688833c

edit: ah, there was already one...

@johngrasty
Copy link

I'm seeing this as well. I didn't notice this issue already, but I also put together a repo that reproduces it. This is from skelton project, not the demo. <svelte:head> was also what I was seeing. Here's the repo in case you need it.

Repo reproducing bug

When clicking the link, the test page will not have the duplication. If reloaded, it will

@UltraCakeBakery
Copy link

I have this issue in my project without svelte:head being in the picture at all. On my end it depended on what I set as the hydration target. No element (or body) doesn't cause any duplicated items at all.

@benmccann benmccann changed the title Duplication of elements on hydration (possible compat issue with Svelte 3.38.0) Duplication of head elements on hydration in Svelte 3.38.0 May 1, 2021
@DhyeyMoliya
Copy link

DhyeyMoliya commented May 1, 2021

@benmccann it is not duplication of head elements, the actual elements of the affected pages are getting duplicated, like forms, divs, etc.

Update : If you mean head elements as the first html element, then ignore this comment. I was understanding head elements as <svelte:head>

@johngrasty
Copy link

@benmccann it is not duplication of head elements, the actual elements of the affected pages are getting duplicated, like forms, divs, etc.

Right. That’s what I’m seeing as well, but in my minimal repo, it seemed that just the first element was being duplicated. For example, if there are two divs that are siblings. If there is a <svelte:head>, the first div will be duplicated but not the second.

@DhyeyMoliya
Copy link

Suprisingly, if I remove <div id="svelte">%svelte.body%</div> from app.html and set target: 'body', everything works well. Similar to @UltraCakeBakery .

@Conduitry Conduitry changed the title Duplication of head elements on hydration in Svelte 3.38.0 Duplication of elements on hydration in Svelte 3.38.0 May 1, 2021
@benmccann
Copy link
Member

I'm curious why the existing tests didn't catch this. https://github.com/sveltejs/svelte/tree/master/test/hydration/samples/head-meta-hydrate-duplicate

Also, I have a SvelteKit app deployed to production using Svelte 3.38.0 at http://c3.ventures/ and don't see this

It'd be nice if someone was able to turn one of the reproductions into a test that fails with #6204 and passes without it

@Conduitry
Copy link
Member

I tried for a little while this morning to produce a failing test, but without success. While the SvelteKit app that reproduces this is quite simple, there's a bit that's tucked inside .svelte/dev/generated/root.svelte that's presumably an important part of reproducing the bug, which makes producing a Svelte-only hydration repro a bit trickier.

@Conduitry
Copy link
Member

This should be fixed now in 3.38.1.

@DhyeyMoliya
Copy link

DhyeyMoliya commented May 1, 2021

This should be fixed now in 3.38.1.

It gives me great happiness to see this.
It was great to see contributors coming together once in awhile to solve an issue in oss. It was very quick and responsive.

@tanhauhau @Conduitry @benmccann @lukeed I thank you all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants