mishoo / UglifyJS

JavaScript parser / mangler / compressor / beautifier toolkit
http://lisperator.net/uglifyjs/
Other
13.17k stars 1.25k forks source link

Property mangling does not always mangle global properties #5953

Open AshleyScirra opened 1 month ago

AshleyScirra commented 1 month ago

Uglify version (uglifyjs -V) 3.19.3

JavaScript input

self.MyGlobal = 1
console.log(self.MyGlobal);

console.log(self.MyGlobal2);

The uglifyjs CLI command executed or minify() options used.

uglifyjs input.js --mangle-props --beautify --output output.js

JavaScript output or error produced.

self.l = 1;
console.log(self.l);

console.log(self.MyGlobal2);

Expected result: both self.MyGlobal and self.MyGlobal2 to be treated consistently, both being renamed to a shorter property.

Observed result: only self.MyGlobal is mangled. For some reason self.MyGlobal2 is not mangled. Perhaps this is because the parser has not seen an assignment to the global property yet.

Impact: This breaks property mangling when using multiple files or a shared nameCache: it's possible MyGlobal2 was assigned to in a different script. For example if input1.js does self.MyGlobal2 = 2, then input2.js does console.log(self.MyGlobal2), the properties are not consistently mangled and the output is broken. Therefore to ensure property mangling works in these cases, UglifyJS must consistently mangle all global properties regardless of whether they are assigned to or only read from.

alexlamsl commented 1 month ago

So this would work as you expected if we use --mangle-props globals a.k.a. #5933:

$ cat test.js
self.MyGlobal = 1;
console.log(self.MyGlobal);
console.log(self.MyGlobal2);
$ uglify-js test.js -b --mangle-props
self.l = 1;

console.log(self.l);

console.log(self.MyGlobal2);
$ uglify-js test.js -b --mangle-props globals
self.l = 1;

console.log(self.l);

console.log(self.o);

I realise I have yet to make a new release which includes that feature, will do in the next few days.

As for your concern on consistency, --name-cache seems to be working as expected:

$ cat one.js
self.MyGlobal = 1;
$ cat two.js
console.log(self.MyGlobal);
$ uglify-js one.js --name-cache names.json --mangle-props
self.l = 1;
$ uglify-js two.js --name-cache names.json --mangle-props
console.log(self.l);
AshleyScirra commented 3 weeks ago

Thanks, let me know when the next version is out and I'll check again with the new globals option.