cerner / terra-dev-site

A development environment for React UI components for documentation and testing
http://engineering.cerner.com/terra-dev-site/
Apache License 2.0
13 stars 19 forks source link

Include a postcss.config.js file #35

Closed JakeLaCombe closed 6 years ago

JakeLaCombe commented 6 years ago

Issue Description

While uplifting terra-framework to React 16, dependencies inside terra-clinical were updated to require the use of a postcss configuration file. Since terra-core and terra-clinical both have them and pull from their perspective site packages, it maybe ideal to include a base postcss config in this package. as well

Issue Type

emilyrohrbough commented 6 years ago

Terra-site does provide the same configuration as the postCssConfig, see here. This configuration was just combined with the webpack config because it was originally separate for the use of storybook and it seemed to increase readability for non-familiar webpack users by having all configuration in one place.

StephenEsser commented 6 years ago

I ran into this same issue while upgrading Kaiju to React 16 and pulling in the latest components from terra-core and terra-clinical. The only solution I've found so far that works is to add this completely empty file or move the post css configuration into it.

https://github.com/cerner/kaiju/blob/2778819c5e9ee820371eb491f62179a0621be6b0/rails/client/postcss.config.js

Here is the pull request with the changes that cause the need of this file: https://github.com/cerner/kaiju/pull/103/files

Without the file webpack fails with the following error.

18:17:14 client.1 |     ERROR in ./node_modules/css-loader?{"sourceMap":true,"importLoaders":2,"localIdentName":"[name]_[local]_[hash:base64:5]"}!./node_modules/postcss-loader/lib?{}!./node_modules/sass-loader/lib/loader.js!./node_modules/terra-button-group/lib/ButtonGroupButton.scss
18:17:14 client.1 |     Module build failed: Error: No PostCSS Config found in: /Users/top-secret-username/kaiju/kaiju/rails/client/node_modules/terra-button-group/lib
18:17:14 client.1 |         at /Users/top-secret-username/kaiju/kaiju/rails/client/node_modules/postcss-load-config/index.js:51:26
18:17:14 client.1 |         at <anonymous>
18:17:14 client.1 |      @ ./node_modules/css-loader??ref--3-2!./node_modules/postcss-loader/lib??ref--3-3!./node_modules/sass-loader/lib/loader.js!./node_modules/terra-button-group/lib/ButtonGroup.scss 3:10-237 11:47-274
18:17:14 client.1 | Child extract-text-webpack-plugin node_modules/extract-text-webpack-plugin/dist node_modules/css-loader/index.js??ref--3-2!node_modules/postcss-loader/lib/index.js??ref--3-3!node_modules/sass-loader/lib/loader.js!node_modules/terra-button-group/lib/ButtonGroup.scss:
18:17:14 client.1 |        3 modules
18:17:14 client.1 |     
18:17:14 client.1 |     ERROR in ./node_modules/css-loader?{"sourceMap":true,"importLoaders":2,"localIdentName":"[name]_[local]_[hash:base64:5]"}!./node_modules/postcss-loader/lib?{}!./node_modules/sass-loader/lib/loader.js!./node_modules/terra-button-group/lib/ButtonGroupButton.scss
18:17:14 client.1 |     Module build failed: Error: No PostCSS Config found in: /Users/top-secret-username/kaiju/kaiju/rails/client/node_modules/terra-button-group/lib
18:17:14 client.1 |         at /Users/top-secret-username/kaiju/kaiju/rails/client/node_modules/postcss-load-config/index.js:51:26
18:17:14 client.1 |         at <anonymous>
18:17:14 client.1 |      @ ./node_modules/css-loader??ref--3-2!./node_modules/postcss-loader/lib??ref--3-3!./node_modules/sass-loader/lib/loader.js!./node_modules/terra-button-group/lib/ButtonGroup.scss 3:10-237 11:47-274
JakeLaCombe commented 6 years ago

The reason I logged it was due to the issue Stephen provided, and noticed core and clinical both had a config file which read the contents of their perspective site packages. If we are looking to keep the configuration for postcss inside the webpack config you linked to, I'd be ok if we just had blank configuration files with a comment noting the need for their presence inside the file itself.

emilyrohrbough commented 6 years ago

This is very interesting, I was actually seeing the same error @StephenEsser was when pulling in the React16 terra-core into terra-ui. This link you provided is very interesting. If the intention for the plugin is to actually contain a postcss config that I would be in favor of following that pattern.

bjankord commented 6 years ago

:-1: for blank config files as a permanent solution. We'll need time to figure out whats going on with the postcss-loader and the postcss-load-config plugins. It seems like there is a bug in one of them from looking at this thread.

mjhenkes commented 6 years ago

to be clear i'm agreeing with the thumbs down 😄

emilyrohrbough commented 6 years ago

Ah, yeah, empty config feels wrong, I meant we could just provide a config!

emilyrohrbough commented 6 years ago

After reading a bit about the loader:

  1. It appears we must provided a config and can provide a config path

  2. I am wondering if we need to added an unique ident key to the postcss-loader options keys after reading this.

  3. I am wondering if the postcss-loader is a main cause for our long build times https://github.com/postcss/postcss-loader/issues/307

StephenEsser commented 6 years ago

I was able to remove the empty config file by adding an ident key following the second option provided above by @emilyrohrbough

https://github.com/cerner/kaiju/pull/103/commits/f3c693cee6b1608805e2812eca498ea7e67daf35

noahbenham commented 6 years ago

Should we add this to the docs for consumers?

emilyrohrbough commented 6 years ago

@noahbenham We can add it as a doc for consumers, however if you are using our configuration it will be provided and if you are not, this would technically be a requirement of the postcss-load itself.