ctrlplusb / react-universally

A starter kit for universal react applications.
MIT License
1.7k stars 244 forks source link

Adds Critical CSS extracted by SC to the actual render #505

Closed oyeanuj closed 6 years ago

oyeanuj commented 6 years ago

Previously, the styles were being extracted but not actually utilized anywhere. This PR extracts them as style elements and passes them to <HTML /> element via <ServerHTML /> and ensures that they are rendered as part of the <HTML /> tag.

So, it is only now SC with SSR is working. Additionally, no FoUC for the SC part (https://github.com/ctrlplusb/react-universally/issues/322)


Edit: Re-purposed a doc change PR for this since I was on the same branch.

ctrlplusb commented 6 years ago

Zing, awesome stuff @oyeanuj! We'll need to get someone who uses styled-components to review this though. :)

sergiokopplin commented 6 years ago

Is there any way to apply that to non styled components branchs? @oyeanuj @strues Google is saying that o should put css inline, but im not sure the better way to do that

oyeanuj commented 6 years ago

@sergiokopplin Seems like there are two parts to your question -

  1. Extract styles needed for server rendering: That I think should already be happening here below. If it isn't, then that's a bug. https://github.com/ctrlplusb/react-universally/blob/83d533a9c780716d18f034f7fb52dbd3a1c4051b/server/middleware/reactApplication/ServerHTML.js#L27-L28

  2. Ensure the styles extracted are the minimum ones needed for the render, aka critical css: That part is a feature of styled-components and other similar libraries. They essentially combine this task with the above task. I am sure going through those repos, you'll find explanation for techniques they use. Or maybe someone else here can chime in.

So, really this PR was making sure the above task was actually happening for styled-components css.

sergiokopplin commented 6 years ago

@oyeanuj.

  1. When i call this guy (clientEntryAssets.css) i get undefined. I want all the styles to put them inline, but i don't know where to pick them.

  2. 👍

oyeanuj commented 6 years ago

@sergiokopplin Hm.. that sounds like a bug. You could check the assets.json file to see if the css assets are being tracked there which implies the build is working properly. If yes, then the bug is somewhere in getClientBundleEntryAssets or else in the build.

@ctrlplusb @strues @diondirza might have more helpful answers.

sergiokopplin commented 6 years ago

thanks @oyeanuj. if it's a bug, we should move this chat into an issue right? haha