search

webpack-contrib / mini-css-extract-plugin

Lightweight CSS extraction plugin
MIT License
4.66k stars 389 forks source link

fix: improve esMododule exports #1084

Closed jantimon closed 6 months ago

jantimon commented 7 months ago

This PR contains a:

Motivation / Use-Case

With esModule: false the mini-css-extract-plugin generates JS modules like this:

module.exports = {
  foo: "src-styles-foo",
  bar: "src-styles-bar",
};

This allows the following import:

import * as styles from "./styles.css";
import styles from "./styles.css";
import { foo } from "./styles.css";

With esModule: true the mini-css-extract-plugin generates JS modules like this:

export {
  "foo": "src-styles-foo",
  "bar": "src-styles-bar"
};

As there is no default export, the following import will not work:

import styles from "./styles.css"; // Fails

Interestingly the test case es-named-export tests for it:

https://github.com/webpack-contrib/mini-css-extract-plugin/blob/master/test/cases/es-named-export/index.js

import css, { aClass, bClass, cClass } from "./style.css";

However when tested manually, the test case fails:

export 'default' (imported as 'css') was not found in './style.css'

This PR adds the defaultExport option to the mini-css-extract-plugin which will allow the user to write the import exactly as in the es-named-exports test case

Breaking Changes

This feature is introduces behind an option. Therefore this is a non-breaking change. However changing the named export would probably also be not a breaking change)

need to check behaviour https://github.com/webpack/webpack/pull/18271#issuecomment-2032400346

linux-foundation-easycla[bot] commented 7 months ago

CLA Signed

The committers listed above are authorized under a signed CLA.

jantimon commented 7 months ago

@alexander-akait - I added it to the namedExport case - is it okay like this?

alexander-akait commented 7 months ago

@jantimon Just want to clarify, do you want to use and default and named export together?

jantimon commented 7 months ago

yes for backwards compatibility - right now nextjs allows both

nextjs uses mini-css-extract-plugin but is explicitly setting esModule: false

unfortunately esModule: false has the drawback that it prevents webpacks esmodule optimizations

so my idea was to switch to esModule but also allow named and default exports just like esModule: false

mini-extract-plugin has also a (currently broken) test on master which mixes default and named imports with esModules:

https://github.com/webpack-contrib/mini-css-extract-plugin/blob/master/test/cases/es-named-export/index.js

alexander-akait commented 7 months ago

@jantimon I see, we need to verify webpack (experimental.css) work the same (or provide an option to do it) and align with this plugin, I will investigate it tomorrow

alexander-akait commented 6 months ago

Sorry for delay, I done the same for built-in CSS support - https://github.com/webpack/webpack/pull/18271/ but want some dicussion with trspack team to align behaviour, then it will be merged I will do the same here

alexander-akait commented 6 months ago

@jantimon I put it under the option, because we still recommend to use only a named export to be better compatibility with future https://web.dev/articles/css-module-scripts

jantimon commented 6 months ago

@alexander-akait thanks for merging 🎉

Initially I added the default export also behind a feature flag so it's totally fine for me that you brought that back..

However I have one question - should the following css module code cause a warning for better future compatibility?

example.module.css

.default {
  color: red
}
alexander-akait commented 6 months ago

@jantimon make sense, but I think better to make it in css-loader