-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
fix(gatsby-plugin-styled-components): fix global styles pollution #9943
fix(gatsby-plugin-styled-components): fix global styles pollution #9943
Conversation
2ad67b7
to
c5b2562
Compare
…tsbyjs#9922) Fix gatsbyjs#9922 `ServerStyleSheet` should be specific to each pages in order to not pollutes the other. Theses changes removes the "global" stylesheet and use the `pathname` for identify each pages.
c5b2562
to
a9f4b23
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @LoicMahieu!
Validated locally, and did small change to use Map
instead of object and delete sheet once it's no longer needed (to not block memory). Will merge as soon as tests passes again
Holy buckets, @LoicMahieu — we just merged your PR to Gatsby! 💪💜 Gatsby is built by awesome people like you. Let us say “thanks” in two ways:
If there’s anything we can do to help, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’. Thanks again! |
Published |
@LoicMahieu @pieh release 3.0.2 breaks release 3.0.1 In 3.0.1 I build my site, each html page has some amount of style tags being injected directly in the head which allows for a nice flash-less Update to 3.0.2 I started noticing some styles flashes in the pages in the built site meaning that the styles are being loaded after the page has rendered. I opened the html files, there are no styles inlined in the head anymore thus causing the style flash. Downgrading to 3.0.1 fixed the problem. Suggesting a revert of this PR and re-release as it introduced major breaking changes. |
@maxmx can you share your repo or create minimal reproduction repo for this? |
Not in front of a computer atm so I can’t make a minimal repro but here is the repo https://github.com/maxmx/mobilo Build to see styles in each html files, then update to 3.0.2 to trigger the bug |
@maxmx Hey, so I cloned your repo and updated |
@pieh I created a branch where I pushed the build. https://github.com/maxmx/mobilo/compare/Build-Bug?expand=1 first commit is built with 3.0.1 second commit is built with 3.0.2 Open the diff of any html file in the second commit to notice what was removed by upgrading to 3.0.2 i.e. All the styled-components generated styles have been removed. This means that the page is not AMP compatible anymore since there are not more inlined styles in the head. I think the assumption behind #9922 may be wrong. You do need every global styles injected in every html files since any of those files can serve as an entry point to the application. To get the instant load feeling you need all the styles to be inlined in the head. The rest of the styles as you navigate can then be injected by javascript. You could http-serve the public folder in 3.0.2 and disable javascript in chrome settings, you'll see that now all the styles are not rendered because they rely on the javascript injection. |
@maxmx I finally find out why I couldn't reproduce - you need to update |
… API hooks (#10045) Those plugins rely on changes introduced in #9401 which was released in `gatsby@2.0.32` Ref: #9943 (comment)
Great thanks @pieh ! |
…tsbyjs#9943) Fix gatsbyjs#9922 `ServerStyleSheet` should be specific to each pages in order to not pollutes the other. Theses changes removes the "global" stylesheet and use the `pathname` for identify each pages. --- Just a quick note for my first PR here: thanks to every contributors for this fantastic project! 👍
… API hooks (gatsbyjs#10045) Those plugins rely on changes introduced in gatsbyjs#9401 which was released in `gatsby@2.0.32` Ref: gatsbyjs#9943 (comment)
Fix #9922
ServerStyleSheet
should be specific to each pages in order to not pollutes the other. Theses changes removes the "global" stylesheet and use thepathname
for identify each pages.Just a quick note for my first PR here: thanks to every contributors for this fantastic project! 👍