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

Warn about potential minification issues? #186

Closed
Rich-Harris opened this issue Oct 13, 2015 · 3 comments · Fixed by #247
Closed

Warn about potential minification issues? #186

Rich-Harris opened this issue Oct 13, 2015 · 3 comments · Fixed by #247

Comments

@Rich-Harris
Copy link
Contributor

An interesting case with UglifyJS – if you use eval or with, the containing scope's variable names can't be mangled. Since Rollup puts every module in a single scope, it's much more likely that mangling will fail for the top level of an entire bundle, meaning that a minified Rollup bundle ends up larger than the equivalent Browserify bundle, despite there being less code. (Gzipping goes some way to alleviating this, but not all the way.)

Do we try and catch and deal with those cases somehow? E.g. warn, or wrap everything (except declarations) in an IIFE if a module contains a top-level eval or with? My thinking is that most people wouldn't investigate too deeply before concluding that Rollup was just less efficient than Browserify or Webpack.

Via mishoo/UglifyJS#830 (comment)

@kzc
Copy link
Contributor

kzc commented Oct 13, 2015

It would be pretty straightforward to add such a warning in uglifyjs when eval() or with is encountered when mangling is enabled.

I think the Rollup project should be made aware of this issue as it could be fairly common. Perhaps they can perform some code transforms on their end to avoid this situation in some cases. i.e., don't hoist a function from a module in certain cases.

@kzc
Copy link
Contributor

kzc commented Oct 13, 2015

Sorry - I mistakenly thought this was a post to the UglifyJS2 project.

@Rich-Harris
Copy link
Contributor Author

@kzc not at all! thank you for clarifying the issue.

I actually misunderstood the problem before – wrapping the eval-containing module in an IIFE wouldn't help, because the eval 'infects' all the parent scopes – instead, we'd need to wrap all the other modules to make them minifiable. Which makes that approach less appealing.

So, warning the user is probably a better solution.

@Rich-Harris Rich-Harris mentioned this issue Oct 22, 2015
1 task
Rich-Harris added a commit that referenced this issue Oct 31, 2015
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.

2 participants