Failed to use in a TypeScript-format webpack configuration

[g-plane]

Describe the bug

Failed to use in a TypeScript-format webpack configuration.

To Reproduce

An example for webpack.config.ts: (Note that it's .ts, not .js)

import * as webpack from 'webpack'
import WindiCSSWebpackPlugin from 'windicss-webpack-plugin'

const config: webpack.Configuration = {
  plugins: [new WindiCSSWebpackPlugin()]
}

Expected behavior

No type-check errors.

Actual behavior

With the code example above, TypeScript will complain:

Type 'WindiCSSWebpackPlugin' is not assignable to type '((this: Compiler, compiler: Compiler) => void) | WebpackPluginInstance'.
  Type 'WindiCSSWebpackPlugin' is not assignable to type 'WebpackPluginInstance'.
    Types of property 'apply' are incompatible.
      Type '(compiler: import("./node_modules/windicss-webpack-plugin/dist/plugin").Compiler) => void' is not assignable to type '(compiler: import("./node_modules/webpack/types").Compiler) => void'.
        Types of parameters 'compiler' and 'compiler' are incompatible.
          Type 'import("./node_modules/webpack/types").Compiler' is not assignable to type 'import("./node_modules/windicss-webpack-plugin/dist/plugin").Compiler'.
            Property '$windi' is missing in type 'Compiler' but required in type '{ $windi: WindiPluginUtils & { dirty: Set<string>; root: string; virtualModules: Map<string, string>; initException?: Error | undefined; invalidateCssModules: (resource: string, modules: string[]) => void; server: Server; }; }'.ts(2322)
plugin.d.ts(24, 5): '$windi' is declared here.

The reason I think is WindiCSSWebpackPlugin uses a extended Compiler type with additional property $windi, not the type Compiler directly from 'webpack', but for users' webpack configuration, it only can be the Compiler type from 'webpack'. This $windi property is assigned in the WindiCSSWebpackPlugin internally, so it shouldn't be exposed to users.

[harlan-zw]

Hey @g-plane

Looks like I wrote the types poorly originally, resulting in your issue above. I've used proper ts augmentation now which should fix it (https://github.com/windicss/windicss-webpack-plugin/commit/11a0b8b6d4a26661571e63108a1628be900f3dc4)

I appreciate the proactive PR though, but i'll prefer the above commit over it, just in-case third-party packages want to use $windi on the webpack compiler.

Fix is in 1.7.6, if you still have any issues let me know and I'll jump on it :)

[g-plane]

However, I've noticed that you augmented the Compiler from 'webpack', which will affect everywhere uses the Compiler type. For example, if someone creates their own webpack plugins in a project that uses this WindiCSSWebpackPlugin, they will be able to see the $windi property, which is unexpected and may cause bugs.

[harlan-zw]

The $windi variable is already attached to the compiler at runtime, the typescript augmentation is simply exposing that fact.

Can you elaborate on why you think it's unexpected or may cause bugs? It's exposed to allow the loaders to work, as well as allowing third-party plugins to use the windi service

[g-plane]

If exposing $windi for other loaders and third-party plugins are expected and documented as public API, that should be OK, but I can't find any documentation about this. Also, generally a property or a variable prefixed with $ means it's private. (Of course, this is just conventional.)

Besides, if there's a project install and use this plugin, and that project contains its own webpack plugins which aren't related to WindiCSS. Although the augmentation is exposing the fact, those plugins may access the $windi property accidentally. Since the $windi property is exposed at type level, the bug of accessing $windi can't be caught by TypeScript. But yes, it may work at runtime because the $windi property is already exposed, but this doesn't match their expectation.

[harlan-zw]

The augmentation is the documentation for the service, if someone wants to use Windi they can read the types. If someone "accidentally" accesses the $windi properly, how is that my problem? I don't see how that's possible anyway.

The $ convention is a pre-existing pattern for webpack plugins, for example, see the original esbuild webpack plugin: https://github.com/privatenumber/esbuild-loader/blob/v1.3.1/src/index.js

I can't see how removing the augmentation or changing the architecture would add any value to DX (only subtract), I appreciate your feedback though and am happy to reconsider if you have a more convincing point