Fix duplicated elements on hydration

babeard commented 3 years ago

Fixes a problem where elements were being duplicated upon re-hydration on slower connections. See https://github.com/sveltejs/sapper/issues/1725

benmccann commented 3 years ago

thanks for tracking this down. I wonder if the better fix wouldn't be to wait to call start on load instead instead in https://github.com/sveltejs/sapper-template/blob/master/src/client.js

Conduitry commented 3 years ago

Huh. What exactly needs to wait for what - and what needs to be fixed - now that Sapper is using <script defer>? https://github.com/sveltejs/sapper/pull/1123

babeard commented 3 years ago

Defer or not doesn't seem to affect the hydration/duplication problem unfortunately. I'm not sure why because spec says that deferred scripts are fetched in parallel and evaluated when the page has finished parsing. I'm not quite sure what else would be going on beside's Chrome not strictly adhering to spec. In Firefox, for example, this issue does not appear.

Why is the window load event preferred over moving scripts to the end of the body? Just curious.

Also, sorry for the poor "gitfu". I'm learning as I go. Any pointers are appreciated!

benmccann commented 3 years ago

Why is the window load event preferred over moving scripts to the end of the body? Just curious.

The the script is encountered soon and can start being parsed sooner. Probably doesn't make much of a difference with the small document size, but it is better practice

babeard commented 3 years ago

I'm also questioning now whether the document not being loaded yet would result in the behavior being seen.

My original hypothesis may be totally incorrect, but the two fixes below do resolve the issue we're seeing. Now the question is why. Do you think Chrome's disk cache retrieval could be at play here? As in the cached resource not being deferred but being retrieved and evaluated before it needs to be?

Brief summary of observations

Chrome based browsers only
Initial SSR renders correctly, then duplicate elements appear (presumably on hydration)
Issue appears when on slower network and in combination with a larger client-[hash].js (in this case, it's png files inlined via base64)
Initial page load doesn't seem to be affected, only on subsequent refreshes.
Disabling cache via chrome devtools eliminates the issue as well.
Disabling service-worker does not fix.
Occurs in all versions of Sapper down to and including 0.27.16. (I have not tested below that). I believe the reason that quite a few people are seeing this error when migrating from 0.27 is because the migration guide suggests %sapper.scripts% can be moved to the <head> section for slightly better performance.
It appears svelte-kit does not inherit this problem 🥳

Working Fixes

append %sapper.scripts% to body
start sapper client on window load event (currently this PR)

sveltejs / sapper-template

Fix duplicated elements on hydration #306

Brief summary of observations

Working Fixes