update mini-css-extract-plugin to webpack 5

[43081j]

Please fill in this template.

• Use a meaningful title for the pull request. Include the name of the package modified.
• Test the change in your own code. (Compile and run.)
• Add or edit tests to reflect the change.
• Follow the advice from the readme.
• Avoid common mistakes.
• Run npm test <package to test>.

---

I've been trying to update another repo to use webpack 5, so it has something like this:

import webpack from 'webpack'; // <- This is webpack5's own types, not DT's v5 wrapper

const config: webpack.Configuration = {
  // ...
  plugins: [
    new miniCSSExtractPlugin() // <- This will error because mini-css-extract's types reference DT's webpack 4 types 
  ]
};

But mini-css-extract-plugin actually supports webpack 5, so maybe it is ok to just upgrade?

cc @sandersn is this how you expected people to bump from 4 to 5? just remove the path mapping?

[typescript-bot]

@43081j Thank you for submitting this PR!

This is a live comment which I will keep updated.

1 package in this PR

• mini-css-extract-plugin on npm, on unpkg
  • owner-approval: @peterblazejewicz
  • 1 added owner: ✎@43081j

@43081j: I see that you have added yourself as an owner, are you sure you want to become an owner?

Code Reviews

Because you edited one package and updated the tests (👏), I can help you merge this PR once someone else signs off on it.

Status

• ✅ No merge conflicts
• ✅ Continuous integration tests have passed
• ✅ Most recent commit is approved by type definition owners or DT maintainers

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.

---

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 52044,
  "author": "43081j",
  "headCommitOid": "c084ddbc01cf542259a06bbeea3a8d42597ec329",
  "lastPushDate": "2021-03-29T20:50:54.000Z",
  "lastActivityDate": "2021-03-31T08:55:30.000Z",
  "maintainerBlessed": false,
  "mergeOfferDate": "2021-03-30T17:22:43.000Z",
  "mergeRequestDate": "2021-03-31T08:55:30.000Z",
  "mergeRequestUser": "43081j",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "popularityLevel": "Popular",
  "pkgInfo": [
    {
      "name": "mini-css-extract-plugin",
      "kind": "edit",
      "files": [
        {
          "path": "types/mini-css-extract-plugin/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/mini-css-extract-plugin/mini-css-extract-plugin-tests.ts",
          "kind": "test"
        },
        {
          "path": "types/mini-css-extract-plugin/package.json",
          "kind": "package-meta-ok"
        },
        {
          "path": "types/mini-css-extract-plugin/tsconfig.json",
          "kind": "package-meta-ok"
        }
      ],
      "owners": [
        "JounQin",
        "dobogo",
        "skovy",
        "peterblazejewicz"
      ],
      "addedOwners": [
        "43081j"
      ],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "sandersn",
      "date": "2021-03-30T21:38:30.000Z",
      "isMaintainer": true
    },
    {
      "type": "approved",
      "reviewer": "peterblazejewicz",
      "date": "2021-03-30T17:22:07.000Z",
      "isMaintainer": false
    }
  ],
  "ciResult": "pass"
}

[typescript-bot]

🔔 @JounQin @dobogo @skovy @peterblazejewicz — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

[typescript-bot]

@43081j The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

[typescript-bot]

@43081j The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

[43081j]

This now fails because:

../tapable/index.d.ts(7,15): error TS2307: Cannot find module './node_modules/tapable' or its corresponding type declarations.

Not too sure how to resolve this as tapable isn't a direct dependency of mini-css-extract-plugin

[typescript-bot]

@43081j The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

[typescript-bot]

@sandersn Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

[43081j]

Ready to merge

Thanks for the help @sandersn, much appreciated.

[typescript-bot]

I just published @types/[email protected] to npm.

[andersk]

This broke webpack 4 support, even though mini-css-extract-plugin upstream still supports webpack 4:

// webpack.config.ts
import type { Configuration } from "webpack";
import MiniCssExtractPlugin from "mini-css-extract-plugin";

const config: Configuration = {
  plugins: [new MiniCssExtractPlugin()],
};

export default config;

// tsconfig.json
{
  "compilerOptions": {
    "esModuleInterop": true
  }
}

$ yarn add @types/mini-css-extract-plugin @types/webpack@^4 mini-css-extract-plugin typescript webpack@^4
$ yarn tsc
…
webpack.config.ts:6:13 - error TS2322: Type 'MiniCssExtractPlugin' is not assignable to type 'Plugin'.
  Types of property 'apply' are incompatible.
    Type '(compiler: import("/tmp/test/node_modules/@types/mini-css-extract-plugin/node_modules/webpack/types").Compiler) => void' is not assignable to type '(compiler: import("/tmp/test/node_modules/@types/webpack/index").Compiler) => void'.
      Types of parameters 'compiler' and 'compiler' are incompatible.
        Type 'Compiler' is missing the following properties from type 'Compiler': webpack, root, watching, intermediateFileSystem, and 30 more.

6   plugins: [new MiniCssExtractPlugin()],
              ~~~~~~~~~~~~~~~~~~~~~~~~~~

Works with @types/[email protected], fails with @types/[email protected].

[peterblazejewicz]

@andersk this should also work, can you verify, thx!

    "paths": {
      "webpack":[
        "node_modules/@types/webpack"
      ]
    },
    "baseUrl": "."

[43081j]

Yep it's a bit difficult to support both, given the types exist in different places for each version of web pack and will conflict. I suspect consumers of v4 will need to map the path or stay on the old version of the types for now at least.

[andersk]

@peterblazejewicz That sort of works if I also add "tapable": ["node_modules/@types/tapable"], but it still pulls in an extra version of webpack and all its dependencies.

Given #51712, can we just add "@types/webpack": "^4 || ^5" to peerDependencies and let the user pick the right one?

[43081j]

Not quite. V4 and v5 both export different types, a compiler from v5 isn't assignable to a compiler from v4 etc.

In reality the solution here I think is to not update the types. The interface is very unlikely to change, if ever for this plugin. This is why you have lock files, you can stay on the old version until you are on web pack 5.

[andersk]

I understand that the v4 Compiler is different from the v5 Compiler, but that shouldn’t be a problem as long as @types/mini-css-extract-plugin imports Compiler from the same version of webpack that the user is using in webpack.config.ts. That’s precisely the reasoning behind my peerDependencies proposal.

[43081j]

i see what you mean, i still feel like there's some reason we wouldn't or couldn't do that. maybe one for @sandersn to confirm since we should align to the rest of the v4/v5 changes he did too

again, though, why would you need the new version? whats stopping you from staying on the previous version of the type definitions? the ones for v4...

i get that the library supports both, but the interface hasn't and won't change in reality. the old version of the types package is perfectly valid for v4, while the new version is valid for v5. you're not gaining anything by updating to the v5 types if you're on v4, its a meaningless update.

just trying to understand the use case here

[andersk]

Obviously I’m not going to upgrade to a version that doesn’t work. But if the only response to “new version doesn’t work” is always going to be “don’t upgrade”, then the world is going to become full of new versions that don’t work and are not upgraded. This is especially true if this model is going to be copied into other packages that I then won’t be able to upgrade, and those packages are depended by other packages that I then won’t be able to upgrade, etc. What I’m saying is, it’s still legitimate to complain about regressions. Let me worry about “why do you need the new version?”; that doesn’t mean we shouldn’t ask “why can’t we fix the new version?”.

[43081j]

the idea was that the new version of the types would be for webpack v5. we intended for it to be a major version bump too but it was already at the version we wanted, we probably should have still bumped it regardless tbh.

leaving the old types as being for v4.

the new version does work, for the webpack version it was built for. it was assumed that if you're not yet on that version of webpack, you just wouldn't upgrade (logically, since you're not on the same version).

i would argue it isn't broken, because it works for the version is exists for. like i said, the interface hasn't and won't change, so the old version is perfectly valid for v4.

you seem to be seeing this as if its 'broken', but somewhere down the line you decided to update to a new version explicitly which wasn't made for the legacy webpack you're using. the old version exists for that.

[sandersn]

@andersk, from reading your comments, I think the real problem is that the newest version of mini-css-extract-plugin supports both webpack 4 and 5, but @types/mini-css-extract-plugin only supports webpack 5. Is that correct?

The general problem is that the webpack 4 and 5's types really are different. I don't think it's possible for all plugin types to support both at once. (Looking at the PR to fix webpack 5 compat at webpack-contrib/mini-css-extract-plugin#698, I'm not even sure all plugins will support both at once.) However, for mini-css-extract-plugin it looks like removing some test coverage would allow the types to apply to webpack 4 or 5.

Peer dependencies would probably work, but (1) nothing else in Definitely Typed uses them yet (2) I'm not sure how to make the DT tests pass with a package.json that specifies only peer dependencies. Maybe we could specify a secondary package.json for testing purposes, which specifies webpack@5 ? It sounds like a complex solution.

Personally I think @43081j is right about using a pinned, old version. I also guess that there isn't much of a slippery slope towards many tools getting stuck in lockstep upgrades like webpack 5 has.

[andersk]

Does {"peerDependencies": {"@types/webpack": "^4 || ^5"}, "devDependencies": {"@types/webpack": "^5"}} satisfy the test coverage requirement?

(Yes, many plugins don’t support both 4 and 5 at once. Those plugins bumped their major version number, since dropping v4 is a backwards-incompatible change. Examples: expose-loader@^2, html-webpack-plugin@^5, postcss-loader@^5, terser-webpack-plugin@^5.)

[43081j]

I don't think DT has support for peer dependencies yet. Or nobody ever tried.

Is there a reason sticking to the old v4 types isn't an option? We should really treat the current types as v5 and the previous types as v4. Otherwise I can see us adding complexity to support something we never really needed to do

[peterblazejewicz]

Given #51712, can we just add "@types/webpack": "^4 || ^5" to peerDependencies and let the user pick the right one?

Not sure if that would work either. @andersk can you please create proper issue, copy paste recent comments from here? thx!

[andersk]

It looks like I’m not the first to report this: #52202. I’ll copy some information there.