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.

Sign up for GitHub

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

Jump to bottom

feat(v2): only create one css file to avoid code-split css loading problem #2007

Merged
merged 2 commits into from
Nov 17, 2019
Merged

feat(v2): only create one css file to avoid code-split css loading problem #2007

merged 2 commits into from
Nov 17, 2019

Conversation

endiliey
Copy link
Contributor

Motivation

Close #2006

See issue for reasoning

Have you read the Contributing Guidelines on pull requests?

yes

Test Plan

  • yarn build, only one css file is created
  • netlify ???
  • no css break on local build

caveat:

  • dont know how big is our css 😭

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 17, 2019
@endiliey endiliey changed the title feat(v2): only create one css file to avoid code-split css loading pr… feat(v2): only create one css file to avoid code-split css loading problem Nov 17, 2019
yangshun
yangshun approved these changes Nov 17, 2019
View reviewed changes
Copy link
Contributor

@yangshun yangshun left a comment

Choose a reason for hiding this comment

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

How big is our CSS now? :)

@docusaurus-bot
Copy link
Contributor

Deploy preview for docusaurus-2 ready!

Built with commit 0601bf9

https://deploy-preview-2007--docusaurus-2.netlify.com

@docusaurus-bot
Copy link
Contributor

Deploy preview for docusaurus-preview ready!

Built with commit 0601bf9

https://deploy-preview-2007--docusaurus-preview.netlify.com

@endiliey
Copy link
Contributor Author

webpack bundle analyzer won't display css size. But before gzip, its 92kb in windows explorer
image

@endiliey endiliey added the status: don't merge yet This PR is completed, but we are still waiting for other things to be ready. label Nov 17, 2019
@docusaurus-bot
Copy link
Contributor

Deploy preview for docusaurus-2 ready!

Built with commit be98ffa

https://deploy-preview-2007--docusaurus-2.netlify.com

@docusaurus-bot
Copy link
Contributor

Deploy preview for docusaurus-preview ready!

Built with commit be98ffa

https://deploy-preview-2007--docusaurus-preview.netlify.com

@yangshun
Copy link
Contributor

yangshun commented Nov 17, 2019 •
edited
Loading

It's 17kb after gzip based on the preview. This is fine considering previously it was 16.3kb split across 2 files.

I inspected the files and saw that the extra CSS is react-toggle, algolia search and some page-specific CSS modules. One downside is that we won't have good caching for CSS since they are all in one file now - one small change anywhere will cause the filename hash to change.

LGTM.

@endiliey
Copy link
Contributor Author

endiliey commented Nov 17, 2019 •
edited
Loading

As a side note it decreases web dev lighthouse performance by 10
image

I wonder if we can somehow reduce the size

This is fine considering previously it was 16.3kb split across 2 files.
Previously it wasn't the case, because the infima css is already loaded in main bundle. while the other css is code-splitted. So it's 10.2kb first and the rest is code-splitted

So going to https://v2.docusaurus.io/ is 10 + 6kb
image

Then going to https://v2.docusaurus.io/docs/introduction is probably another 6kb.

This PR is definitely better though long-term wise. Just wishing we can tree shake that css hehehe

@yangshun yangshun merged commit b5cbaee into master Nov 17, 2019
@lex111
Copy link
Contributor

lex111 commented Nov 17, 2019

What about something like critical CSS or a similar technique?

@endiliey
Copy link
Contributor Author

Happy to accept PR on that :)

@endiliey endiliey added the pr: bug fix This PR fixes a bug in a past release. label Nov 18, 2019
@endiliey endiliey deleted the endi/css-extract branch November 21, 2019 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yangshun yangshun yangshun approved these changes

Assignees
No one assigned
Labels
CLA Signed Signed Facebook CLA pr: bug fix This PR fixes a bug in a past release. status: don't merge yet This PR is completed, but we are still waiting for other things to be ready.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

The same CSS files are injected several times when switching pages
5 participants
@endiliey @docusaurus-bot @yangshun @lex111 @facebook-github-bot