gatsbyjs / gatsby

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

How to merge media queries with same conditions? #20133

Closed in-in closed 4 years ago

in-in commented 4 years ago

Summary

I use gatsby-plugin-sass and nested media queries for each CSS class.

.header {
  background-color: rebeccapurple;

  @media (min-width: 480px) {
    background-color: seagreen;
  }

  @media (min-width: 640px) {
    background-color: purple;
  }

  @media (min-width: 960px) {
    background-color: slateblue;
  }
}

After the npm run build command, my production CSS file is bloated because there are a lot of extra media queries. In an ideal world, I want all the rules relating to the same media queries to be within that query and not duplicated.

I tried to use postcss-combine-media-query plugin that was supposed to help me to merge all the media queries, but that didn't happen. I do not know exactly why, it seems that this plugin does not have access to the final CSS file.

Relevant information

Demo project: https://github.com/in-in/gatsby-starter-default

Environment (if relevant)

Environment System:
OS: Linux 4.15 Linux Mint 19.2 (Tina)
CPU: (4) x64 Intel(R) Core(TM) i5-4670 CPU @ 3.40GHz
Shell: 5.4.2 - /usr/bin/zsh
Binaries:
Node: 12.13.1 - ~/.nvm/versions/node/v12.13.1/bin/node
npm: 6.12.1 - ~/.nvm/versions/node/v12.13.1/bin/npm
Languages:
Python: 2.7.15+ - /usr/bin/python
Browsers:
Firefox: 72.0b6
npmPackages:
gatsby: ^2.18.8 => 2.18.11
gatsby-image: ^2.2.34 => 2.2.36
gatsby-plugin-manifest: ^2.2.31 => 2.2.33
gatsby-plugin-offline: ^3.0.27 => 3.0.29
gatsby-plugin-react-helmet: ^3.1.16 => 3.1.18
gatsby-plugin-sass: ^2.1.26 => 2.1.26
gatsby-plugin-sharp: ^2.3.5 => 2.3.7
gatsby-source-filesystem: ^2.1.40 => 2.1.42
gatsby-transformer-sharp: ^2.3.7 => 2.3.9

File contents (if changed)

gatsby-config.js:

...
{
      resolve: 'gatsby-plugin-sass',
      options: {
        implementation: require('sass'),
        includePaths: ['./src'],
        cssLoaderOptions: {
          localIdentName: '[local]--[hash:base64:5]',
          camelCase: false,
        },
        postCssPlugins: [
          require('postcss-logical'),
          require('postcss-combine-media-query'),
        ]
      },
    },

package.json: N/A gatsby-node.js: N/A gatsby-browser.js:

import("./src/styles/index.scss")

gatsby-ssr.js: N/A

LekoArts commented 4 years ago

Thank you for opening this!

Please reach out to the author of this PostCSS plugin as it might be the case that your code shown above is not supported - since it doesn't show nested queries in its README: https://github.com/SassNinja/postcss-combine-media-query

Feel free to comment here if you or you both can verify that it should work nevertheless and you're sure that it's a bug in Gatsby.

We're marking this issue as answered and closing it for now but please feel free to comment here if you would like to continue this discussion. We also recommend heading over to our communities if you have questions that are not bug reports or feature requests. We hope we managed to help and thank you for using Gatsby!

in-in commented 4 years ago

It's obvious that the plugin doesn't have nested queries problems because it works with AST.

Moreover, I double-checked my code by rewriting it into a flat structure.

.header {
  background-color: rebeccapurple;
}

@media (min-width: 480px) {
  .header {
    background-color: seagreen;
  }
}

@media (min-width: 640px) {
  .header {
    background-color: purple;
  }
}

@media (min-width: 960px) {
  .header {
    background-color: slateblue;
  }
}

The result is the same - an bloated final CSS file.

And the main problem is not in the plugin but in the fact that Gatsby doesn't have any opportunity to remove unnecessary CSS code.

Feel free to keep this issue closed :+1: Because it's the easiest way to solve this problem.

LekoArts commented 4 years ago

And the main problem is not in the plugin but in the fact that Gatsby doesn't have any opportunity to remove unnecessary CSS code.

I don't think that's something that Gatsby needs to do, the respective tools is responsible for that - in your case the PostCSS plugin.

My point is still valid, please reach out to the author as he has the most context on this and other PostCSS plugins work without problems. I myself don't have enough context on the underlying PostCSS config so I can't help with that, sorry.

wardpeet commented 4 years ago

Hey @in-in.

Sadly, this is not something we can do by default in Gatsby and is like @LekoArts said, not a gatsby concern. Currently, we run postcss on all css files separately as you mentioned. We run cssnano on the combined bundle, which does all safe optimizations for you.

The behavior you want could be an unsafe transform as ordering is critical in CSS so relying on a plugin to merge these queries could give you some unexpected results.

More info why this is a rule you better not enable on a full css file can be found below:

You can still run postcss-cli on a postbuild step that does these transforms for you.

I'll be closing this as I think I answered this correctly.