gatsby-plugin-postcss #6217

mdreizin · 2018-06-28T20:31:10Z

Add migration guide from v1 to v2
Add more examples how to use it
Add @KyleAMathews as an owner on NPM

gatsbybot · 2018-06-28T20:37:14Z

Deploy preview for using-drupal ready!

Built with commit 72060e6

https://deploy-preview-6217--using-drupal.netlify.com

gatsbybot · 2018-06-28T20:38:06Z

Deploy preview for gatsbygram ready!

Built with commit 72060e6

https://deploy-preview-6217--gatsbygram.netlify.com

mdreizin · 2018-07-02T22:21:27Z

@KyleAMathews Did you have a chance to look at this PR?

Maybe, you have some comments and ideas and I'm open to discussion :)

mdreizin · 2018-07-02T22:28:10Z

Is it reasonable to remove postcss related code from gatsby core and ship it as a separate plugin?

For instance, add migration guide to use this plugin and add missing plugins (flexbugs, autoprefixer).

m-allanson · 2018-07-03T13:31:23Z

This looks good!

Is it reasonable to remove postcss related code from gatsby core and ship it as a separate plugin?

This appeals to me, but from a beginner perspective I think we should keep autoprefixer in Gatsby core. This is something Create React App does too and seems like a pretty sensible default. I don't have any strong thoughts on keeping the flexbugs plugin though.

It looks like enabling this plugin will disable any existing PostCSS config? That seems like a good approach too. Enabling gatsby-plugin-postcss will remove Gatsby's default autoprefixer config and require you to configure autoprefixer in the way that you want.

jquense · 2018-07-03T17:04:38Z

It looks like enabling this plugin will disable any existing PostCSS config? That seems like a good approach too. Enabling gatsby-plugin-postcss will remove Gatsby's default autoprefixer config and require you to configure autoprefixer in the way that you want.

I don't think this is a good idea, that will remove autoprefixing from folks using sass or less as well. We chatted a bit about this in the PostCSS config PR, Postcss is used in two different capacities, this one is as a preprocesser replacement, and others as tooling around browser compat.

It may be fine to remove the flexbugs (its a bit prescriptive) plugin from the default, but i don't think autoprefixer should be removed :)

jquense · 2018-07-03T17:09:09Z

packages/gatsby-plugin-postcss/src/gatsby-node.js

+  const isSSR = stage.includes(`html`)
+  const originalConfig = getConfig()
+
+  originalConfig.module.rules = originalConfig.module.rules.filter(rule => {


this would be unexpected behavior to me, plguins should be additive here I think, I would really dislike if if this plugin removed my loaders elsewhere, especially since this doesn't always replace it with something else.

e.g. if I have gatsby-plugin-sass installed with a postcss config meant to augment my sass workflow, this plugin would remove that, and then replace it with a loader that doesn't handle .scss files.

at the very least this should only target the gatsby css-loader

@jquense Great catch, I will fix that.

mdreizin · 2018-07-03T20:32:54Z

Are you agree to have postcss field under options?

I'm just asking, because now we can't use the followings:

module.exports = {
  plugins: [
    {
      resolve: `gatsby-plugin-postcss`,
      options: {
        plugins: () => [require(`postcss-preset-env`)({ stage: 0 })]
      },
    },
  ],
}

due to limitation of load-plugins.

jquense · 2018-07-03T20:51:04Z

maybe postcssPlugins to be really clear?

madeleineostoja · 2018-07-03T23:01:40Z

I don't think this is a good idea, that will remove autoprefixing from folks using sass or less as well

Only if someone is using both the postcss and sass/less plugin though? In which case it'ss fairly straightforward – if you use the postcss plugin (in any capacity), then you configure your own autoprefixer, doesn't matter what other plugins you also use.

ooloth · 2018-07-05T08:41:46Z

I agree with removing the default autoprefixer plugin when using the postcss plugin. I’d like to be able to control what version of autoprefixer is being used and what settings are passed to it (e.g. turning grid on/off), so removing the default configuration seems like the cleanest approach.

As @seaneking points out, this would only affect those using the postcss plugin and its docs could mention the clean slate and the need to add your own autoprefixer configuration.

mdreizin · 2018-07-08T21:10:25Z

@jquense did you have a chance to review it?

jquense · 2018-07-08T21:35:56Z

There is no need to remove the auto prefixing, adding this plugin will already override the default css handling since it will run before it. All I'm concerned with is that this plugin doesn't break behavior for unrelated other plugins 👍 since that is both unexpected and likely to lead to bugs

jquense

Looking great, just a few notes!

jquense · 2018-07-08T21:39:37Z

packages/gatsby-plugin-postcss/src/gatsby-node.js

+  return options
+}
+
+const removeBuiltInCssLoaders = config => {


There shouldn't be a need for this. This plugin will already intercept all css files and run the through producing the extracting file or tye inlined style before the built-in rule can run. I'd say trying to not mess with the config is safer long term and in more use cases than effectively guessing which loader is the built in one

@jquense I completely agree with you.

Unfortunately, if we have i.e. two rules with the same test matchers they will be applied in the order they are defined in module.rules:

module: { rules: [{ test: /\.css$/, use: [] }, { test: /\.css$/, use: [] }] }

If we don't remove them we will get:

Error: ./src/components/layout/index.css Module build failed (from ./node_modules/mini-css-extract-plugin/dist/loader.js): ModuleBuildError: Module build failed (from ./node_modules/mini-css-extract-plugin/dist/l oader.js): Error: Didn't get a result from child compiler

Now we have a couple of options:

Remove existing css rules manually

Remove existing css rules from the core completely and tell users to use that plugin

Remove existing css rules from the core and move their defaults into that plugin

Hmm loaders shouldn't be run in order of defining, but in reverse order (see https://stackoverflow.com/questions/32234329/what-is-the-loader-order-for-webpack) I'd like to take a moment to understand what's happening let me try and do some debugging

It looks like Gatsby is always (?) using oneOf for handling CSS, to prevent *.module.css files from matching both module and non-module CSS rules. Would it be possible to prepend these new rules before the existing ones? The end result would look like:

oneOf: [ postcssModuleRule, postcssRule, rules.cssModules(), rules.css() ]

This way, the default rules would never match anyway, because the prepended ones use the same regexes.

@szimek I tried your approach and it worked fine. Thanks! I will publish an update.

@jquense What do you think about an approach suggested by @szimek? I have already used it and it would be nice if you could review it.

I think this is a good approach for now 👍

jquense · 2018-07-08T21:40:13Z

packages/gatsby-plugin-postcss/src/gatsby-node.js

+  { actions, stage, loaders, getConfig },
+  pluginOptions
+) => {
+  const isProduction = stage !== `develop`


There are two develop stages, develop and develop-html

szimek · 2018-07-16T21:16:29Z

@mdreizin It works! Thank you!

mdreizin · 2018-07-17T07:44:47Z

@jquense @szimek We could switch to use .pcss extension and remove complicated logic (prepending postcss rules).

In this case users have to rename all .css files to .pcss if they want to use this plugin.

What do you think about it?

madeleineostoja · 2018-07-17T07:51:59Z

Personally I'd vote strong no on .pcss extensions just to clean up implementation details. Hurts UX and is far from a common standard.

mdreizin · 2018-07-17T07:56:07Z

@seaneking I agree with you.

PS: It was just a thought came to my mind and I decided to share with you. It could help to simplify this plugin.

szimek · 2018-07-17T07:58:57Z

@mdreizin I'd prefer to keep using .css extension. The logic is somewhat complicated, but it works :) It doesn't modify existing loaders, so it doesn't depend that much on changes in the default Gatsby CSS rules.

mdreizin · 2018-07-17T18:47:05Z

@szimek @seaneking @jquense @m-allanson As I can see it is the final version and I would kindly ask you to review it again. If everything is ok let's merge it 🎉.

m-allanson

@mdreizin I've just been trying this out locally and it's looking great!

Could you add m-allanson and pieh as package maintainers on npm? Also kylemathews but I think you've done that already?

Once that's done, this can be merged and published 🎉.

Big thanks to everyone that's helped out with this. Now I'll be able to upgrade a couple of my sites to Gatsby v2 :)

m-allanson · 2018-07-19T14:42:59Z

Having another read through - I find the readme a bit confusing. Should the config go in postcss.config.js or gatsby-config.js?

Under the How to Use section it says "If you need to pass options to PostCSS use the plugins options; see postcss-loader for all available options." and then says that you should create a postcss.config.js file. But you should only do one of these things.

Later on there's another section about additional post processing. Is that separate from the previous section?

I'd maybe emphasise putting stuff in postcss.config.js, then say something like "alternatively, you can pass options directly to the Gatsby plugin in gatsby-config.js", followed by an example. What do you think?

mdreizin · 2018-07-19T15:48:35Z

@m-allanson I have just added you and also pieh as package maintainers (kylemathews is already added).

I will try to revise the readme and fix existing issues.

mdreizin · 2018-07-19T19:08:48Z

@m-allanson Done. It is ready for the review :)

KyleAMathews · 2018-07-19T19:10:51Z

Deploy preview for using-postcss-sass failed.

Built with commit 72060e6

https://app.netlify.com/sites/using-postcss-sass/deploys/5b50f06fb13fb17597b6528e

KyleAMathews · 2018-07-19T19:31:00Z

docs/docs/migrating-from-v1-to-v2.md

-Use [`onCreateWebpackConfig`](/docs/add-custom-webpack-config) to specify your postcss plugins.
-
-Note: there will be a `postcss` plugin that allows you to configure postcss from a standard postcss config file. [Follow this discussion on issue 3284](https://github.com/gatsbyjs/gatsby/issues/3284).
+Use [`gatsby-plugin-postcss`](https://github.com/gatsbyjs/gatsby/tree/master/packages/gatsby-plugin-postcss) to specify your particular PostCSS configuration.


Could you add the postcss configuration to replicate how Gatsby v1 is setup? That'll make it really easy to migrate. So the instructions for the npm install + the postcss.config.js file to add.

gatsby/packages/gatsby/src/utils/webpack.config.js

Line 454 in 389c829

postcss(wp) {

@KyleAMathews Yes, sure. Thanks for the suggestion.

@KyleAMathews Done. Could you please to review it again?

KyleAMathews

Amazing work @mdreizin! This is a huge contribution for adding a really key component to the ecosystem!

Merging and publish 🎉

KyleAMathews · 2018-07-19T20:16:55Z

Deploy preview for using-remark failed.

Built with commit 72060e6

https://app.netlify.com/sites/using-remark/deploys/5b50f070b13fb17597b6529a

KyleAMathews · 2018-07-19T20:16:56Z

Oh, as soon as TravisCI is happy that is :-)

gatsbot · 2018-07-19T20:36:38Z

Holy buckets, @mdreizin — we just merged your PR to Gatsby! 💪💜

Gatsby is built by awesome people like you. Let us say “thanks” in two ways:

We’d like to send you some Gatsby swag. As a token of our appreciation, you can go to the Gatsby Swag Store and log in with your GitHub account to get a coupon code good for one free piece of swag. (Currently we’ve got a couple t-shirts available, plus some socks that are really razzing our berries right now.)
We just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. You’ll receive an email shortly asking you to confirm. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

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!

KyleAMathews · 2018-07-19T20:36:49Z

💥

KyleAMathews · 2018-07-19T20:37:06Z

And definitely claim your free swag @mdreizin! You deserve 3 or 4!

mdreizin · 2018-07-19T20:45:54Z

@KyleAMathews Thanks a lot! It is my pleasure to contribute.

In case of free time I will try to help you guys with v2.

fardarter · 2018-07-24T14:30:44Z

Hi all. I'm trying to get this working with an existing build with file.module.scss. Am I correct in saying that this is supposed to replace "gatsby-plugin-sass"? If so, what .scss processor should I be using and what is the syntax for including it?

The only one I've been able to find is this: https://github.com/jonathantneal/postcss-sass

But the syntax doesn't really seem to match what is needed by the plugin array.

The thing I really want to get working is the pxtorem converter: https://github.com/cuth/postcss-pxtorem for pinch to zoom issues.

I'm quite new to this part of the ecosystem, so would appreciate some help.

mdreizin · 2018-07-24T20:18:58Z

@fardarter The main idea behind this plugin is to provide convenient way to process *.css files throught PostCSS and this is not a replacement for gatsby-plugin-sass. If you would like to use Sass you should to use gatsby-plugin-sass.

fardarter · 2018-07-25T16:38:59Z

What I'm really hoping for is a pipeline where I can pass SASS output through PostCSS.

…

On Tue, 24 Jul 2018 at 22:19, Marat Dreizin ***@***.***> wrote: @fardarter <https://github.com/fardarter> The main idea behind this plugin is to provide convenient way to process *.css files throught PostCSS and this is not a replacement for gatsby-plugin-sass. If you would like to use Sass you should to use gatsby-plugin-sass. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6217 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ALYpBW-qzmB1HJdQ_zXwIUTwf9ITq0KQks5uJ4E_gaJpZM4U8CCQ> .

feat: add postcss plugin

6356a59

mdreizin mentioned this pull request Jun 28, 2018

[v2] How to manually specify postcss plugins? #5778

Closed

mdreizin added 2 commits June 28, 2018 23:45

test: update obsolete snapshots

41f1f4f

chore(package): add postcss-loader dependency

6234e54

jquense reviewed Jul 3, 2018

View reviewed changes

fix: removes only .css and .module.css loaders

664385c

mdreizin added 2 commits July 4, 2018 00:19

refactor: use postcssPlugins instead of postcss

7c37ca5

refactor: use a new method to remove CSS loaders

99e4c37

docs: add general sections

ea8c8a7

mdreizin added 6 commits July 5, 2018 22:51

refactor: use existing naming convention

6b0abac

test: allow to mock resolve

0e5e286

fix: delete postCssPlugins property

9d46631

test: fix failed tests

0cb508d

docs: update migration guide

5966f7b

docs: how to use section

bfdafa1

mdreizin changed the title ~~WIP: gatsby-plugin-postcss~~ gatsby-plugin-postcss Jul 8, 2018

jquense reviewed Jul 8, 2018

View reviewed changes

chore: add resolve to .gitignore

3112f9c

docs: update PostCSS plugins section

05ea329

m-allanson previously approved these changes Jul 19, 2018

View reviewed changes

mdreizin added 2 commits July 19, 2018 22:00

docs: update migration guide

78c5211

docs: make documentation less confused

78a0946

mdreizin dismissed m-allanson’s stale review via 78a0946 July 19, 2018 19:07

Merge branch 'master' into postcss

42d2451

KyleAMathews reviewed Jul 19, 2018

View reviewed changes

docs: add detailed migration guide

72060e6

KyleAMathews approved these changes Jul 19, 2018

View reviewed changes

KyleAMathews merged commit 70b4cd7 into gatsbyjs:master Jul 19, 2018

m-allanson mentioned this pull request Jul 25, 2018

Feature Request: make Postcss plugins configurable, update dependancies and add to documentation #3284

Closed

gatsby-plugin-postcss #6217

gatsby-plugin-postcss #6217

Conversation

mdreizin commented Jun 28, 2018 • edited Loading

gatsbybot commented Jun 28, 2018 • edited Loading

gatsbybot commented Jun 28, 2018 • edited Loading

mdreizin commented Jul 2, 2018

mdreizin commented Jul 2, 2018

m-allanson commented Jul 3, 2018

jquense commented Jul 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdreizin commented Jul 3, 2018

jquense commented Jul 3, 2018

madeleineostoja commented Jul 3, 2018 • edited Loading

ooloth commented Jul 5, 2018

mdreizin commented Jul 8, 2018

jquense commented Jul 8, 2018

jquense left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdreizin Jul 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

szimek Jul 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

szimek commented Jul 16, 2018

mdreizin commented Jul 17, 2018

madeleineostoja commented Jul 17, 2018

mdreizin commented Jul 17, 2018

szimek commented Jul 17, 2018

mdreizin commented Jul 17, 2018 • edited Loading

m-allanson left a comment

Choose a reason for hiding this comment

m-allanson commented Jul 19, 2018

mdreizin commented Jul 19, 2018

mdreizin commented Jul 19, 2018

KyleAMathews commented Jul 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KyleAMathews left a comment

Choose a reason for hiding this comment

KyleAMathews commented Jul 19, 2018

KyleAMathews commented Jul 19, 2018

gatsbot bot commented Jul 19, 2018

KyleAMathews commented Jul 19, 2018

KyleAMathews commented Jul 19, 2018

mdreizin commented Jul 19, 2018

fardarter commented Jul 24, 2018

mdreizin commented Jul 24, 2018

fardarter commented Jul 25, 2018 via email

mdreizin commented Jun 28, 2018 •

edited

Loading

gatsbybot commented Jun 28, 2018 •

edited

Loading

gatsbybot commented Jun 28, 2018 •

edited

Loading

madeleineostoja commented Jul 3, 2018 •

edited

Loading

mdreizin Jul 11, 2018 •

edited

Loading

szimek Jul 15, 2018 •

edited

Loading

mdreizin commented Jul 17, 2018 •

edited

Loading

KyleAMathews commented Jul 19, 2018 •

edited

Loading