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

Better hydration #4308

Closed
avi opened this issue Jan 23, 2020 · 40 comments · Fixed by #6204 or #6395
Closed

Better hydration #4308

avi opened this issue Jan 23, 2020 · 40 comments · Fixed by #6204 or #6395

Comments

@avi
Copy link

avi commented Jan 23, 2020

Is your feature request related to a problem? Please describe.
Currently hydration is very expensive, it searches for the correct node type clears it and re-adds the attributes and finally unmounts and remounts the DOM node by calling insertBefore/append
This causes iframes to reload.
In the current implementation setting the innerHTML to '' and doing client side render is faster and cleaner

Describe the solution you'd like
All DOM should be reused, attributes should not be cleared, and no node should be insertBefore/append unless there is an actual reason.
DOM nodes shouldn't be scanned for the correct nodeName - assume everything is in order on the first miss clear all the rest and start adding new nodes as needed
Text nodes need special treatment as they collapse in SSR

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

How important is this feature to you?
It is a deal breaker for us - we have quite a few iframes in use and using hydration in it's current form reloads them, it also slows our TTI. So we are stuck with either using only client-side rendering or picking a different solution

Additional context

@avi
Copy link
Author

avi commented Feb 9, 2020

If anyone else is blocked by the iframes ssr/hydration problems or really cares about hydration speed, I'm currently maintaining a fork https://github.com/wix-incubator/wix-svelte which you can use directly just point your package.json
"svelte": "ssh://git@github.com:wix-incubator/wix-svelte.git#master" (or hard code the current sha1 if you don't want to get updates)

@sandrina-p
Copy link

sandrina-p commented Feb 12, 2020

At the moment this is a major problem on a website I'm working on with Sapper. I opened an issue on Sapper repo. sveltejs/sapper#1088

(@avi I'll try your fork tomorrow and let you know if it worked)

@sandrina-p
Copy link

sandrina-p commented Feb 13, 2020

@avi I just tested your fork on my project (see commit sandrina-p/portfolio_v3@96cfa06) and it worked! Thanks! 😻

@benwoodward
Copy link

benwoodward commented May 17, 2020

I'd like to see this improvement absolutely. Currently we're server-side rendering components via node CLI, then hydrating on the client (building a language learning app). Because SSR elements are cleared and re-added it results in the rendered page being displayed, before disappearing to be replaced with a loading message, and then the hydrated page appears.

@jackyef
Copy link
Contributor

jackyef commented Jun 16, 2020

Just found this issue after looking around. +1 for this!

My specific issue related to this is that it causes Largest Contentful Paint to only be registered after hydration is done, because it removes the DOM node and re-insert it.

@avi
Copy link
Author

avi commented Jun 16, 2020 via email

@benmccann
Copy link
Member

@avi thanks for taking another look at this!

I had a question on overall approach. Take a look at #4975 and let me know what you think

@avi
Copy link
Author

avi commented Jun 16, 2020

The problem is that this a breaking change right now and we need to decide if the old way is worth preserving to make migration easier or not. I think it isn’t because right now the diff between doing hydration and cleaning all the SSRed DOM and doing client side render is better than using hydration as is... but that is easy for me to say because we don’t use svelte right now partly because of this issue... I need a decision/guidance from a maintainer...

@sandrina-p
Copy link

the diff between doing hydration and cleaning all the SSRed DOM and doing client side render is better than using hydration as is...

This is a deal-breaker for websites that have animations or media autoplay (e.g. audio, videos, gif, etc...) on page load because it would restart them when the CSR is done, as I explained in the issue sveltejs/sapper#1088.

@benmccann
Copy link
Member

benmccann commented Jun 16, 2020

For what it's worth, I'm leaning towards the idea that client-side rendering (CSR) with a CDN that pushes preload assets and dynamic rendering is a better solution in nearly all cases than SSR + hydration. See sveltejs/sapper#383 (comment) and https://developers.google.com/web/updates/2019/02/rendering-on-the-web for more details. I've really been pushing on that path in Sapper with sveltejs/sapper#1269 and sveltejs/sapper#1275. I might be almost as happy to remove hydration altogether because it seems like a trap that's easy to fall into. Intuitively it makes sense, but I'm not sure it actually has any benefits over the CSR + CDN + dynamic rendering solution, which hasn't received as much attention - though happy to be corrected if I'm overlooking something or explain further if more details would be helpful

@antony antony added the has pr label Jun 16, 2020
@avi
Copy link
Author

avi commented Jun 16, 2020 via email

@Conduitry
Copy link
Member

This being a breaking change isn't just some theoretical concern - I have a project right now where there's some text related to timezones that might get changed during hydration vs what came from the server. If we want to have something that makes more assumptions about what it's starting with, it would need to be opt-in rather than a default, and ideally it would also be something over which more control can be exerted at compile time, as no one knows better than the application author which bits are safe to assume will already match. I don't know what that new API or component syntax would look like.

@avi
Copy link
Author

avi commented Jun 16, 2020 via email

@sandrina-p
Copy link

If I'm not mistaken, in React, when the SSR (server-side render) and CSR (client-side render) don't match, there's a warning in the console about it. Maybe here it could follow the same approach and explain that those types of "diffs" must be fixed.

image

Example: Check the URL to activate an element (by adding a class active). This will cause a diff between SSR and CSR because window does not exist in the server. So, the solution is to only check the URL after onMount(). That way both SSR and CSR will match.

@benmccann
Copy link
Member

I got a bit more detail from the other maintainers on why this PR has languished. There's an overall feeling that the current approach of repairing the DOM is considered to be a feature not a bug. There are cases it handles that the PR would not such as progressive enhancement of a select that turns into an autocomplete upon client render. There's also a recognition that the current implementation has performance concerns and edge cases it doesn't handle well (iframes, videos, etc.) and leaves much to be desired. However, I think there's some feeling that we should investigate whether it's possible to create a solution that addresses these shortcoming without losing out on the existing DOM repairing feature.

I know there's been a lack of communication on this PR. We've tried to make a handful of changes to make it easier to discuss changes lately such as setting up #svelte-dev and #contributing channels on Discord and better surfacing the RFC process. The hope is that we can discuss any big changes before they get to the implementation stage to work out the details in the most efficient manner, so please feel to free to find us via those avenues.

@avi
Copy link
Author

avi commented Sep 21, 2020 via email

@m4rrc0
Copy link

m4rrc0 commented Sep 24, 2020

Hey, since the topic of this issue is 'Better hydration' and maintainers seem to be investigating the subject, I'll drop my 2 cents on a somewhat perpendicular concern... ^^
If it diverges too much from the original discussion, I am glad to take this discussion somewhere else.

It would be nice to have a way to target parts of the DOM for hydration more precisely. For the moment, we can only wipe out everything inside a DOM node.
(I am currently using an alternative implementation that replaces the node I need to hydrate for convenience.)

The use case is partial hydration of a component containing multiple top-level elements (that may be side by side with other elements from other components). Partial hydration is a feature that is on everyone's mind when looking at the future of static site generators (SSG) or server side rendering. I am working on an SSG and I have the feature working but it comes with compromises that I would love to be able to avoid, namely the whole component that we want to hydrate with JS needs to be wrapped in an HTML element alone.
While it does not seem that big of a deal, it is the kind of things the dev needs to understand, watch for and keep in mind at all times. We cannot assume that it 'just works' and pick and choose the parts we want to hydrate. And it is counter intuitive since Svelte allows us to create side-by-side elements in a component easily.

I have read a few issues regarding this behavior and understand the concern of shipping too much code when it only applies to edge use cases. I am posting this here because I am wondering if there is a better solution for hydrating that does not involve shipping more code. I don't know the ins and outs of hydration with Svelte so I am mainly talking about developer experience here.

I am happy to discuss hydration and provide examples of how I use it if it can help bring the discussion forward. :)

PS: Another SSG project doing partial hydration with Svelte is ElderJS. I haven't investigated yet if the author managed to workaround the limitations I mention.

@nickreese
Copy link

Elder.js maintainer here.

You nailed it. Components must be wrapped in a named div so we can target it. The implementation breaks inline, inline-block components (icons for instance), grid components but most of can be fixed with css.

The greater problem we’ve faced is the behavior of the CSS being emitted during SSR. If there are any false-y values the css is just excluded.

While it makes sense to exclude css that is not relevant when rendering on the client, on the server it makes 0 sense.

We’re still trying to find a work around.

@benmccann
Copy link
Member

I don't want to get too far off-topic here, but Sapper's CSS handling was recently rewritten and we plan to refactor it out into a separate Rollup plugin in the hopefully not-too-distant future (I'm mainly waiting to have a release or two without any CSS bug reports to make sure things are really working well first). I'm not fully aware of what your issues are, but you might want to watch the Sapper release notes for when that happens. Feel free to ping me on Discord or elsewhere for more details, so that we don't take this off-topic here

@PierBover
Copy link

PierBover commented Mar 27, 2021

For what it's worth, I'm leaning towards the idea that client-side rendering (CSR) with a CDN that pushes preload assets and dynamic rendering is a better solution in nearly all cases than SSR + hydration. See sveltejs/sapper#383 (comment) and https://developers.google.com/web/updates/2019/02/rendering-on-the-web for more details. I've really been pushing on that path in Sapper with sveltejs/sapper#1269 and sveltejs/sapper#1275. I might be almost as happy to remove hydration altogether because it seems like a trap that's easy to fall into. Intuitively it makes sense, but I'm not sure it actually has any benefits over the CSR + CDN + dynamic rendering solution, which hasn't received as much attention - though happy to be corrected if I'm overlooking something or explain further if more details would be helpful

As a temporary solution until hydration is improved I currently disabled SSR+hydration in my project and switched to CSR. I just send the data into a <script type="application/json"> and the page component to the client. It works better than I expected. If you go back the state is maintained in forms, JS, etc. I haven't tested this thoroughly though. I suspect there will be issues.

Honestly, there should be huge red warnings in the docs about the current state of hydration in Svelte which seems almost unusable. Makes you wonder if that's the reason why Sapper pushed people into a SPA (and apparently SvelteKit will too).

Edit:

I was wrong. The JS state is not maintained when going back with CSR. The form values are though.

After a long discussion with Rich on Twitter about client-side routing, I have to concede the SSR+hydration approach is less desirable than I initially thought for my use case (backend dashboard).

I totally wish hydration was better in Svelte, but if using client-side routing this is definitely much less of an issue.

@pranaypratyush
Copy link

As a temporary solution until hydration is improved I currently disabled SSR+hydration in my project and switched to CSR. I just send the data into a <script type="application/json"> and the page component to the client. It works better than I expected. If you go back the state is maintained in forms, JS, etc. I haven't tested this thoroughly though. I suspect there will be issues.

Honestly, there should be huge red warnings in the docs about the current state of hydration in Svelte which seems almost unusable. Makes you wonder if that's the reason why Sapper pushed people into a SPA (and apparently SvelteKit will too).

Although a bit unrelated but having an option in sveltekit to switch between ssr and this method while preserving all the other features would be great.

@nickreese
Copy link

Any open blockers on this?

Huge SEO priority as we’ve seen Google is very slow at indexing pages that have large chunks of their content hydrated.

To me this points to Gbot waiting for the JS rendering of the page to be done before analyzing the page for quality mainly because the DOM is destroyed and rebuilt. Yes this is speculation but very hard to understand why these pages would be taking so long to be indexed. It is a common theme across several sites.

@PierBover
Copy link

To me this points to Gbot waiting for the JS rendering of the page to be done before analyzing the page for quality mainly because the DOM is destroyed and rebuilt. Yes this is speculation but very hard to understand why these pages would be taking so long to be indexed. It is a common theme across several sites.

Yeah GoogleBot definitely waits for JS execution. A couple of years ago I remember Google made a big deal out of GoogleBot finally being able to execute ES6.

There are services that pre-render sites to static HTML specifically for crawlers such as: https://prerender.io/

Google has actually an article with more info about this: https://developers.google.com/search/docs/guides/dynamic-rendering

@nickreese
Copy link

nickreese commented Apr 1, 2021

I’m definitely familiar. The thing is that we’re SSRing (static exporting) the entire page and partially hydrating 3-4 components. (Elder.js)

If a component that is hydrated is above the fold Google delays indexing until the JS bot finishes rendering which takes days to weeks.

This isn’t the case with all SSR setups and I believe is unique to Svelte’s poor hydration model that throws out the existing DOM and rebuilds it.

benmccann added a commit that referenced this issue Apr 21, 2021
* Improve SSR hydration performance

- Fixes #4308 by avoiding de- and reattaching nodes during hydration
- Turns existing append, insert and detach methods into "upserts"

The new "hydration mode" was added in order to maintain the detach by
default behavior during hydration. By tracking which nodes are claimed
during hydration unclaimed nodes can then removed from the DOM at the
end of hydration without touching the remaining nodes.

Co-authored-by: Jonatan Svennberg <jonatan.svennberg@gmail.com>
@benmccann
Copy link
Member

Reopening this since the performance improvement had to be reverted (#6290) due to a bug. While reverting we added a test to prevent future regressions. We might be able to get some more tests in by fixing up and merging #4444

If anyone would like to take another stab at getting the performance improvement in, the issue with the original change is described in #6279 (comment) and we'd welcome a PR that improves performance while avoiding that issue.

@benmccann benmccann reopened this May 3, 2021
@benmccann
Copy link
Member

@tanhauhau I just happened to stumble upon #3766 today where you said that you're merging consecutive text-a-like nodes. I think there's a good chance that would actually allow us to get the hydration performance fix back in as it was without any additional changes. It was failing when encountering consecutive text-nodes because they were represented as multiple nodes, but would be merged by the browser such that hydration would fail on the client because it was looking for multiple nodes, but they'd been merged. If we're able to get your PR in and we always treat them as a single text node then I think we wouldn't have this issue.

@danawoodman
Copy link
Contributor

Sorry if this was answered elsewhere, but has "better hydration" been addressed or has it been abandoned?

@istarkov
Copy link

@danawoodman its periodiocally added, then reverted, so its hard to maintain current state :-)

@hbirler
Copy link
Contributor

hbirler commented Aug 18, 2021

I believe the primary issue mentioned here (slowness and animation flicker) has been fixed since Svelte 3.38.3 with the #6395 PR. There seems to be some issues to iron out in some edge cases (There were some problems with @html nodes, but I haven't had the time to look at how everything stands in the last couple of weeks) but most use cases should be fine with the current implementation. If your application has not improved (or has had a regression) please open an issue 😄

@eriknyk
Copy link

eriknyk commented Apr 21, 2023

SSR aside of expensive is veeeery slower on my production app, so in the end I've opted to disable SSR globally to have an acceptable performance in my app

@PierBover
Copy link

PierBover commented Apr 21, 2023

@eriknyk what version of Svelte are you using?

I did some SSR benchmarks recently with 3.58.0 and the SSR perf was quite good. Almost as good as Solid which is one of the fastest and even dedicated template engines like Handlebars and Art.

https://github.com/PierBover/fastify-vite-ssr-benchmarks

@benmccann
Copy link
Member

#9662 skips hydration for select attributes and adds a warning in dev mode if the SSR and client-side values are different. This is similar to the idea @avi implemented in #4309, but for a few attributes that are unlikely to differ rather than the entire DOM so that repair still occurs if there are differences elsewhere. Interested users may wish to test that PR or Svelte 5 pre-releases containing the PR after it's merged

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

Successfully merging a pull request may close this issue.