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

Flash of content on page load #65

Closed
fabian-michael opened this issue Aug 28, 2020 · 17 comments
Closed

Flash of content on page load #65

fabian-michael opened this issue Aug 28, 2020 · 17 comments
Labels
bug Something isn't working high high priority

Comments

@fabian-michael
Copy link

fabian-michael commented Aug 28, 2020

Loading a site results in a flash of content and thereby a bad cumulative layout shift. I suspect the hydration to be responsible for that.

Bildschirmfoto von 2020-08-28 13-58-53

Edit: See the second screenshot in Lighthouse where the content is gone

@jimafisk
Copy link
Member

Thanks @fabian-michael! This has been bothering me too. I think you're right that this is because of how we're hydrating the page. I used the model Rich provided here when setting this up. There are probably better hydration approaches out there that I need to look into.

@jimafisk jimafisk added the bug Something isn't working label Aug 28, 2020
@jimafisk
Copy link
Member

When compiling we should be setting hydratable to true.

I think it's put well here:

It is important to use the hydratable svelte compiler option for the build of the client code, since we want the client code to hydrate the HTML and not overwrite it.

Per the svelte docs:

If true when generating DOM code, enables the hydrate: true runtime option, which allows a component to upgrade existing DOM rather than creating new DOM from scratch. When generating SSR code, this adds markers to <head> elements so that hydration knows which to replace.

Not sure how exactly this will fit in since we're doing the replacement at the <html> level and not using a bundler like rollup, but I'm looking into it.

@jimafisk jimafisk added the high high priority label Sep 17, 2020
@jimafisk
Copy link
Member

I have a proof of concept working for this.

I modeled ejected/main.js after https://svelte.dev/docs#Creating_a_component:

import App from './App.svelte';

const app = new App({
	target: document.querySelector('#server-rendered-html'),
	hydrate: true
});

And made sure to add hydrate: true per https://svelte.dev/docs#svelte_compile when compiling the client (not the SSR compile). This post was hugely helpful for understanding this: sveltejs/svelte#2901 (comment)

Also found this React discussion about render() vs hydrate() helpful from a conceptual standpoint, the main takeaway being:

If you call ReactDOM.hydrate() on a node that already has this server-rendered markup, React will preserve it and only attach event handlers, allowing you to have a very performant first-load experience.

jimafisk added a commit that referenced this issue Sep 21, 2020
jimafisk added a commit that referenced this issue Sep 21, 2020
@jimafisk
Copy link
Member

Looks like this is nesting top level <html> instead of replacing it...

nested

@jimafisk
Copy link
Member

Getting the "parent" of the <html> element seems to work:

import Router from './router.svelte';

let target = document.querySelector('#hydrate-plenti').parentNode;

new Router({
  target: target,
  hydrate: true
});

Makes me think that we could probably remove the injection of the #hydrate-plenti ID completely and just target the <html> parent directly. That would simplify some of the build and move us closer to my original vision of allowing the global html wrapper to be more flexible (currently it has to be a file located at layout/global/html.svelte, but it would be cool if this was user defined).


Not super helpful, but a similar discussion is happening here: https://stackoverflow.com/questions/61640429/how-to-replace-the-contents-of-a-target-in-svelte-instead-of-appending-to-childr

@jimafisk
Copy link
Member

Console error:

  • Chrome: Uncaught (in promise) DOMException: Failed to execute 'insertBefore' on 'Node': Only one element on document allowed.
  • Firefox: Uncaught (in promise) DOMException: Node.insertBefore: Cannot have more than one Element child of a Document

@jimafisk
Copy link
Member

jimafisk commented Sep 22, 2020

I don't think Svelte was designed to hydrate the <html> element, although for the sake of allowing our users to architect everything from svelte templates (as opposed to having to rig up an index.html file and create an entrypoint), I still think it's the way forward for our project.

Unfortunately you get the same error if you try to explicitly set the anchor:

import Router from './router.svelte';

let anchor = document.querySelector('html');

new Router({
  target: document,
  anchor: anchor,                           
  hydrate: true    
});                                         

Related issue: sveltejs/svelte#2573

jimafisk added a commit that referenced this issue Sep 22, 2020
@jimafisk
Copy link
Member

The updated hydration should work despite the console error, I guess we just live with that for now ¯\_(ツ)_/¯

@fabian-michael I just retested with the new v0.2.37 release, on mobile "cumulative layout shift" seems ok, but it's still flagging it on desktop.

Mobile

mobile

Desktop

desktop

I'll have to look into that some more. If you get a chance to retest, can you let me know if the flash of content looks better at least? Thanks!

@jimafisk
Copy link
Member

There still seems to be an issue around hydrating <svelte:component this={route} {...content.fields} {allContent} />.

Related: sveltejs/svelte#1067

@fabian-michael
Copy link
Author

Hey sorry for the late reply. I haven't got much time.
I would really like to help if I had the time, but unfortunately I got too much to do at the moment.

I'll have a look if I find some time.

@jimafisk
Copy link
Member

No worries @fabian-michael! I did switch out the element replacements we were doing previously for dom repairing hydration, which I think is the better way to go. Unfortunately I think some of the same issues we were experiencing previously are still there, so feel free to hold off testing. This appears to be an ongoing Svelte bug that the community is trying to work out, hopefully more on that soon.

@jimafisk
Copy link
Member

sveltejs/svelte#5470

@jimafisk
Copy link
Member

jimafisk commented Oct 7, 2020

Comment from Baltimore Go about potential cause of flash of content:

Click to see screenshot

Screenshot from 2020-10-06 20-45-54

I think this is a separate issue that also may be happening, if you throttle your connection you might see a brief loading of unstyled content. Svelte has the option to export to a stylesheet or include with JS class: sveltejs/svelte#3604 (comment)

The main issue I'm observing is the elements created by <svelte:component> are briefly removed entirely during hydration.

jimafisk added a commit that referenced this issue Oct 12, 2020
@jimafisk
Copy link
Member

This does not appear to be the hydration issue I thought it was. I ended up pulling down the svelte fork with better hydration, but still had the same problem where the dynamic component was being removed. I also was having trouble replicating this issue in a non-plenti project.

After a little more digging, it seems the issue is related to how we're getting the route value that's used as this for the dynamic component. In a new plenti project if you simply hardcode that value, the removing and re-adding of the element disappears. Here's an example of the layout/global/html.svelte updates you'd have to make for the homepage to work properly:

<script>
  import Index from '../content/index.svelte';
  export let route, content, allContent;
</script>

<svelte:component this={Index} {...content.fields} {allContent} />

@jimafisk
Copy link
Member

I have a working model for this now that fixes the CLS on initial load: 1eccf04. Big thanks to @lukeed for helping me work through the routing setup!

There is one more issue I have to resolve unfortunately before pushing out a release. The insertBefore error previously was just polluting our console logs, now that I've reworked the router to stop the flash of content it makes every page 404 (unless I edit node_modules/svelte/internal/index.mjs directly). I've opened an issue with full diagnosis of the problem here: sveltejs/svelte#5547

@jimafisk
Copy link
Member

I ended up reworking this a bit more. I had previously combined the router and the entrypoint, but that was causing flashes of unstyled content when navigating through client routes. It was hydrating the full dom on every route (including the <html> wrapper, <head> etc), which was more work than we needed to do, since we really want to just swap out the dynamic content and leave the "global" markup unchanged after the initial page load. It seems to be working better with regards to the Cumulative Layout Shift (CLS) issue:

CLS Screenshot

lh

This still needs a modified node_modules/svelte/internal/index.mjs for the document replacement issue. It also seems to work pretty consistently in Firefox, but I still notice some style flashes in Chrome at times.

@jimafisk
Copy link
Member

The newest release (v0.2.39) should fix the issue of dynamic components being removed during hydration: https://github.com/plentico/plenti/releases/tag/v0.2.39.

Note if you're coming from an older version of plenti and want to test the new release, check out this issue so it doesn't break your site: #76

If folks continue to notice flashing content, please reopen this ticket. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high high priority
Projects
None yet
Development

No branches or pull requests

2 participants