Bug with hoist_funs?

[zylongaming1]

Bug report or feature request?
Bug

ES5 or ES6+ input?
ES5

Uglify version (uglifyjs -V)
uglify-js@2.8.23

JavaScript input

try
{
	const testvar = 1;
	function testfunc()
	{
		console.log(testvar);
	}
	testfunc();
}
catch(err)
{
	console.log(err);
}

The uglifyjs CLI command executed or minify() options used.
UglifyJS.minify("path-to-file.js");

JavaScript output or error produced.

function testfunc(){console.log(testvar)}try{const testvar=1;testfunc()}catch(t){console.log(t)}

which the output jsbeautifys to:

function testfunc() {
    console.log(testvar)
//error : testvar is not defined
}
try {
    const testvar = 1;
    testfunc()
} catch (t) {
    console.log(t)
}

It appears adding the following option "hoist_funs": false resolves the issue, as the function is able to read the const variable properly

I'm not entirely sure if this is intended, but if it is then hoist_funs should be disabled by default at the very least.

[zylongaming1]

Note
If you change const to var it works fine.

[kzc]

The problem stems from the fact that const is not part of the ES5 specification and was incorporated into uglify-js with var scope, which is incorrect. Arguably const support should be removed from uglify-js.

I'd suggest using uglify-es for ES6 minification - except for the fact it has the same bug.

As you discovered changing it to var will also work correctly.

[kzc]

Duplicate of harmony bug #1358

@alexlamsl What should we do with this fake ES5 const feature?

[zylongaming1]

I am rather "uninformed" with the different types of javascript. Didn't realize const was not part of ES5. I was thinking perhaps if the compiler detects a reference to a const variable within a function, simply leave the function where it is, instead of moving it. Why does uglifyjs even move functions?

[kzc]

Why does uglifyjs even move functions?

It provides better gzip compression when mangle is in effect.

[zylongaming1]

My main goal here is to just minify any javascript file safely regardless of ES5, ES6, etc

Would this be sufficient options in your opinon?

{compress:{"unused": false,"hoist_vars": false,"hoist_funs":false,"if_return":false,"join_vars":false,"loops":false}}

[alexlamsl]

@kzc now that I read up on https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/const I'm thoroughly surprised support for const is added to master at all, since it claims for instance IE only supports this in version 11.

I think I'll need to get an idea on how often that feature is actually used in the field, which I expect is not frequent enough if IE10- aren't cool with it.

But the nuclear option is certainly on the table for this feature on master, may be even 2.x (though a note in the docs as known issue is probably more appropriate for the latter.)

[alexlamsl]

@zylongaming1 ES6 language features would trip up even the parser for uglify-js, so anything non-ES5 you'll need to use uglify-es.

[zylongaming1]

so uglify-es is able to parse es5, and es6 with no issues?

[alexlamsl]

uglify-js supports ES5 & below, so if it fails to parse anything ES6 it is by design.

uglify-es should support the latest fashion by the name of JavaScript, so if it fails to parse something please open an issue so that anyone interested may go and fix it.

[kzc]

@alexlamsl I don't have a strong opinion on this matter. Obviously const should be correctly supported with ES6 block scope in uglify-es, but it's up to you what to do with const in the ES5-only uglify-js. It's non-standard and non-compliant as is - but it could be considered grandfathered behavior.

Maybe such use of const could error out in ES5-only uglify-js unless a new option is used to convert const to var - something like -c const_to_var. It would produce shorter output. Or is that just making the problem worse?

[alexlamsl]

@kzc change of scope with const_to_var would indeed set off fireworks 😎

I am more than happy to remove const from 3.x, add to docs as known issue to 2.x, then retain/reinstate whatever the shape of const is in harmony so it can be fixed properly there.

What I'm wondering is whether there are significant amount of code out there which claims to be ES5 but uses const. But if MDN is correct, then that should not be the case since most of those libraries support IE9, and IE<11 does not support const.

[kzc]

I am more than happy to remove const from 3.x, add to docs as known issue to 2.x, then retain/reinstate whatever the shape of const is in harmony so it can be fixed properly there.

I think that's the best option. There will still be fireworks but it will be easier to explain with the uglify-es alternative (once const block scope is fixed in harmony).

[alexlamsl]

#1910 and #1916 should address this issue for master and v2.x respectively.

After those two are merged, this issue will be harmony-specific.

[jampy]

2.8.24 is released on Github. When will it be published on npm?

I think this is the bug that broke all my recent production builds .

[alexlamsl]

@jampy not sure exactly what you mean, but if you are looking for const basically v2.x does not properly support it:

https://github.com/mishoo/UglifyJS2/blob/v2.x/README.md#note

As for the actual release itself - it was going through CI before I head out. When I get back in a few hours' time I will press the button 😉

[alexlamsl]

Fixed in uglify-es 3.0.12