Skip to content

Conversation

@jonathantneal
Copy link
Contributor

@jonathantneal jonathantneal commented Jul 8, 2019

What kind of change does this PR introduce?

This change allows the existing PostCSS Loader to respect userland postcss.config.js configuration files in their ability to define the PostCSS plugins used in a project.

See https://github.com/postcss/postcss-loader#usage

Did you add tests for your changes?

No, and I did not see similar tests for PostCSS plugins in the existing project.

Summary

I would like to use PostCSS Normalize and PostCSS Preset Env, but I am unable to do this in my postcss.config.js configuration file due to how PostCSS Loader is currently configured.

My workaround has been to detect and delete the existing PostCSS Loader plugins option, which then allows the configuration file to be followed. However, I think it would be better if the zero-configuration (i.e Autoprefixer) is preserved unless a configuration file explicitly defines its own plugins.

Does this PR introduce a breaking change?

No, this PR does not introduce a breaking change.

Other information

This change adds a postcss-load-config dependency, but it should de-dupe along with the other nested postcss-load-config dependency. See these results for npm ls postcss-load-config:

[email protected] /preactjs/preact-cli/packages/cli
├── [email protected] 
└─┬ [email protected]
  └── [email protected]  deduped

@jonathantneal jonathantneal force-pushed the support/postcss-config-plugins branch 2 times, most recently from 5208f9e to b31a22d Compare July 9, 2019 13:19
@jonathantneal jonathantneal force-pushed the support/postcss-config-plugins branch from b31a22d to 79a85c2 Compare July 9, 2019 13:29
@jonathantneal
Copy link
Contributor Author

I see some positive feedback in the form of comment reactions. However, is this PR under consideration by the Preact CLI team?

I am hopeful, as my similarly themed PRs were merged — #829 to support Browserslist configuration files, and #831 to support pcss files.

let postcssPlugins;

try {
postcssPlugins = loadPostcssConfig.sync(cwd).plugins;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good to me, just wondering if we should load more than just the plugins?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

Okay, forgive the long response, but here are my thoughts on loading more than just the plugins from a PostCSS configuration.

The other options are to, map, from, parser, syntax, and stringifier.

I am confident that to and from can be ignored because webpack determines those locations. I am somewhat confident that parser, syntax, and stringifier can be ignored at this time, because css-loader and style-loader can still choke on non-CSS syntaxes.

I am on the fence about map. On the one hand, it can be ignored, because source map configuration needs to be unified across all CSS loaders in order to be useful. However, if PostCSS configuration enables or disables CSS source maps for a project, then that does sound useful. The question is whether PostCSS configuration is the most logical place to put control over CSS source maps.

@ForsakenHarmony ForsakenHarmony merged commit 6c95e17 into preactjs:master Jul 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants