gatsbyjs / gatsby

The best React-based framework with performance, scalability and security built in.
https://www.gatsbyjs.com
MIT License
55.27k stars 10.32k forks source link

In development, all CSS from a site is pulled in on every page due to the lack of code splitting #26434

Closed pcolmer closed 3 years ago

pcolmer commented 4 years ago

Description

I've encountered two CSS-related oddities that occur when using gatsby develop and not gatsby build. This seems to be (partly) related to #9911 but that issue remains closed so I'm re-reporting the problems here.

Problem 1: a page displayed under gatsby develop shows a bootstrap spinner but, under gatsby build, it does not.

Problem 2: a page displayed under gatsby develop changes colours when a completely unrelated line in a completely unrelated file is commented out & the file saved.

Steps to reproduce

Both problems can be reproduced with https://github.com/pcolmer/gatsby-bug-poc. After cloning the repo, you need to mkdir src/markdownpages as the directory is missing from the repo and I can't get it added to the repo.

Problem 1:

  1. gatsby develop
  2. Go to http://localhost:8000/ - you see a fairly bland page.
  3. Go to http://localhost:8000/test - you see a bootstrap spinner in the middle of the top of the page.
  4. Ctrl+C to stop Gatsby.
  5. gatsby build
  6. gatsby serve -p 8000

Repeat steps 2 & 3. Step 2 should look the same. For step 3, you should see the text "Loading..." in the top left corner of the page, because the bootstrap CSS isn't getting loaded.

Problem 2:

  1. Switch to the branch "possible-fix".
  2. gatsby develop
  3. Go to http://localhost:8000/

Notice that I've added a nav bar and that the background is blue.

Open components/loading.js, comment out line 2 (import "../styles/minimal.scss") and save it. Gatsby rebuilds the site ... and the nav bar's background changes to green, which is the colour it should be.

Expected result

For problem 1, the spinner should never be displayed because the page is not loading the bootstrap CSS. The branch possible-fix applies a CSS change to the test page to correct that error. However, the fact that the spinner displays under gatsby develop misleads the developer into thinking that everything is OK.

For problem 2, the banner should be green.

Actual result

For problem 1, the spinner is displayed during gatsby develop but not during gatsby build/gatsby serve

For problem 2, the banner is blue until a CSS-related line in an unrelated file is commented out.

Environment

  System:
    OS: Linux 4.19 Ubuntu 20.04.1 LTS (Focal Fossa)
    CPU: (4) x64 Intel(R) Core(TM) i5-6300U CPU @ 2.40GHz
    Shell: 5.0.17 - /bin/bash
  Binaries:
    Node: 12.18.1 - ~/.nvm/versions/node/v12.18.1/bin/node
    npm: 6.14.7 - ~/.nvm/versions/node/v12.18.1/bin/npm
  Languages:
    Python: 3.8.2 - /usr/bin/python
  npmPackages:
    gatsby: ^2.24.47 => 2.24.47
    gatsby-cli: ^2.12.87 => 2.12.87
    gatsby-plugin-react-helmet: ^3.3.10 => 3.3.10
    gatsby-plugin-react-helmet-canonical-urls: ^1.4.0 => 1.4.0
    gatsby-plugin-sass: ^2.3.12 => 2.3.12
    gatsby-source-filesystem: ^2.3.19 => 2.3.24
    gatsby-transformer-remark: ^2.8.25 => 2.8.28
  npmGlobalPackages:
    gatsby-cli: 2.12.84
pcolmer commented 4 years ago

I've done some further digging into this ... the problem where the spinner appears in develop but not in build seems to be down to the retrieval of blob:http://localhost:8000/1083e78e-a3fd-43c4-86dc-3f57e39fa4d1 which seems to be a mega-CSS bundle provided by gatsby develop, and it includes the bootstrap CSS even though the test page isn't retrieving it.

With the colour changing problem, there are two blobs getting pulled in and the second blob is the one that defines the blue colour instead of the green colour. greping the code for the colour references shows that the green (correct) colour is defined in src/styles/theme.scss while the blue (incorrect) colour is defined in node_modules/bootstrap/dist/css/bootstrap.css confirming that, again, gatsby develop appears to be pulling in all CSS files it can find, even if they aren't referenced.

github-actions[bot] commented 4 years ago

Hiya!

This issue has gone quiet. Spooky quiet. πŸ‘»

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! πŸ’ͺπŸ’œ

pcolmer commented 4 years ago

I definitely do want to keep this open until it is resolved.

I've created clear repro steps that show distinct differences in the behaviour between gatsby develop and gatsby build and it is very frustrating that there are these differences. It makes it really hard to figure out what to do to fix this when, ultimately, gatsby itself is getting in my way.

hayyaun commented 4 years ago

In my case, gatsby-plugin-minify was making this problem, which led the production build to reload all styles after a page was loaded completely in the production build.

github-actions[bot] commented 4 years ago

Hiya!

This issue has gone quiet. Spooky quiet. πŸ‘»

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! πŸ’ͺπŸ’œ

pcolmer commented 4 years ago

Time for a copy/paste of my reply from September 6th.

I definitely do want to keep this open until it is resolved.

I've created clear repro steps that show distinct differences in the behaviour between gatsby develop and gatsby build and it is very frustrating that there are these differences. It makes it really hard to figure out what to do to fix this when, ultimately, gatsby itself is getting in my way.

pcolmer commented 4 years ago

Just a heads-up to the Gatsby team: I have no intention of letting this issue die through inactivity. I've taken the time and trouble to provide a simple reproduction of the problem. The problem is clearly within Gatsby itself.

THIS NEEDS FIXING!

faridsaud commented 4 years ago

similar issue as @pcolmer

github-actions[bot] commented 3 years ago

Hiya!

This issue has gone quiet. Spooky quiet. πŸ‘»

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! πŸ’ͺπŸ’œ

pcolmer commented 3 years ago

So doesn't the Gatsby team care about issues with clear reproduction steps?

Is nobody on the team even looking at this?

github-actions[bot] commented 3 years ago

Hiya!

This issue has gone quiet. Spooky quiet. πŸ‘»

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! πŸ’ͺπŸ’œ

CanRau commented 3 years ago

Please keep open 😌

github-actions[bot] commented 3 years ago

Hiya!

This issue has gone quiet. Spooky quiet. πŸ‘»

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! πŸ’ͺπŸ’œ

pcolmer commented 3 years ago

I'm still here, waiting for someone from the Gatsby team to chime in ...

ventocis commented 3 years ago

@pcolmer I was having the same issue. I wasn't using the gatsby-browser.js file. Creating that file and importing my css solved the issue for me.

pcolmer commented 3 years ago

@ventocis thank you for the suggestion, but the POC repo does have that file in it:

https://github.com/pcolmer/gatsby-bug-poc/blob/master/gatsby-browser.js

My concern is that I don't understand why gatsby is behaving differently between the development and build phases of a project.

ventocis commented 3 years ago

Sorry I should have checked that. Very good point about the inconsistency

github-actions[bot] commented 3 years ago

Hiya!

This issue has gone quiet. Spooky quiet. πŸ‘»

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! πŸ’ͺπŸ’œ

pcolmer commented 3 years ago

Still here. Still waiting for the Gatsby team to chime in.

attila-kun commented 3 years ago

I too have encountered what you noted as "Problem 2". It is very easy to reproduce even with a barebones project, it takes 4 lines (!) in total under the src folder: https://github.com/attila-kun/gatsby-dev-vs-prod

Reproduction steps can be found in the project's README.

The problem seems to come from the commons.css file generated in develop mode (one can see this being downloaded under Chrome DevTools' "Network" tab). This seems to contain the CSS for ALL imports, not just the imports of the currently viewed page.

pcolmer commented 3 years ago

@attila-kun Thank you for sharing that. It completely fits what I've seen and I really appreciate you providing another reproductive recipe.

Hopefully the Gatsby team will find some time to look at this sooner rather than later. In the meantime, I'm having to look at other frameworks because the difference between develop & production in Gatsby is just too hard to manage. Gatsby's loss.

attila-kun commented 3 years ago

@pcolmer I reached a similar conclusion. The fact that this issue is reproducible without any exotic dependencies in a "hello world"-level project makes me worried that I'd be venturing onto a minefield by choosing Gatsby in its present form.

I am willing to revisit Gatsby once it has reached a higher level of maturity but for now I'll be looking at 11ty, in the hopes that it will have fewer "surprises".

KyleAMathews commented 3 years ago

I checked out the reproduction. It's setting a background-color on the div element. I'm not sure this is a bug though. The front page div is still colored red even in production if you first visit /red/ and then navigate to the home page. That in develop you always see this is I'd argue a feature not a bug as it's more confusing to only see strange colors pop up depending on what order you navigate the site.

This issue is endemic to CSS that CSS you write on one page can easily affect other pages β€” which is why we highly recommend you use CSS Modules or css-in-js or other techniques which automatically prevent this issue from happening. If you ported this code to 11ty or any other system, it'd show the same problem.

See https://www.gatsbyjs.com/docs/how-to/styling/css-modules/ for more. See this commit to see how to port the reproduction to use CSS Modules which now works the way you'd hope: https://github.com/KyleAMathews/gatsby-dev-vs-prod/commit/25a2d5525f9d488da9c183068bfceeaf15116d0e

attila-kun commented 3 years ago

@KyleAMathews:

I checked out the reproduction. It's setting a background-color on the div element. I'm not sure this is a bug though. The front page div is still colored red even in production if you first visit /red/ and then navigate to the home page.

This is not what I see, no matter what order I'm visiting the pages. Please visit this first: https://attila-kun.github.io/gatsby-dev-vs-prod/public/red/ Then this: https://attila-kun.github.io/gatsby-dev-vs-prod/public/

Both links are pointing to the prod build's output. You can see that the second link's div is white. In fact, if you look at the page source of the two links, you can find the string background-color:red in the first one, but not in the second.

KyleAMathews commented 3 years ago

You need to do a client-side route change. Otherwise the full reload won't show the overlap. Gatsby sites are SPAs which has some wrinkles that are different than traditional sites.

On Sat, Jan 16, 2021, 2:29 PM Attila Kun notifications@github.com wrote:

@KyleAMathews https://github.com/KyleAMathews:

I checked out the reproduction. It's setting a background-color on the div element. I'm not sure this is a bug though. The front page div is still colored red even in production if you first visit /red/ and then navigate to the home page.

This is not what I see, no matter what order I'm visiting the pages. Please visit this first: https://attila-kun.github.io/gatsby-dev-vs-prod/public/red/ Then this: https://attila-kun.github.io/gatsby-dev-vs-prod/public/

Both links are pointing to the prod build's output. You can see that the second link's div is white. In fact, if you look at the page source of the two links, you can find the string background-color:red in the first one, but not in the second.

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/gatsbyjs/gatsby/issues/26434#issuecomment-761690087, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAARLBYGO7TX3VZQUQJFZZTS2IHOHANCNFSM4P56XWIQ .

attila-kun commented 3 years ago

@KyleAMathews: I think there are at least 2 possible ways to make the developer experience more consistent between develop and production:

  1. If client-side routing is a requirement for Gatsby to work correctly, then stand-alone html pages should not be generated by the build. Otherwise Gatsby's users will naively assume (like I did) that those pages can be served as-is.
  2. It should be ensured that navigating to a page will only pull in CSS which is imported from that page. I'm in favour of this option as it is very natural to assume that different pages' imports don't interfere (which is currently not the case).
pcolmer commented 3 years ago

@KyleAMathews

I checked out the reproduction.

Which reproduction? I suspect not the one that I provided when I created this issue because, although I do reference a colour issue, I don't use the red colour.

which is why we highly recommend you use CSS Modules or css-in-js or other techniques which automatically prevent this issue from happening.

How does that work when you are using third-party code like the spinner? How do I correctly ensure that the spinner's CSS is pulled in when it is referenced the same way as any other node object?

I am very unhappy that this issue has been closed. If you truly believe that my reproduction has flaws in it, please point out to me where it needs to be fixed. Otherwise, please accept that the reproduction shows a problem with Gatsby that needs to be fixed and reopen this issue.

KyleAMathews commented 3 years ago

@pcolmer my apologies. I was just looking at the emails coming in from this issue so saw @attila-kun's recent reproduction not yours in the original comment.

I'll re-open this as it is a different issue than @attila-kun's reproduction and does need solved still.

Thank for your the nice reproduction. The problem is basically we don't do page-level code splitting in development like we do in production. Which means that in production, each page only pulls in the css used on that page while in development, we pull in all css. This needs fixed https://github.com/gatsbyjs/gatsby/issues/8342

github-actions[bot] commented 3 years ago

Hiya!

This issue has gone quiet. Spooky quiet. πŸ‘»

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! πŸ’ͺπŸ’œ

pcolmer commented 3 years ago

It still needs solving ...

github-actions[bot] commented 3 years ago

Hiya!

This issue has gone quiet. Spooky quiet. πŸ‘»

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! πŸ’ͺπŸ’œ

pcolmer commented 3 years ago

@KyleAMathews now that it has been acknowledged that this issue needs to be fixed, is it really necessary for the bot to regularly mark this as a stale issue?

I will keep on commenting in order to stop the issue being closed but it just adds to my level of frustration ...

github-actions[bot] commented 3 years ago

Hiya!

This issue has gone quiet. Spooky quiet. πŸ‘»

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! πŸ’ͺπŸ’œ

theskillwithin commented 3 years ago

beep boop

github-actions[bot] commented 3 years ago

Hiya!

This issue has gone quiet. Spooky quiet. πŸ‘»

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! πŸ’ͺπŸ’œ

github-actions[bot] commented 3 years ago

Hey again!

It’s been 60 days since anything happened on this issue, so our friendly neighborhood robot (that’s me!) is going to close it. Please keep in mind that I’m only a robot, so if I’ve closed this issue in error, I’m HUMAN_EMOTION_SORRY. Please feel free to comment on this issue or create a new one if you need anything else. As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks again for being part of the Gatsby community! πŸ’ͺπŸ’œ

stamengeorgiev commented 3 years ago

So... it seems that the loading of all available css in dev mode is still here? I think that mentioning it in the documentation will be nice.