indutny / webpack-common-shake

CommonJS Tree Shaker plugin for WebPack
918 stars 13 forks source link

Please upgrade to Webpack v5 #40

Open budarin opened 4 years ago

budarin commented 4 years ago

the current version is not compatible with Webpack 5

jkrems commented 4 years ago

Out of curiosity: Did you try the native tree-shaking for CommonJS in webpack 5? It should have the advantage of working across CJS/ESM boundaries.

budarin commented 4 years ago

I have tried - it does not work I have compared two build results: current wp4 & wp5 - wp5 does not tree shake commonjs: for an example: core.js is included into bundle entirely instead of needed modules as is with the plugin

jkrems commented 4 years ago

@budarin Do you have a repro / link to a webpack issue for this case? Maybe it's a configuration issue or something specific to core.js (I assume you mean the polyfills)? If the tree-shaking doesn't work for core-js, that feels worth fixing in wp5.

@sokra Are you aware of the common-shake plugin and/or do you know if the native support wp5 should "just work" as a drop-in replacement?

budarin commented 4 years ago

I can't share repo - it's company's project I simply have made upgrade webpack and have excluded the plugin and compare generated bundle sizes

Webpack 4 image

webpack 4 config gist

webpack 5 image

webpack 5 config gist

jkrems commented 4 years ago

Sorry, subtle difference "repro" vs "repo". In other words: do you know of a minimal set of files that triggers the behavior? It would be a decent chunk of work to touch up this plugin for WP5 with a very real risk that it would be almost immediately irrelevant as WP5 fixes those bugs. So I'd personally be much more likely to look at fixing the tree-shaking bug in Webpack than to work on the plug-in. But for that I'd need to know what the actual bug is. :)

The above is just from my perspective, other people involved with this plugin may feel differently.

sokra commented 4 years ago

Webpack 5 commonjs tree shaking is kind of work in progress. The foundation is there, but it doesn't support all syntax possible. That's something we plan to extend in future, but we have to be very careful as this is enabled by default and it must not break any kind of code.

A very common case that is not supported yet is e. g. const x = require("x"); x.y. Another case is module.exports = { y: 42, z: 24 }.

The most valuable contribution would be test cases especially edge cases for commonjs tree shaking. Maybe we can copy some from here?

sokra commented 4 years ago

Note that tree shaking in webpack also means that export names are mangled to shorter names, which makes it extra tricky. Another challenge is esm interop.

budarin commented 4 years ago

@sokra The plugin successfully deals with many problems - you could use the experience from the plugin