Documentation example seems wrong

[SetTrend]

Issue description or question

The sample code in readme.md reads (comments removed for brevity):

const CleanWebpackPlugin = require('clean-webpack-plugin');

const webpackConfig = {
    plugins: [
        new CleanWebpackPlugin(),
    ],
};

module.exports = webpackConfig;

I, however, have to write:

Webpack Config

const cleanWP = require('clean-webpack-plugin').default;

module.exports = {
  plugins: [
    new cleanWP()
  ]
};

... for Visual Studio Code (and the TypeScript compiler) to correctly construct clean-webpack-plugin.

Please note the additional .default in the first line.

Shouldn't this additionally required .default be mentioned in the readme.md file?

Environment

Run: npx envinfo --system --binaries --npmPackages clean-webpack-plugin,webpack

 System:
    OS: Windows 10
    CPU: (8) x64 Intel(R) Xeon(R) Gold 6142 CPU @ 2.60GHz
    Memory: 11.00 GB / 16.00 GB
  Binaries:
    Node: 10.15.3 - C:\Program Files\nodejs\node.EXE
    npm: 6.4.1 - C:\Program Files\nodejs\npm.CMD

[johnagan]

Not sure it's required to include .default since the class is exported as default:
https://github.com/johnagan/clean-webpack-plugin/blob/master/src/clean-webpack-plugin.ts#L315

Are you getting errors without it?

[chrisblossom]

.default is not required because we use babel-plugin-add-module-exports. As you have found out, VS Code / Typescript doesn't provide hints if you don't include .default. See this thread for more information.

I do not think the readme should be updated to include .default in the example for the sake of VS Code / type hints, but it might be possible to modify the types included to work with and without .default. @SimenB / @DanielRosenwasser did either of you ever figure out a solution to this?

[SimenB]

You can do export =, and intellisense should be happy. That's not really a good solution, though. Happy to learn otherwise!

[chrisblossom]

Thanks for the response @SimenB!

I'm not familiar enough with Typescript to understand the implications of switching to use export = in combination with tsconfig.json's "module": "commonjs" setting. I know we'd need to use jest's babel-plugin-jest-replace-ts-export-assignment.js.

Looks like all we lose is the ability for someone to require .default and everyting else would work as expected? Wonder if Typescript type hints will work correctly if using import cleanWebpackPlugin from 'clean-webpack-plugin';

[chrisblossom]

After some testing, looks like unless tsconfig.json has "esModuleInterop": true, the following error is thrown:

webpack.config.ts:1:8 - error TS1192: Module '"/Users/chris/github/clean-webpack-plugin/dist/clean-webpack-plugin"' has no default export.

1 import CleanWebpackPlugin from 'clean-webpack-plugin';
         ~~~~~~~~~~~~~~~~~~


Found 1 error.

[chrisblossom]

From the docs:

When exporting a module using export =, TypeScript-specific import module = require("module") must be used to import the module.

Looks like without "esModuleInterop": true, import CleanWebpackPlugin = require('clean-webpack-plugin'); works.

As of right now I'm not against the idea of migrating to use export =, but it is probably a breaking change. Thoughts? I'm not sure it is worth upping the version to v3 for just this. Also, I would definitely prefer a less-hacky solution if anyone has any other ideas.

[SimenB]

Oh, this lib is written in TS, I thought it just had a d.ts file. Then I would not change anything and just document the fact that consumers needs to add .default if their IDE/typechecker is yelling at them. The interop with CJS will always be a bit painful/ugly I think.

Jest will move to ES module exports in the next major and ditch export =.

One thing you can do is have a named export in addition to the default one, so people can do const CleanPlugin = require('clean-webpack-plugin').plugin; or something. Not much of a difference, but looks better than default, at least to my eyes

[chrisblossom]

#138 will resolve this issue.

[johnagan]

released with v3.0.0

[jinliming2]

Maybe it's not a good idea to just replace the default export with named export.

I think it's better to keep both named export and default export at the same time.

export CleanWebpackPlugin;
export default CleanWebpackPlugin;

[chrisblossom]

@jinliming2 I wish there were a better way for common js and modules to work. I personally think dropping the default export is the best solution and I don't see us reverting the change.

If you'd like to rename the named export, you can do so:

// es modules variable named "CleanPlugin"
import { CleanWebpackPlugin as CleanPlugin } from 'clean-webpack-plugin';

// common js variable named "CleanPlugin"
const { CleanWebpackPlugin: CleanPlugin } = require('clean-webpack-plugin');