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

[@types/mini-css-extract-plugin] When using 1.4.1 - not assignable to type 'Plugin' #52202

Closed
vladimir-djokic opened this issue Apr 6, 2021 · 3 comments

Comments

@vladimir-djokic
Copy link
Contributor

What I'm using in my project:

  • webpack 4.46.0
  • webpack-cli 3.3.12
  • mini-css-extract-plugin 1.4.0
  • @types/node 14.14.37
  • webpack config is .ts file (TypeScript)

Just installed the latest @types/mini-css-extract-plugin 1.4.1 and I get the following error when I try to build:

Type 'MiniCssExtractPlugin' is not assignable to type 'Plugin'. Types of property 'apply' are incompatible.

Differences between @types/mini-css-extract-plugin 1.4.0 and 1.4.1 type definitions:
mini-css-141-issue

Note: This all works correctly with @types/mini-css-extract-plugin 1.4.0

@vladimir-djokic vladimir-djokic changed the title When using @types/mini-css-extract-plugin 1.4.1 - not assignable to type 'Plugin' [@types/mini-css-extract-plugin] When using 1.4.1 - not assignable to type 'Plugin' Apr 6, 2021
@andersk
Copy link
Contributor

andersk commented Apr 9, 2021

This problem was introduced in #52044, which added webpack 5 support but broke webpack 4 support. I think this is fixable and should be fixed, but there seems to be some disagreement on both of these points. Copy of the discussion starting here:

@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.

@43081j
Copy link
Contributor

43081j commented Apr 9, 2021

Ideally we should have bumped the major version to make the intent clear: the latest version is for v5 and the previous is for v4.

If you're still on v4, you can pin the previous version of the types and it'll be fine until you upgrade to the matching version of webpack (v5).

We could likely add complexity to try support both in one, but I'm struggling to understand why. You installed the version for a newer webpack than what you have, you can simply go back to the version for v4.

Again I think the actual issue here is that we didn't do a major version bump, not that we don't support v4.

@vladimir-djokic
Copy link
Contributor Author

I'll close this issue as there is enough information here for anyone that might come upon similar scenario as I did.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants