egoist / rollup-plugin-postcss

Seamless integration between Rollup and PostCSS.
MIT License
678 stars 218 forks source link

Css module support is broken in 3.1.3 #296

Open vladshcherbin opened 4 years ago

vladshcherbin commented 4 years ago

In 3.1.2 docs example works, imports in js return correct objects.

postcss({
  modules: true
})

In 3.1.3 same code imports return undefined. From https://github.com/egoist/rollup-plugin-postcss/pull/292 there is new onlyModules option I believe. Using it brings back correct objects.

postcss({
  modules: true,
  onlyModules: false
})

However, it's a breaking change, this option is not documented and I have no idea if using it is a correct workaround.

Would be awesome to see official solution to this one 🙏

vladshcherbin commented 4 years ago

Using object instead of true returns undefined again:

postcss({
  modules: {
    localsConvention: 'dashesOnly'
  },
  onlyModules: false
})
song-jia commented 4 years ago

I've same issue, set autoModules: false resolve my problem. My settings:

postcss({
    modules: {
        generateScopedName: 'myapp_[name]_[local]'
    },
    extract: true,
    autoModules: false
})
wardpeet commented 4 years ago

@katywings do you have an idea what could cause this? It seems like https://github.com/egoist/rollup-plugin-postcss/pull/292 is the culprit.

katywings commented 4 years ago

In 3.1.2 docs example works, imports in js return correct objects.

@vladshcherbin How did you reproduce this issue? Whats the file name of the css file you are importing? Does it include .module.? Its really hard to reproduce your problem without a reprodution repo / zip of some sorts ^.^


In 3.1.3 same code imports return undefined

I could not reproduce this. postcss({ modules: true }) correctly lets me import styles.css as a module in my test repo.


Using object instead of true returns undefined again

Yeah, because if you have autoModules enabled - which is the default :D - and your css file doesn't have a .module. suffix it will not be interpreted as module.


To answer the question in which cases a css file is interpreted as module: The description of #292 includes a detailed table of when a file is to be considered a css module.

History about #292 / autoModules rollup-plugin-postcss by default has the autoModules option enabled, this default is nothing new. This option makes sure that rollup-plugin-postcss only interprets css files as modules, if they're suffixed with .module. - as an example blub.module.css. But in <= 3.1.2 there was a bug which caused this option to break in certain situations. #292 fixed those.


About onlyModules: this is an internal variable. Adding onlyModules doesn't have any effect, because its literally no setting - you cannot change it from the outside. If you want to know what it internally does: it explicitly tracks if the modules setting is === true


Here is a quick output of many different cases which all worked fine imho:

// default plugin settings
/home/katja/Projects/rollup-postcss-test/dist/defaults.js 
styles.css false
styles.module.css [ 'my-class', 'my-class2' ]

// { modules: {} }
/home/katja/Projects/rollup-postcss-test/dist/modules-obj.js 
styles.css false
styles.module.css [ 'my-class', 'my-class2' ]

// { modules: {}, autoModules: false }
/home/katja/Projects/rollup-postcss-test/dist/modules-obj-no-auto.js
styles.css [ 'my-class', 'my-class2' ]
styles.module.css [ 'my-class', 'my-class2' ]

// { modules: true }
/home/katja/Projects/rollup-postcss-test/dist/modules-true.js
styles.css [ 'my-class', 'my-class2' ]
styles.module.css [ 'my-class', 'my-class2' ]

ZIP which includes the top cases: rollup-postcss-test.zip

You can run it with npm install && npm run build && npm start

wardpeet commented 4 years ago

Thanks @katywings for the great explanation. @vladshcherbin any chance you can give us a demo project?

Twipped commented 4 years ago

I also got bit by this bug after upgrading my project from 3.1.1 to 3.1.8. I downgraded back to 311 and it started working again. I can also confirm that adding autoModules: false fixed it.

This is my plugin config:

  postcss({
    extract: false,
    modules: { scopeBehaviour: 'global' },
    use: [ 'sass', [ 'prepend', { files: [ resolve('styles.scss') ] } ] ],
    loaders: [
      {
        name: 'prepend',
        test: /\.(scss)$/,
        async process ({ code }) {
          const { files = [] } = this.options;
          const contents = await Promise.all(files.map((f) => fs.readFile(f), { encoding: 'utf8' }));
          contents.push(code);
          return { code: contents.join('\n\n'), map: undefined };
        },
      },
    ],
  }),

I think this may be the relevant change. This will cause autoModules to become true when options.autoModulesis undefined. Previously it only became true if it was undefined AND it was a module file.

slavafomin commented 3 years ago

Wow, it almost broke our release, good thing that tests caught this.

After upgrading to 4.0.0 our styles stopped being processed as CSS modules therefore returning undefined to JS and emitting stylesheets without properly rewritten classnames. Setting autoModules to false helped it. By the way, we are not using .module in our filenames.

WesAtIgloo commented 3 years ago

Definitely a breaking change, shouldn't have been released as a bugfix. We are not using .module, so thankfully this was caught while we were updating versions. Time to write tests to make sure this gets caught early