Uglifyjs: unused functions are not removed

[edwardcoolson]

I'm new to uglifyJS, it seems that it does not remove unused functions. I have this simple code:

function isEmpty(o)
{
  for(var key in o)
    if(o.hasOwnProperty(key))
      return false;
  return true;
} 

function isBrowserIE()
{
  return navigator.userAgent.toLowerCase().indexOf('msie') > -1;
}

var o = {};
o.prop1 = 5;
o.prop2 = 10;

var b = isEmpty(o);
alert(b);

I've minified this code online here: http://lisperator.net/uglifyjs/
and got the following code:

function isEmpty(r){for(var o in r)if(r.hasOwnProperty(o))return!1
return!0}function isBrowserIE(){return navigator.userAgent.toLowerCase().indexOf("msie")>-1}var o={}
o.prop1=5,o.prop2=10
var b=isEmpty(o)
alert(b)

As anyone can see, the unused function isBrowserIE() is still there. I used default compression options. The compression option Discard unused variables/functions was turned on by default.

Is it a bug or just kept for future releases?
Thank you.

[Skalman]

Your code defines a function isBrowserIE() on the global object, and if Uglify removed the function it would no longer be globally defined – i.e. the code would do something different. Try wrapping everything in an IIFE so that the function isn't globally defined:

(function () {
  function isEmpty(o)
  {
    for(var key in o)
      if(o.hasOwnProperty(key))
        return false;
    return true;
  }

  function isBrowserIE()
  {
    return navigator.userAgent.toLowerCase().indexOf('msie') > -1;
  }

  var o = {};
  o.prop1 = 5;
  o.prop2 = 10;

  var b = isEmpty(o);
  alert(b);
}());

[michaelficarra]

Top level declarations are observable. They add their names to the global object.

[edwardcoolson]

Okay, in our example with all the functions at our fingertips it is quite possible. But in real life I'm using a bunch of js files (may be external). There can be globally defined functions like isBrowserIE() in those files. I use ones and don't use others. In fact, I'm using only a small part of those functions, say 10 percents of the whole lot.

How can I write my code to get rid of those 90% unused functions? With uglifyJS it seems that all that stuff will directly go in my production code. Is it some way to solve this? (Manual removal of unused functions is not a solution for me.)

[edwardcoolson]

@Skalman You wrote: "... if Uglify removed the function it would no longer be globally defined". It's exactly what I want! My code decides what to use and what not! There is no other user of the code except me. So if I don't use a function I don't need it at global scope! Please remove it from my production code!

[Skalman]

@edwardcoolson I also wrote "Try wrapping everything in an IIFE so that the function isn't globally defined". An IIFE (immediately-invoked function expression) is written like this:

(function () {
  ...all your code...
)());

[edwardcoolson]

@Skalman So I guess I need these automation steps:

• concatenate all external js code + my js code that uses it
• wrap that all inside IIFE
• minify that IIFE with uglifyJS

Sounds reasonable. Thank you Dan.
But to me, if there was an option in uglifyJS to remove unused global functions it would be more straightforward.

By the way, what automation tool is better for this, Grunt or Gulp?

[avdg]

Both automation tools should be fine, they are practically the same for basic usage.

[edwardcoolson]

Thank you guys

[edwardcoolson]

@Skalman I wrapped my code inside IIFE -- unfortunately unused functions are still not removed! As an example I have this minimum code that still reproduces the error. As you see, all is inside IIFE but this hapless isBrowserIE() function that I don't use (BTW I don't use any other function in this example) is still there. This is the original code:

(function() {

var JSON = JSON || {};

JSON.stringify = JSON.stringify || function (obj)
{ 
  var t = typeof (obj); 
  if (t != "object" || obj === null) { 

    // simple data type 
    if (t == "string") obj = '"'+obj+'"'; 
    return String(obj); 
  } 
  else { 

    // recurse array or object 
    var n, v, json = [], arr = (obj && obj.constructor == Array); 

    for (n in obj) { 
      v = obj[n]; t = typeof(v); 

      if (t == "string") v = '"'+v+'"'; 
      else if (t == "object" && v !== null) v = JSON.stringify(v); 

      json.push((arr ? "" : '"' + n + '":') + String(v)); 
    } 

    return (arr ? "[" : "{") + String(json) + (arr ? "]" : "}"); 
  } 
}

JSON.parse = JSON.parse || function(str)
{
  //if (str === "") str = '""';

  try {
    eval("var p=" + str + ";");
  }
  catch(e) {
    throw new SyntaxError('JSON.parse');
  }
  return p;
}

function isBrowserIE()
{
  return navigator.userAgent.toLowerCase().indexOf('msie') > -1;
}


function MyObject(a, b)
{
  var a_ = a;
  var b_ = b;

  this.sum = function()
  {
    return a_ + b_;
  }
}

var o = new MyObject(5, 10);
var s = o.sum();

})();

This is minified code:

!function(){function isBrowserIE(){return navigator.userAgent.toLowerCase().indexOf("msie")>-1}function     MyObject(r,t){var n=r,e=t
this.sum=function(){return n+e}}var JSON=JSON||{}
JSON.stringify=JSON.stringify||function(r){var t=typeof r
if("object"!=t||null===r)return"string"==t&&(r='"'+r+'"'),r+""
var n,e,i=[],o=r&&r.constructor==Array
for(n in r)e=r[n],t=typeof e,"string"==t?e='"'+e+'"':"object"==t&&null!==e&&    (e=JSON.stringify(e)),i.push((o?"":'"'+n+'":')+(e+""))
return(o?"[":"{")+(i+"")+(o?"]":"}")},JSON.parse=JSON.parse||function(str){try{eval("var      p="+str+";")}catch(e){throw new SyntaxError("JSON.parse")}return p}
var o=new MyObject(5,10),s=o.sum()}()

So I come to conclusion that uglifyJS does not remove unused functions even inside IIFE.

[Skalman]

@edwardcoolson, the reason is that you are using eval(). As far as Uglify knows, you might be using isBrowserIE(). Actually, the following snippet would call the function:
JSON.parse('{notReallyJson:isBrowserIE()}')

You have some other bugs in your code too, e.g. var JSON = JSON || {}; doesn't do what you want it to do, because variable declarations are hoisted. That will be interpreted as:

var JSON;
JSON = JSON || {};

You probably mean var JSON = window.JSON || {} or perhaps var JSON = window.JSON = window.JSON || {}.

[edwardcoolson]

@Skalman Thank you Dan. The code:

var JSON = JSON || {};

was designed to be used in global scope not inside IIFE. When wrapping into IIFE as you rightly noted it should be replaced for:

var JSON = window.JSON || {};

So, in other words, uglify's requirement to wrap all code into IIFE -- to be able to remove unused functions -- causes serious side effects. On the other hand, not wrapping all code into IIFE does not remove unused functions altogether. So, uglifyJS turns out to be useless for my use case. (Remember, I use bunch of external code that I don't want to put my hands on.)

By the way, the corrected statement

var JSON = window.JSON || {};

if wrapped into IIFE also does not remove isBrowserIE() for modern browsers that have JSON natively supported! The function body with that devil's eval():

JSON.parse = JSON.parse || function(str)
{
  //if (str === "") str = '""';

  try {
    eval("var p=" + str + ";");
  }
  catch(e) {
    throw new SyntaxError('JSON.parse');
  }
  return p;
}

is not defined if JSON.parse() is natively supported! But uglifyJS does not trouble itself to analyze this.

Verdict. To be able to safely & effectively use uglifyJS I need the compress option 'Remove top level unused functions/vars'. Let it be turned off by default, if you are so bothered about top level names availability.

Thank you.

[michaelficarra]

@edwardcoolson You're using a direct call to eval. There's no way we could know that you didn't ever intend to access your isBrowserIE function through that. You should be able to resolve your problem by replacing the direct call to eval with an indirect call to eval: (0, eval)(...).

[alexlamsl]

The original code now works with -c toplevel:

$ uglifyjs test.js -mc toplevel
WARN: Dropping unused function isBrowserIE [test.js:9,9]
function isEmpty(r){for(var o in r)if(r.hasOwnProperty(o))return!1;return!0}var o={};o.prop1=5,o.prop2=10;var b=isEmpty(o);alert(b);