Don't hydrate certain components in client #397
Conversation
|
PRbuilds results: LightHouse Reporting --automated message |
| const { children } = this.props; | ||
|
|
||
| return typeof window === 'undefined' ? ( | ||
| <div>{children}</div> |
There was a problem hiding this comment.
Should probably be a <Fragment> instead of a div
|
Thanks @GHaberis! The reduction in time to interactive is promising. I'd be really keen to investigate the developer experience a bit more, especially how we can help the developer identify the 3 conditions you specify. Firstly what happens at runtime if the conditions aren't satisfied? Do things break or do you see an error message in the console? Secondly, are there any additional checks we can run inside the |
|
I like the way this is its own component, this should let us use any new Suspense features if we get them. Could we use this with tree shaking or changes to the imports to lessen the js bundle size too? |
|
Thanks @GHaberis for looking at this. How do we feel about relying on an implementation detail of React here, which is not guaranteed to be stable? It seems like a fragile base for key parts of our site behaviour and performance going forward. I've also got a few other concerns with this kind of approach:
Are there any alternatives we can explore to this approach? Or would we simply be better off leaving as is until we hit closer to production and can get a fuller sense of the performance cost of client-side React+emotion? |
|
I think we discussed this yesterday, but if Atoms need to be fully fledged components then we lose quite a lot of the benefit here. :( |
For those who weren't around, is there a tl;dr version of this discussion? |
|
@AWare @SiAdcock is this a specific example of point three in my list of concerns above? 'breaks the ability to compose/move components (as you need to fully understand the parent context), whereas a key benefit of the whole components approach is that components work, and can be understood, in isolation of specific context' |
@nicl I agree, it's far from ideal to rely on a coincidental consequence of a
@alex this is true, as discussed once we implement these components we'd have to look at wrapping each block within an article body individually with
@SiAdcock I was thinking about this. One thing we discussed last week was that all components that require clientside behaviour are situated in their own directory, lint rules could prevent lifecycle methods being used on components outside this directory. All static components could be imported via some kind of proxy, like a static component registry which wraps |
|
@GHaberis you mean this Dan?! https://dan.church/ |
|
Closing this for now as we evaluate clientside JS options. |
This is an experimental feature!
We have a performance concern around React unnecessarily hydrating parts of our application which render exactly the same in the client as they do on the server. The same issues has been raised here facebook/react#12617
One of the options put forward in this issue involves using "dangerouslySetInnerHTML with empty content" because React won't try to manipulate the tree of a dangerouslySetInnerHTML node on the client.
This PR introduces a
<Freeze>component. On the client therenderfunction ofFreezereturns an emptydivwith dangerouslySetInnerHTML set on it meaning React doesn't touch it, whilst on the server it renders the children nested within it. The idea is any children wrapped in this component will not be hydrated on the client by React, avoiding the processing overhead involved in hydrating these components.To be wrapped by a
<Freeze>a component must satisfy the following conditions:At the moment the onus is entirely on the developer using
<Freeze>to make sure these conditions are met, if they're not the only way you'd know there's an issue is via runtime bugs!I've tested this branch against
masteronCODEusing https://www.webpagetest.org and have found the following:gh-freeze-components- Moto G4 - Chrome - 3GSlow - 18 runsFirst Interactive (Median) 12.8s
master- Moto G4 - Chrome - 3GSlow - 18 runsFirst Interactive (Median) 14.8s
To do
An additional improvement (not applied here) of omitting certain components from the render cycle in the client is that we could remove the article data currently inline within the
<head>that's currently used during hydration. This should reduce the weight of the HTML document.