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

Please upgrade to Webpack v5 #40

Open
budarin opened this issue Oct 13, 2020 · 8 comments
Open

Please upgrade to Webpack v5 #40

budarin opened this issue Oct 13, 2020 · 8 comments

Comments

@budarin
Copy link

budarin commented Oct 13, 2020

the current version is not compatible with Webpack 5

@jkrems
Copy link
Collaborator

jkrems commented Oct 15, 2020

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
Copy link
Author

budarin commented Oct 15, 2020

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
Copy link
Collaborator

jkrems commented Oct 15, 2020

@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
Copy link
Author

budarin commented Oct 16, 2020

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
Copy link
Collaborator

jkrems commented Oct 16, 2020

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
Copy link

sokra commented Oct 16, 2020

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
Copy link

sokra commented Oct 16, 2020

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
Copy link
Author

budarin commented Oct 16, 2020

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

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