create-guten-block icon indicating copy to clipboard operation
create-guten-block copied to clipboard

Duplicate CSS is being compiled from common.css.

Open mikemcalister opened this issue 6 years ago • 18 comments

Bug Report

Expected Behavior

Styles added to the common.scss file should be compiled down to blocks.style.build.css and only show once instance of each style. There should be no duplicate css or comments in blocks.style.build.css.

Actual Behavior

The common.scss file is compiling multiple instances of styles and comments into blocks.style.build.css. See our common.scss file here, and see the compiled duplicate code here. It seems that styles/comments are being duplicated for each registered block. If you have 11 blocks, it shows up 11 times.

I can also confirm this in other plugins using CGB to generate blocks.

Steps to Reproduce

  1. Create multiple blocks.
  2. Add styles to common.scss and save.
  3. Build/compile the common.scss file and check out blocks.style.build.css.

Reproducible Demo

You can check out the issue in our Atomic Blocks plugin.

mikemcalister avatar Dec 04 '18 19:12 mikemcalister

That's super weird. I'm open to receive a PR for this. On holidays at the moment.

ahmadawais avatar Dec 08 '18 12:12 ahmadawais

I'm seeing this too. It's bringing my devTools to a crawl when I inspect a block because I have 23 cgb blocks on my current dev site, so every rule in my common.scss file is duplicated 23 times when parsing through the computed styles. I haven't had a chance to eject and look at the config, but I think the solution is probably using the data object in the sass-loader options for the common.scss file.

8bit-echo avatar Dec 11 '18 22:12 8bit-echo

@8bit-echo that's exactly what I am doing here

https://github.com/ahmadawais/create-guten-block/blob/master/packages/cgb-scripts/config/webpack.config.dev.js#L65

ahmadawais avatar Dec 11 '18 23:12 ahmadawais

I'm having the same issue. Using the latest version.

gregbia avatar Dec 26 '18 12:12 gregbia

After doing a bit of research this behavior makes sense under the current implementation. the data object is injecting the @import xxx line at the beginning of every component's stylesheet and therefore all the rules in common.scss are in-fact duplicated. Maybe what needs to happen instead is to use sass-resource-loader instead. This is how the Vue-CLI is currently solving this same problem. I'll see if I can do some testing on that this week and report back.

8bit-echo avatar Jan 02 '19 22:01 8bit-echo

On vacations so happy to accept a PR. Didn't get time to investigate further. @8bit-echo your comment makes sense. Send a PR if you happen to solve the issue. Just use an ejected state to change configuration until it makes sense. đź‘Ť

ahmadawais avatar Jan 04 '19 07:01 ahmadawais

Took a look at this today, but unfortunately the behavior with my solution is the same. @mikemcalister, it seems that the problem is user error on our parts. Both sass-loader and sass-resource-loader state explicitly that common files should not contain any renderable css declarations. Looks like the best course of action is to convert any actual css rules inside common.scss to mixins and variables, or move the global styles into the theme instead.

@ahmadawais I think we're good to close this issue.

8bit-echo avatar Jan 08 '19 00:01 8bit-echo

I am still getting duplicate CSS regardless. This seems like a serious issue.

gregbia avatar Jan 17 '19 10:01 gregbia

It does seem that this issue is primarily a user error, although perhaps more clear documentation would help alleviate the problem a little? I found my stylesheets ballooning, and it was difficult to find out the source of the problem. The issue is that common.scss is evidently not meant to be imported into the front end and admin (Gutenberg), but rather should only import partials, and variables, in particular.

So, the solution is to clean out the common.scss and leave only things that support the other block-specific partials (as @8bit-echo mentioned).

bassplayer7 avatar Feb 24 '19 02:02 bassplayer7

Any new ideas? I'd rather be able to use the common.scss file as a container for code that needs to be shared among several blocks.

rodrigodagostino avatar Jun 10 '19 06:06 rodrigodagostino

I disagree with the sentiment that this is user error. A file called common.scss implies that it contains shared styles, not just shared variables, mixins, and functions. It that is indeed its purpose, it should be renamed to something more fitting, and the comments in the file should warn against outputting CSS.

Also, I am not seeing a solution for creating shared CSS that applies to both the front end and back end. The theme CSS may contain styles that don't apply to the back end (e.g. resets), and trying to export everything into variables and mixins, while being cumbersome, creates problems on the front end if your theme CSS uses those shared styles as well.

Can we have an actual common.scss that doesn't duplicate styles?

thomashigginbotham avatar Nov 04 '19 18:11 thomashigginbotham

@thomashigginbotham I completely agree on every single point you raised up. In fact, I've been putting a lot of thought on this subject these last few days, and I got to the point were I realised that the current SCSS/CSS structure does not work for me at all. The author seems to be kind of indifferent to these “special needs”, so I will be probably be switching to wp-scripts and develop my own SCSS compiling solution.

rodrigodagostino avatar Nov 04 '19 19:11 rodrigodagostino

We started moving away from this tool for a few reasons, and in the interim we worked around the common styles issue by creating a "block" that was just for common styles. Most everything in this toolkit is outdated now. See webpack version as an example.

mindctrl avatar Nov 04 '19 19:11 mindctrl

@mindctrl you can help update this tool. I have a family to support and just doing open source work for free doesn't help at all. I have received almost negligible help in the maintenance or improvement of create-guten-block, and a couple hundred dollars in donation. That's not a sustainable model.

ahmadawais avatar Nov 04 '19 19:11 ahmadawais

@ahmadawais I understand that. I'm not bagging on you about it. Life is real, interests change. Just stating the facts. I looked at contributing by updating a bunch of dependencies, but as you know that turned into a big rabbit hole. Instead of doing that, I just switched to wp scripts for the same reasons you don't spend your time here... family and life. Maybe it's a good time to shut it down?

mindctrl avatar Nov 04 '19 21:11 mindctrl

I want to change it to a more dependable model, it is just going to be a huge change for everyone who depends on this. So, that's something to be taken care of.

ahmadawais avatar Nov 04 '19 21:11 ahmadawais

Here's what I did as a workaround for creating CSS common to all blocks:

  1. Create a shared folder (e.g. /src/shared).
  2. Add a file named editor.scss to the folder, and add your common backend style declarations to it.
  3. Add a file named style.scss to the folder, and add your common frontend and backend style declarations to it.
  4. Add a file named style-imports.js to the folder, and import the two Sass files (e.g. import './editor.scss'; import './style.scss';)
  5. Open /src/blocks.js, and import the style-imports.js file (e.g. import './shared/style-imports.js';).

Basically, what this does is cause Webpack to treat the shared folder as a new block, but because it never gets registered, only the CSS from it gets applied.

thomashigginbotham avatar Nov 20 '19 19:11 thomashigginbotham

@thomashigginbotham Thanks so much, you just eradicated a ton of bloat for me and even fixed some IE11 display issues to boot. Looks like it was choking and getting confused by all the repetition.

jondewitt avatar Nov 25 '19 19:11 jondewitt