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

Guides - Issues with Tree Shaking & Production #1331

Closed
jakearchibald opened this issue Jun 23, 2017 · 4 comments · Fixed by #1508
Closed

Guides - Issues with Tree Shaking & Production #1331

jakearchibald opened this issue Jun 23, 2017 · 4 comments · Fixed by #1508
Assignees

Comments

@jakearchibald
Copy link

I was trying to get tree shaking working. Here are a few notes about the tree shaking and production guides:

The article explains what tree shaking is (really well), but it doesn't really cover how to achieve it.

--optimize-minimize will do it, but that also throws in uglify, which can make it difficult to confirm if treeshaking has worked, and will throw errors if you're using ES2016+.

One of the guides should cover how to enable tree shaking without enabling other optimisations. This will help users confirm that code is being removed, without having to read minified code.

The production guide should cover how to avoid errors when using ES2016+, either by using the uglify harmony branch, or BabiliWebpackPlugin. None of the docs I've seen cover how to use BabiliWebpackPlugin instead of uglify while retaining tree shaking.

Tree shaking will break if using babel + the es2015 preset, as it transpiles the imports. I've filed babel/babel-loader#477, but it may be worth mentioning in the guide too.

@jakearchibald
Copy link
Author

The production guide suggests:

new webpack.LoaderOptionsPlugin({
  minimize: true,
  debug: false
})

…will perform tree-shaking, but that plugin appears to be deprecated, and doesn't appear to be handling tree-shaking.

@insin
Copy link

insin commented Jun 23, 2017

After playing about with this, it seems like Uglify is responsible for actually removing the unused code - Webpack's role in tree shaking appears to be avoiding generating __webpack_exports__[id] = whatever statements for unused exports so Uglify doesn't count them as "used" for the purposes of dead code elimination.

You can give UglifyJsPlugin {mangle: false} config to generate a build which makes it easier to confirm if code you expected to be removed has been.

@jakearchibald
Copy link
Author

Yeah, it seems to behave that way. Babili does the same.

I guess the docs led me to think the code removal was performed by webpack.

@skipjack skipjack changed the title Issues with tree shaking & production guides Guides - Issues with Tree Shaking & Production Jun 23, 2017
esbenp added a commit to esbenp/webpack.js.org that referenced this issue Aug 1, 2017
skipjack pushed a commit that referenced this issue Aug 3, 2017
Note that --optimize-minimize is not required for tree shaking.

Closes #1473
Ref #1331
dear-lizhihua referenced this issue in docschina/webpack.js.org Aug 5, 2017
Note that --optimize-minimize is not required for tree shaking.

Closes #1473
Ref #1331
@skipjack skipjack self-assigned this Aug 10, 2017
@skipjack
Copy link
Collaborator

skipjack commented Aug 10, 2017

After playing about with this, it seems like Uglify is responsible for actually removing the unused code - Webpack's role in tree shaking appears to be avoiding generating webpack_exports[id] = whatever statements for unused exports so Uglify doesn't count them as "used" for the purposes of dead code elimination.

@insin @jakearchibald I've seen a few other discussions recently that all came to the same conclusion as well. Essentially it's the combination of ES6 static import / export statements and the UglifyJsPlugin that enable tree shaking from my understanding. A warning was added recently in #1474 that starts to clarify this but I'll elaborate on this and address the other things mentioned above in the rewrites.

skipjack added a commit that referenced this issue Aug 11, 2017
This continues our work #1258 and resolves most of the issues mentioned
in #1331. It is a WIP though, we still need to populate the `TODO` code
blocks.
skipjack added a commit that referenced this issue Aug 26, 2017
This continues our work #1258 and resolves most of the issues mentioned
in #1331. It is a WIP though, we still need to populate the `TODO` code
blocks.
skipjack added a commit that referenced this issue Aug 26, 2017
This continues our work #1258 and resolves the issues mentioned
in #1331. The updates to tree-shaking were more just clean up and
synchronization while production included more significant changes
and a strong recommendation of `webpack-merge`. The old guide
pointed out that _you could_ just maintain two separate configs and
duplicate the overlap but I don't think we should recommend this.

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

Successfully merging a pull request may close this issue.

3 participants