facebook / docusaurus

Easy to maintain open source documentation websites.
https://docusaurus.io
MIT License
55.71k stars 8.34k forks source link

The same CSS files are injected several times when switching pages #2006

Closed lex111 closed 4 years ago

lex111 commented 4 years ago

🐛 Bug Report

I noticed a strange bug when switching pages (Home -> Docs -> Blog) CSS styles are duplicated (re-injected as I understand it correctly).

Have you read the Contributing Guidelines on issues?

Yes

To Reproduce

  1. Open devtools and focus on styles (body eg)

  2. Open Home page (https://v2.docusaurus.io/) and we can already see that the styles are duplicated! image

  3. Alright, go ahead, click on Docs (https://v2.docusaurus.io/docs/introduction) in navbar and check the styles in devtools again, now the styles are duplicated several times (+3 new)! image

Expected behavior

Styles should not be duplicated (re-injected) when moving to other pages.

Actual Behavior

For some reason, the same styles are injected, which overridden the earlier ones.

Reproducible Demo

See To Reproduce section.

endiliey commented 4 years ago

Well, it's mostly because of code splitted css. example from https://github.com/facebook/docusaurus/blob/6204d9a9ec1a64cdbee53ca02c785da3380bf73e/packages/docusaurus-theme-classic/src/theme/Layout/styles.css#L8-L23

First of all, this requires a high level understanding of how webpack works. The thing is, webpack mini-css-extract-plugin create one css file per js file and then the splitChunk try to cleverly take out a common one if possible but the algorithm is much more complicated than that because it tries to minimize number of request too https://webpack.js.org/plugins/split-chunks-plugin/#defaults

So https://v2.docusaurus.io/docs/introduction and https://v2.docusaurus.io/ actually both depends on Layout (which depends on that css) and both have their own extra css

This is just an illustration but i guess this can serve the purpose Example /docs/introduction requires final css like this

.body {
  margin: 0;
}
.content {
   flex: 1 0 auto;
}

And / final required css is like this

.body {
  margin: 0;
}
.github {
   color: red
}

Here is the question. Do you think webpack will take out the common body { margin: 0;} ? Such that if i go to /docs/introduction, it will request a.css and this common.css ? and if i go to /, it will just request b.css since common.css has been loaded before

Turns out that it doesn't, because the common.css is too small. and its just better to focus on one css request on first initial page load, thats why the reinjection happens.

Actually this is not happening to CSS only, there are lot of duplicated JS out there for all generated webpack projects.

endiliey commented 4 years ago

One solution is to move https://github.com/facebook/docusaurus/blob/6204d9a9ec1a64cdbee53ca02c785da3380bf73e/packages/docusaurus-theme-classic/src/theme/Layout/styles.css#L8-L23 to main bundle.

That's why people nowadays use css-in-js, it has better tree shaking ✌️ (and chunking)

yangshun commented 4 years ago

It's what you said was true, it's weird behavior by webpack though. Reinjecting styles is a bad idea as precedence ordering matters. And I'd assume webpack wouldn't reinject the same style module again if the module has already been loaded before.

Regardless, this issue is probably something to do with webpack.

endiliey commented 4 years ago

Another solution is to extract all css into single file. But that is not a good idea as well since that means a lot of unused css in certain page. All has pros and cons.

Its a very weird bug, although its not breaking any website. In fact, requesting one css file is better than requesting two files. Its also very small in size. It just happens to be the body selector so its noticeable.

Still better than going to https://docusaurus.io/docs/en/tutorial-setup and https://docusaurus.io/en/. Everytime new big css file is requested.

endiliey commented 4 years ago

It's what you said was true, it's weird behavior by webpack though. Reinjecting styles is a bad idea as precedence ordering matters. And I'd assume webpack wouldn't reinject the same style module again if the module has already been loaded before. Regardless, this issue is probably something to do with webpack.

But its actually two different files.

Page a need a.css

.body {
  margin: 0;
}
.content {
   flex: 1 0 auto;
}

Page b need b.css

.body {
  margin: 0;
}
.github {
   color: red
}

That's why going from pageA to pageB kinda override the same css.

lex111 commented 4 years ago

In fact, I am observing such a issue in reusable components.

I have a Showcase component that is used on the home page and on the Users page.

If I go to the first page first, and why go to the latter and look at devtools, I will see the following result:

image

endiliey commented 4 years ago

Let me put more investigation sometimes next week if i got the chance. I still suspect this is because all of that routes is code-splitted though

endiliey commented 4 years ago

I did an investigation. So the assumption was right, our best solution could possibly be grouping all the css file only into one file. Otherwise, code-splitting CSS will result all of above.

I just checked Gatsby https://github.com/gatsbyjs/gatsby/blob/67d8b6be93bb4971dadf6aa8e305af2b53560155/packages/gatsby/src/utils/webpack.config.js#L494-L506 and Vuepress https://github.com/vuejs/vuepress/blob/d2fef5d8f6f00aca13b5594c6084a54e1c8c3b18/packages/%40vuepress/core/lib/node/webpack/createBaseConfig.js#L275-L291

They all created one css file only 😢

A very particular interesting stuff is that code-splitting css might cause issue on ordering image

If we don't want to do this, there's no good solution to fix this problem :) (yet)

yangshun commented 4 years ago

Yeah I agree. CSS is usually small in comparison to JS and it's ok to not split it.