Skip to content
This repository has been archived by the owner on Jun 3, 2019. It is now read-only.

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

Merged
merged 5 commits into from
Oct 5, 2017
Merged

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

merged 5 commits into from
Oct 5, 2017

Conversation

oyeanuj
Copy link
Contributor

@oyeanuj oyeanuj commented Sep 27, 2017 •
edited
Loading

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 (#322)

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

@oyeanuj oyeanuj changed the title Doc fixes for Feature/styled components Adds Critical CSS extracted by SC to the actual render Oct 1, 2017
@oyeanuj oyeanuj mentioned this pull request Oct 1, 2017
Open
@oyeanuj oyeanuj mentioned this pull request Oct 1, 2017
Closed
@ctrlplusb
Copy link
Owner

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

@ctrlplusb ctrlplusb requested a review from strues October 4, 2017 20:43
strues
strues approved these changes Oct 5, 2017
View reviewed changes
Copy link
Collaborator

@strues strues left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect. Going to merge this

@strues strues merged commit eaf3724 into ctrlplusb:feature/styled-components Oct 5, 2017
@sergiokopplin
Copy link
Collaborator

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
Copy link
Contributor Author

oyeanuj commented Oct 5, 2017

@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.

    react-universally/server/middleware/reactApplication/ServerHTML.js

    Lines 27 to 28 in 83d533a

    // Resolve the assets (js/css) for the client bundle's entry chunk.
    const clientEntryAssets = getClientBundleEntryAssets();

  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
Copy link
Collaborator

@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
Copy link
Contributor Author

oyeanuj commented Oct 5, 2017

@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
Copy link
Collaborator

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@strues strues strues approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@oyeanuj @ctrlplusb @sergiokopplin @strues