invert reduce_vars tracking flag

[alexlamsl]

Modules like webpack and grunt-contrib-uglify still uses ast.transform(compressor) before Compressor.compress(ast) was introduced.

Workaround this compatibility issue by deactivating reduce_vars in such case.

fixes #1516

[alexlamsl]

@kzc I didn't know .compress() was a recent addition (c55dd5e, #1040) and assumed everybody is using that 😓

Anyway, with this PR what would happen is that if somebody uses ast.transform(compressor) then reduce_vars would be an noop, as passes currently would.

[kzc]

I see the need for the PR, but be aware that negative variable names lead to all sorts of bugs down the road - especially when the negative variable is negated with !.

[kzc]

I didn't know .compress() was a recent addition (c55dd5e, #1040) and assumed everybody is using that

Oh well. These things happen.

[alexlamsl]

@kzc agreed with the suboptimal naming - how about fixed_value?

[kzc]

how about fixed_value?

I'm afraid I do not know the purpose of this property. If you think that name matches its purpose, then sure.

[alexlamsl]

I'm afraid I do not know the purpose of this property.

It marks the variable as not being assigned/modified outside of var x = value.

[kzc]

I'm stumped to come up with a positive name for not_modified.

original is one possibility, but it has other meanings.

http://www.thesaurus.com/browse/original?s=t

[alexlamsl]

I've picked fixed in the end since it's short and hasn't been used in the codebase yet.

[alexlamsl]

OT: making a release for html-minifier actually involves grunt-contrib-uglify. But even with its latest available version it depends on uglify-js 2.7.5, so I'm confused as to why people were fetching 2.8.0 instead.

[kzc]

So this will re-enable reduce_vars in a way that will work with the low level transform() API?

[alexlamsl]

So this will re-enable reduce_vars in a way that will work with the low level transform() API?

It re-enables reduce_vars by default, but if someone calls transform() directly instead of compress(), then reduce_vars (and passes) would effectively be disabled.

[kzc]

I see. That makes sense.

Downstream low level Uglify API users would still have to update that line of code to make use of the compress options reduce_vars and passes.

[alexlamsl]

That is something to bear in mind going forward if/when somebody reports an issue about sub-optimal minification. But better that than completely broken output...

(I'm waiting on Travis CI, which seems to be experiencing major outage.)

[kzc]

Can you add this patch to this PR?

--- a/README.md
+++ b/README.md
@@ -621,7 +621,7 @@ function uglify(ast, options, mangle) {
 
   // Compression
   uAST.figure_out_scope();
-  uAST = uAST.transform(UglifyJS.Compressor(options));
+  uAST = UglifyJS.Compressor(options).compress(uAST);
 
   // Mangling (optional)
   if (mangle) {
@@ -865,7 +865,7 @@ toplevel.figure_out_scope()
 Like this:
 ```javascript
 var compressor = UglifyJS.Compressor(options);
-var compressed_ast = toplevel.transform(compressor);
+var compressed_ast = compressor.compress(toplevel);
 
 The `options` can be missing.  Available options are discussed above in

[kzc]

Pending the Travis CI tests, I assume you'll make a 2.8.2 release with this PR shortly?

[alexlamsl]

Pending the Travis CI tests, I assume you'll make a 2.8.2 release with this PR shortly?

That is my current thinking, unless you'd like a grace period in order to get some quality sleep, for instance?

[kzc]

I think it's fine for a 2.8.2 release once Travis CI runs successfully.

[alexlamsl]

(CI testing took 30 min 25 sec)

Edit: battery of test/Jetstream.js tests passed.
Cannot run test/benchmark.js due to HTTP 500 errors from http://builds.emberjs.com/tags/v2.11.0/ember.prod.js

[kzc]

@alexlamsl Since master has been tagged with v2.8.2 would you mind merging master into harmony and tagging harmony-v2.8.2? Can skip tagging 2.8.1 on harmony.

[alexlamsl]

@kzc thanks for the reminder - #1521