Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] add CSS Modules (postcss-modules) support #227

Closed
michael-ciniawsky opened this issue May 10, 2017 · 18 comments · Fixed by #322
Closed

[Feature] add CSS Modules (postcss-modules) support #227

michael-ciniawsky opened this issue May 10, 2017 · 18 comments · Fixed by #322

Comments

@michael-ciniawsky
Copy link
Member

michael-ciniawsky commented May 10, 2017

https://medium.com/@sokra/79583bd107d7
css-modules/css-modules#240
css-modules/css-modules#241

@felixsanz
Copy link

@michael-ciniawsky Looks like with 2.x my CSS modules are now broken. Is this related?

Is there a workaround? Thanks!

@michael-ciniawsky
Copy link
Member Author

@felixsanz Are you using LoaderOptionsPlugin in your webpack.config.js? Normally it shouldn't affect the CSS Modules implementation of css-loader, but this was claimed a few times now. #232 #222

@TrySound
Copy link

Shouldn't postcss-modules use messages for output instead of callback for this feature?
Btw, this works great in modular-css

@felixsanz
Copy link

felixsanz commented May 18, 2017

@michael-ciniawsky I'm using css-loader's modules: true option. I'm not using LoaderOptionsPlugin anywhere. I'll continue in #232

@michael-ciniawsky michael-ciniawsky changed the title [2.1.0] Add CSS Module Support (postcss-modules) [2.1.0] Add CSS Modules(postcss-modules) May 25, 2017
@michael-ciniawsky michael-ciniawsky changed the title [2.1.0] Add CSS Modules(postcss-modules) [2.1.0] Add CSS Modules (postcss-modules) May 25, 2017
@michael-ciniawsky michael-ciniawsky added this to Feature in Dashboard May 25, 2017
@michael-ciniawsky michael-ciniawsky changed the title [2.1.0] Add CSS Modules (postcss-modules) feat: add CSS Modules (postcss-modules) Sep 1, 2017
@michael-ciniawsky michael-ciniawsky changed the title feat: add CSS Modules (postcss-modules) [Feature] add CSS Modules (postcss-modules) support Oct 30, 2017
@ghost
Copy link

ghost commented Oct 30, 2017

Could this issue be the reason why webpack HMR no longer detects when I change the style of an existing class?

@michael-ciniawsky
Copy link
Member Author

I don't think so, show your config

@michael-ciniawsky
Copy link
Member Author

When did it exactly stop working?

@ghost
Copy link

ghost commented Oct 30, 2017

I've narrowed it down to the following updates (older version of postcss works without issue):

-    "postcss": "^5.2.18",
+    "postcss": "^6.0.13",
-    "postcss-loader": "^0.8.0",
+    "postcss-loader": "^2.0.8",
-    "postcss-modules": "^0.3.0",
+    "postcss-modules": "^1.1.0",
-    "postcss-nested": "^1.0.0",
+    "postcss-nested": "^2.1.2",

Webpack config looks like:

'use strict';

var path = require('path');
var webpack = require('webpack');
var DotenvPlugin = require('dotenv-to-webpack');
var ExtractTextPlugin = require('extract-text-webpack-plugin');

var envPath;
if (process.env.DOTENV_PATH) {
  envPath = path.join(process.env.DOTENV_PATH, './.env.development');
} else {
  envPath = path.join(__dirname, './.env.development');
}

module.exports = {
  devtool: 'cheap-module-eval-source-map',
  entry: {
    App: [
      'webpack-hot-middleware/client',
      'babel-polyfill',
      './src/renderApp.js'
    ],
    BasicTheme: [
      'webpack-hot-middleware/client',
      './src/themes/BasicTheme/BasicTheme.js'
    ]
  },
  resolve: {
    extensions: ['.js']
  },
  output: {
    path: path.join(__dirname, 'dist'),
    filename: '[name].js',
    chunkFilename: '[id].js',
    publicPath: '/dist/',
    library: '[name]',
    libraryTarget: 'umd'
  },
  plugins: [
    new DotenvPlugin({ path: envPath }),
    new webpack.HotModuleReplacementPlugin(),
    new webpack.NoEmitOnErrorsPlugin(),
    new ExtractTextPlugin('[name].css')
  ],
  module: {
    noParse: /dist\/localforage(|\.min).js/,
    rules: [
      {
        test: /\.js$/,
        use: ['babel-loader?'+JSON.stringify({
          plugins: [
            ['react-transform', {
              transforms: [{
                transform: 'react-transform-hmr',
                imports: ['react'],
                locals:  ['module']
              }]
            }]
          ]
        })],
        include: [
          path.resolve(__dirname, 'src')
        ]
      },
      {
        test: /\.css$/,
        use: ExtractTextPlugin.extract({
          fallback: 'style-loader',
          use: 'css-loader?modules&sourceMap&importLoaders=1&localIdentName=[name]__[local]___[hash:base64:5]!postcss-loader'
        }),
        include: [
          path.resolve(__dirname, 'src')
        ]
      }
    ]
  }
};

The CSS files are imported like so (from within the theme's .js file which maps the class names from their original class name to their localIdentName):

import root from './root.css';
import App from './App.css';
import Foo from './Foo.css';
import Bar from './Bar.css';

export default {
  classes: {
    ...root,
    ...App,
    ...Foo,
    ...Bar
  }
};

if (process.env.NODE_ENV !== 'production') {
  if (module.hot) {
    const reloadTheme = require('provide-theme').reloadTheme;

    module.hot.accept([
      './root.css',
      './App.css',
      './Foo.css',
      './Bar.css'
    ], () => {
      reloadTheme('BasicTheme', {
        classes: {
          ...require('./root.css'),
          ...require('./App.css'),
          ...require('./Foo.css'),
          ...require('./Bar.css')
        }
      });
    });
  }
}

The problem is that, after upgrading the postcss dependencies above, it no longer detects when one of the CSS files have changed... unless a new class name is added or a class name is removed.

For example, if we start with:

.App {
  background: red;
}

And change the background:

.App {
  background: blue;
}

It no longer detects that the CSS has changed.

Only if we add a new class name will it say the CSS has changed:

.App {
  background: blue;
}
.SomeNewClass {
  background: green;
}

@michael-ciniawsky
Copy link
Member Author

michael-ciniawsky commented Oct 30, 2017

You can't really use the postcss-modules plugin with webpack. It purpose is for other build tools like gulp/grunt. CSS Modules are handled by css-loader which is responsible for exporting the locals correctly

import styles from './file.css' // <= only works with 'css-loader' atm

export default (props) => (
  <div className={styles.container}></div>
)
import './file.css'
import styles from './modules.json' // <= `postcss-modules` (Crap, don't use it)

export default (props) => (
  <div className={styles.container}></div>
)

@michael-ciniawsky
Copy link
Member Author

That's why this issue exists 😛

@ghost
Copy link

ghost commented Oct 30, 2017

I'm confused as to what to do here. Is the problem with css-loader then? What can I do to help?

@michael-ciniawsky
Copy link
Member Author

michael-ciniawsky commented Oct 30, 2017

You are using postcss.config.js atm ? Remove the postcss-modules plugin from it and change your webpack.config.js to the following.

 {
   test: /\.css$/,
   use: env === 'production' 
    ? ExtractTextPlugin.extract({
      fallback: 'style-loader',
      use: [
        { 
          loader: 'css-loader',
          options: { 
            modules: true,
            localIdentName: '[name]__[local]___[hash:base64:5]',
            importLoaders: 1
          },
          'postcss-loader'
       ]
     }),
     : [ // `style-loader` pitches the HMR Code and enables HMR
       { loader: 'style-loader', options: { sourceMap: true } },
       { 
         loader: 'css-loader',
         options: { 
           modules: true,
           localIdentName: '[name]__[local]___[hash:base64:5]',
           importLoaders: 1,
           sourceMap: true
         },
         { loader: 'postcss-loader', options: { sourceMap: true } }
    ],
    include: [
      path.resolve(__dirname, 'src')
    ]
 }

Also remove your custom HMR Code from the component,style-loader adds the necessary HMR Code automatically. HMR doesn't work with the extract-text-webpack-plugin (full reload only in watch mode)

@ghost
Copy link

ghost commented Nov 1, 2017

The problem is that, for theming, I need the object which comes from the generated CSS class names so that it may be provided to components as a classes prop.

/* App.css */
.App {
  background: red;
}
.AppContainer {
  margin: 0 auto;
}
/* BasicTheme.js */
import App from './App.css';

export default {
  classes: {
    ...App
  }
};
import theme from './BasicTheme';

console.log(theme);
/*
{
  classes: {
    App: '_someMinifiedClassName',
    AppContainer: '_anotherMinifiedClassName'
  }
}
*/

The minified class names are used within components like so:

import React from 'react';
import PropTypes from 'prop-types';

const App = ({ classes }) => (
  <div className={classes.App}>
    etcetera
  </div>
);

App.propTypes = {
  classes: PropTypes.object.isRequired
};

export default App;

When the CSS changes, these class names (the classes object) might change, and the components need to be given the new classes object. This used to work without issues, but now HMR isn't detecting simple changes (like changing the background color of an existing class).

@michael-ciniawsky
Copy link
Member Author

if (process.env.NODE_ENV !== 'production') {
  if (module.hot) {
    const reloadTheme = require('provide-theme').reloadTheme;

    module.hot.accept([
      './root.css',
      './App.css',
      './Foo.css',
      './Bar.css'
    ], () => {
      reloadTheme('BasicTheme', {
        classes: {
-          ...require('./root.css'),
+          ...require('./root.css').default,
-          ...require('./App.css'),
+          ...require('./App.css').default,
-          ...require('./Foo.css'),
+          ...require('./Foo.css').default,
-          ...require('./Bar.css')
+          ...require('./Bar.css').default
        }
      });
    });
  }
}

@michael-ciniawsky
Copy link
Member Author

michael-ciniawsky commented Nov 1, 2017

from.js

const x = {}

export default x

toCJS.js

const x = require('./from.js').default

toESM.js

import x from './from.js'

@michael-ciniawsky
Copy link
Member Author

Otherwise please post your issue on e.g StackOverflow as it is unrelated to this issue and postcss-loader

@ghost
Copy link

ghost commented Nov 1, 2017

The require statements are not the issue (just verified that it does not need .default).

The issue is that it never even makes it to this block of code because it doesn't accept the css files as being changed (when they are in fact changed):

if (process.env.NODE_ENV !== 'production') {
  if (module.hot) {
    const reloadTheme = require('provide-theme').reloadTheme;

    module.hot.accept([
      './root.css',
      './App.css',
      './Foo.css',
      './Bar.css'
    ], () => {
      /*
       * NEVER REACHES THIS POINT
       */
      reloadTheme('BasicTheme', {
        classes: {
          ...require('./root.css'),
          ...require('./App.css'),
          ...require('./Foo.css'),
          ...require('./Bar.css')
        }
      });
    });
  }
}

As far as I can tell, the issue is related to postcss, as it only began happening after the following updates:

-    "postcss": "^5.2.18",
+    "postcss": "^6.0.13",
-    "postcss-loader": "^0.8.0",
+    "postcss-loader": "^2.0.8",
-    "postcss-nested": "^1.0.0",
+    "postcss-nested": "^2.1.2",

@ghost
Copy link

ghost commented Nov 1, 2017

It is entirely possible the issue is actually with css-loader. I guess I'll start a thread over there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment