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.

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

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

Closed
lex111 opened this issue Nov 17, 2019 · 9 comments · Fixed by #2007
Closed

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

lex111 opened this issue Nov 17, 2019 · 9 comments · Fixed by #2007
Labels
bug An error in the Docusaurus core causing instability or issues with its execution help wanted Asking for outside help and/or contributions to this particular issue or PR.

Comments

@lex111
Copy link
Contributor

lex111 commented Nov 17, 2019

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

@lex111 lex111 added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers v2 labels Nov 17, 2019
@endiliey
Copy link
Contributor

endiliey commented Nov 17, 2019

Well, it's mostly because of code splitted css. example from

html,
body {
height: 100%;
}
body {
margin: 0;
padding-top: var(--ifm-navbar-height);
transition: var(--ifm-transition-fast) ease color;
}
body > div {
height: 100%;
display: flex;
flex-direction: column;
}

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

endiliey commented Nov 17, 2019

One solution is to move

html,
body {
height: 100%;
}
body {
margin: 0;
padding-top: var(--ifm-navbar-height);
transition: var(--ifm-transition-fast) ease color;
}
body > div {
height: 100%;
display: flex;
flex-direction: column;
}
to main bundle.

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

@yangshun
Copy link
Contributor

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

endiliey commented Nov 17, 2019

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

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

lex111 commented Nov 17, 2019

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

endiliey commented Nov 17, 2019

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 endiliey added bug An error in the Docusaurus core causing instability or issues with its execution help wanted Asking for outside help and/or contributions to this particular issue or PR. and removed bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Nov 17, 2019
@endiliey
Copy link
Contributor

endiliey commented Nov 17, 2019

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution help wanted Asking for outside help and/or contributions to this particular issue or PR.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants