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

Discussion: Reconsider showing breakdown of concatenated modules #188

Closed
valscion opened this issue Jun 6, 2018 · 6 comments
Closed

Discussion: Reconsider showing breakdown of concatenated modules #188

valscion opened this issue Jun 6, 2018 · 6 comments

Comments

@valscion
Copy link
Member

valscion commented Jun 6, 2018

I'm opening this issue to consolidate discussion around showing contents of concatenated modules.

Original issues and implementation pull requests:

Problems and concerns related to this feature:


Quick summary of #161, quoting @mpontus #161 (comment), emphasis mine:

This change however is not reflected in the grid drawn by webpack-bundle-analyzer. If I'm interpreting it correctly, it still shows as if all modules from "ramda/es" have been included into the bundle.

Individual import:

Destructuring import

I believe parsed size is accurate.

If I change the code to use a few other functions imported from "ramda/es" like this:

import { add, concat, compose } from "ramda/es";

document.write(compose(concat("Hello"), add(2))(3));

The crux of the issue can be summed up by @th0r's comment here #161 (comment):

The thing is actual dead code elimination is done by UglifyJSPlugin, not webpack itself. Webpack just packs and annotates source code in the way that UglifyJS will be able to determine unused code and remove it from the resulting bundle.

This is the reason you see a lot of actually "excluded" modules under concatenated module - webpack concatenated them all but UglifyJS removed all the unused ones.

Given that we are now in a situation where webpack-bundle-analyzer might show incorrect information instead vs. "less information", we should reconsider if the tradeoff of showing breakdown of concatenated modules contents is a good thing after all.

This tradeoff can be one of the reasons why the upcoming webpack-bundle-analyzer support for create-react-app could be removed: facebook/create-react-app#4563 (comment)

/cc @bugzpodder @th0r @joshwcomeau @gaearon

@th0r
Copy link
Collaborator

th0r commented Jun 6, 2018

@sokra can we do something about it? Can webpack provide more accurate data about contents of concatenated modules?

@valscion
Copy link
Member Author

valscion commented Jun 6, 2018

I fear that until webpack can do dead-code elimination the same way UglifyJS can, we'll be a bit out of luck about this 😕

@valscion
Copy link
Member Author

I think we should remove support for showing tree shaken module information, as it is misleading. See another example by @generalov here: facebook/create-react-app#4563 (comment)

What do you think, @th0r? Should we disable this feature until we can figure out of webpack supports not showing tree-shaken modules?

@valscion valscion mentioned this issue Sep 6, 2018
@valscion
Copy link
Member Author

This issue has been resolved by the new v3 version 🎉

@simeyla
Copy link

simeyla commented Dec 21, 2018

@valscion what does 'solved' mean? I'm new to the analyzer. I see there's a checkbox to show it but it may be inaccurate. Is that what you meant? I assume the same inaccuracies with respect to UglifyJS still apply? Thx.

@valscion
Copy link
Member Author

Yes there are inaccuracies but now it's communicated that inaccuracies exist, and that it is no longer the default.

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

No branches or pull requests

3 participants