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

some minification performed even though minify not enabled #371

Closed
kzc opened this issue Sep 6, 2020 · 4 comments
Closed

some minification performed even though minify not enabled #371

kzc opened this issue Sep 6, 2020 · 4 comments

Comments

@kzc
Copy link
Contributor

kzc commented Sep 6, 2020

It doesn't appear to be possible to output code without at least some degree of minification - is this by design?

$ esbuild --version
0.6.31

Actual:

$ echo 'false, true, a = true && b, a = false || b, typeof !!7, typeof !!!x;' | esbuild
false, true, a = b, a = b, "boolean", typeof !x;

$ echo 'false, true, a = true && b, a = false || b, typeof !!7, typeof !!!x;' | esbuild --minify-whitespace
!1,!0,a=b,a=b,"boolean",typeof!x;

Expected:

$ echo 'false, true, a = true && b, a = false || b, typeof !!7, typeof !!!x;' | npx uglify-js
false,true,a=true&&b,a=false||b,typeof!!7,typeof!!!x;

Encountered when trying to verify non-minified whitespace-removed esbuild output of uglify tests against whitespace-removed input.

@evanw
Copy link
Owner

evanw commented Sep 6, 2020

This is intentional. It's constant folding, not minification. This is useful when combined with --define for compile-time flags. Not doing so can make bundling slower. For example, React contains this code:

if (process.env.NODE_ENV === 'production') {
  module.exports = require('./cjs/react-dom.production.min.js');
} else {
  module.exports = require('./cjs/react-dom.development.js');
}

Usually you use --define:process.env.NODE_ENV="development" in development. If constant folding is not done then the parser does not discover that the if statement condition is statically false, and the bundler ends up bundling both the production build and the development build.

@kzc
Copy link
Contributor Author

kzc commented Sep 6, 2020

It's not documented as such:

  --minify-whitespace   Remove whitespace

It breaks from convention for other minifiers like uglify and terser with compress disabled and Google Closure Compiler with WHITESPACE_ONLY.

Minification of syntax is generally understood to be a collection of standard compiler optimizations of which constant folding is one. One would not expect to optimize code with just --minify-whitespace enabled. It'd be used for debugging generally, or to verify parsing fidelity.

There seems to be a consistency issue as well - if you look at the top post, notice that true and false are handled differently with no flags versus --minify-whitespace.

Closing issue since it's by design.

@kzc kzc closed this as completed Sep 6, 2020
@evanw
Copy link
Owner

evanw commented Sep 6, 2020

Oh I see, you were probably talking about the true to !0 conversion. Yeah I could change that behavior I guess. The reason why that happens currently is that --minify-syntax rewrites the syntax in the parser while the printer currently only looks at --minify-whitespace but the true to !0 conversion happens in the printer.

@kzc
Copy link
Contributor Author

kzc commented Sep 7, 2020

The true/!0 observation was just a tangent.

esbuild performs some optimizations by default that cannot be disabled. It would be useful to have a new flag to not perform any optimizations or minification transforms of any kind to accurately output the code as input, providing functionality that other minifers presently support.

Proposed behavior:

$ echo 'false, true, a = true && b, a = false || b, typeof !!7, typeof !!!x;' | esbuild --no-opt
false, true, a = true && b, a = false || b, typeof !!7, typeof !!!x;
$ echo 'false, true, a = true && b, a = false || b, typeof !!7, typeof !!!x;' | esbuild --no-opt --minify-whitespace
false,true,a=true&&b,a=false||b,typeof!!7,typeof!!!x;

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

2 participants