terser/terser

properties are not mangled if it's in a deferred top level object

gdh1995 opened this issue · 3 comments

Bug report or Feature request? Bug report

Version (complete output of terser -V or specific git commit) from v3.14.1 to the latest v4.1.2

code to run

#!/usr/bin/env node
"use strict";
var opts = {
  "mangle": {
    "properties": true,
    "toplevel": true
  },
  nameCache: { vars: {}, props: {} }
};

var terser = require('terser');
terser.minify('var Bar = {};', opts);
var result = terser.minify('var Foo = { foo: function() { return Bar.bar() } };', opts);
var expected = "var r={o:function(){return a.v()}};";
console.log("[task] test uglifying external properties",
    "\n[Current ]", result.code, "\n[Expected]", expected);

expected result

var r={o:function(){return a.v()}};

real result

var r={o:function(){return a.bar()}};

why this is a bug

In a real case, if a Bar is defined in some later files, I want to pre-define Bar to make terser uglifiy this name, and I also want to uglify all its properties. But I don't like to pass all input files to terser during one calling to .minify

  • I'm using Gulp, and the file which defines Bar is in another task
  • currently, I add an extra step to call terser.minify("var Bar = {};") by myself, and abort its output
    • so that the gulp task's output keeps smallest.

This usage works well under terser v3.10.*, and something becames wrong since terser v3.14.1 .

I've read #219 , but I think my usage is still worth a new if-branch.

debugging

I find a commit in v3.14.1 seems to blame:

https://github.com/terser-js/terser/blob/b4dda6d2ccdc94ac42f6dbf459af50ac343f9454/lib/propmangle.js#L168-L173

When I reverted it to a simple add(node.property);, this bug disappeared.

The old issue message is here: it's too long and somehow incorrect, so I move it here just to archive it.

description

When I compiled Vimium C at commit gdh1995/vimium-c@c7cb108 using different versions of terser, the terser v4.1.2 cause null pointer exception, and after debugging, I found some properties are not mangled unexpectedly. After further tests, I've confirmed:

  • all versions since v3.14.1, including v3.16.0, v3.16.1, v3.17.0, v4.1.2, have such a bug
  • while v3.14.0, v3.13.1, v3.11.0, v3.10.12 and v3.10.11 works well (at least only caring about the function for mangling properties)

I tested removing it in v3.14.1 but this bug still exists.

error details

Vimium C has quite a few steps to compile it:

  1. firstly, compile all TypeScript scripts to JavaScript and save them in a folder of dist/.build/
  • the gulp task is gulp build/ts
  1. the next step is to uglify files in the dist/.build/lib and dist/.build/content folder
  • the gulp task which uses terser is named "min/content"
  • in this step, terser generates good results (at least all wanted properties are mangled)
  1. the third step is to uglify files in dist/.build/background, and some other steps are for dist/.build/pages and dist/.build/front
  • the gulp tasks are min/bg and min/others
  • here, all config data passed to terser is just the same as the task min/content in the step 2,
    except that I load a "nameCache" dict and setup config.nameCache to it,
    • the cache looks like { vars: {...}, props: {...}, timestamp: 0 } - vars and props are from terser's output in step 2, and they will be re-wrapped and passed to terser.

And:

  • I removed the extra nameCache by running ENABLE_NAME_CACHE=0 DEBUG=1 gulp min/bg, this bug also occurred.
  • I tried modifing Gulpfile.js and runs min/bg ignoring min/content, but it was still there.

Hey there!

Nice extension :)

What you described is not exactly a bug. Terser has no way of knowing whether lowercase bar is a property that exists.

What I found though, is that if I create another artificial file and use Bar.bar in any way, Terser does not "find out" that bar is a property, so it doesn't minify .bar in the third code example. Like this:

#!/usr/bin/env node                                                                           
"use strict";                                                                                 
var opts = {                                                                                  
  "mangle": {                                                                                 
    "properties": true,                                                                       
    "toplevel": true                                                                          
  },                                                                                          
  nameCache: { vars: {}, props: {} }                                                          
};                                                                                            
                                                                                              
var terser = require('./dist/bundle.js');                                                     
terser.minify('var Bar = {};', opts);                                                         
terser.minify('Bar.bar = 123', opts);  // <- this is what I added                                                         
console.log(opts.nameCache)                                                                   
var result = terser.minify('var Foo = { foo: function() { return Bar.bar() } };', opts);      
var expected = "var r={o:function(){return a.v()}};";                                         
console.log("[task] test uglifying external properties",                                      
    "\n[Current ]", result.code, "\n[Expected]", expected);                                   

I suppose this is the bug you're trying to report. In that case, sounds like an interesting bug. And indeed I want this to be a Terser feature. When this is fixed I'll ensure there's a test that protects your use case.

Thanks for your report!

Thanks.

idea 1

If it's hard to tell whether a variable is from outsides, I think a new option may be suitable to leave this problem to final users.

idea 2

For the project I mentioned above, it's in TypeScript and I've applied many "strict" rules, so I have a terser config looking like this (from https://github.com/gdh1995/vimium-c/blob/master/scripts/uglifyjs.dist.json):

var config = {
  "compress": {
    "properties": true,
    "pure_getters": true,
    "toplevel": false,
  },
  "mangle": {
    "properties": {
      "regex": /^_|_$/
    },
    "toplevel": true,
    "reserved": [
      // # expected names in web-extension content
      "WeakSet", "Set",
      // # expected names in 3rd-party extensions' contents
      "requestIdleCallback",
      // # content global names:
      "browser",
      "VimiumInjector", "VDom", "VKey",
      "VHints", "VScroller", "VOmni", "VFind", "VVisual", "VMarks",
      "VHud", "VPort", "VEvent",
      // ...
    ],
    // I've ensured all other global names of my code MUST start / end with "_",
    // and they are mangled correctly by terser v3.10.*.
  }
};

And as said in the comment, only those global variables name which starts/ends with "_" will be mangled, so I wonder if terser may think "all matched global names and their properties visits should be uglified" when it knows:

  • "not all" properties are expected to be mangled
  • or, some global variables have been mangled, and pure_getters is true.