webpack-contrib / mini-css-extract-plugin

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

Getting various problems with webpack5 and typescript #649

Closed richtera closed 3 years ago

richtera commented 3 years ago

Expected Behavior

esModule and nameModules settings should be compatible between style-loader and mini-css-extract-plugins.

Actual Behavior

I am seeing typescript being happy with import styles from "something.css" during HRM and style-loader, but then creating warnings with mini-css during production compiles. Turning off esModule and using require() also works in style-loader, but esModules are forced in mini-css during production compiled even though esModule is off. So far I have not found a combination that either turns on or off esModules both in production and HMR. I have added a

Code

function makeCss(obj) {
  return obj && obj.__esModule ? obj.default : obj;
}
const styles = makeCss(require("../styles.scss"));

to make it work.

How Do We Reproduce?

I didn't have to do anything specially but upgrade webpack to get the error. I had to hold back typescript at 3.9.7

richtera commented 3 years ago

Ok I think I found the combination of problems when using webpack5 and mini-css

  1. It seems esModule: false is being ignored in the mini-css plugin during final code generation but not in style-loader
  2. Empty css files now cause an es module with a null default value instead of an empty object.
  3. During the analysis process of webpack esModule: false is being observed correctly
alexander-akait commented 3 years ago

Sorry, we don't provide typescript types yet

It seems esModule: false is being ignored in the mini-css plugin during final code generation but not in style-loader

No

Empty css files now cause an es module with a null default value instead of an empty object.

yes, it was internal change

During the analysis process of webpack esModule: false is being observed correctly

...

alexander-akait commented 3 years ago

mini-css-extract-plugin !== style-loader, you should not use __esModule in client code

richtera commented 3 years ago

But this is broken.

  1. Typescript defs are not necessary.
  2. esModule needs to be observed
  3. I do get null errors where default is undefined for empty css files in production.
  4. Production compiles correctly analyze with esModule on or off but crash at runtime.
alexander-akait commented 3 years ago

I don't understand what you want to achieve

esModule needs to be observed

It is true by default for style-loader and mini-css-extract-plugin

I do get null errors where default is undefined for empty css files in production.

What do you mean

Production compiles correctly analyze with esModule on or off but crash at runtime

Yes, because it is runtime error

And no, it is not broken

alexander-akait commented 3 years ago

with namedExport you don't have default export and namedExport works only with esModule: true

richtera commented 3 years ago

Everything was working fine with webpack4. When I switched to webpack5 my production builds started crashing. I traced it down to the differences between style-loader in dev and mini-css in production. Switching esModule: false did not resolve the crash but adding a default rule into the empty scss file did.

alexander-akait commented 3 years ago

Please create reproducible test repo

richtera commented 3 years ago

@evilebottnawi Here is a sample repo that shows the JSON "imported" https://github.com/getselfstudy/mini-css-test When running with esModule on you can see that a file without any rules starting with . will end up without a "default" property. When running without esModule you can still see the default property. I hooked up yarn start and yarn build; yarn serve to setup up dev and production with esModule: true and yarn start-non-module and yarn build-non-module; yarn serve to setup dev and production with esModule: false. I hope this helps.

alexander-akait commented 3 years ago

I don't understand your problem, you use require, so you will have default, because modules in ES syntax, you need migrate on import or disable esModule, namedExport doesn't work without esModule: false

alexander-akait commented 3 years ago
{
  esModule: true,
  modules: {
    namedExport: true
  }
}

doesn't work on plugin level, it is loader options https://github.com/webpack-contrib/mini-css-extract-plugin#loader-options

alexander-akait commented 3 years ago

I will fix it in the next release

richtera commented 3 years ago

@evilebottnawi I used require in order to show that "default" is undefined when it should be {}. So it's close, but sometimes it makes the wrong decision, but then suddenly production compiles crash. Especially with typescript, because when it finds __esModule it will force default to be used and will get an undefined from the stylesheet which was causing us problems. I agree that 90% of the times it's fine to return undefined, but it is confusing to be ok in dev mode and then crash in production :)

alexander-akait commented 3 years ago

"default" is undefined when it should not be {} when you use namedExport

richtera commented 3 years ago

If that's true then maybe it's a problem in style-loader as well. I just think the two need to match. Otherwise dev will work and prod will crash. So whatever the rules are I think it can be handled in the code. But if they don't match then it's unpredictable.

alexander-akait commented 3 years ago

@richtera style-loader works same, what is the problem, I don't understand you, you had a wrong configuration and I fix schema in the latest release

richtera commented 3 years ago

@evilebottnawi Ok I will try to figure out exactly how to this causes a problem. When I switched to webpack5 everything in dev worked, but my production builds were access violating since the style default was undefined. Originally I had no esModule or namedExport defined at all, I added those to try and make my production builds behave the same as the dev builds.