Minifiying functions declared inside if blocks fails

Feb 4, 2013 at 11:10 AM
In version 4.67 the minifiying of functions defined inside if statements inside orher functions work. In the latest versions, these functions gets assigned the same letter already used by other minified variables, ex.:

(function(OpenLayers){
OpenLayers.Format.JSON = OpenLayers.Class(OpenLayers.Format, {
...
read: function(json, filter) {
  ...
  if (...) {
      function walk(...) {
      }
  }
  ...
}
}
})(OpenLayers)

is minified something like this:

(function(n){
n.Format.JSON = n.Class(n.Format, {
...
read: function(n, t) {
  ...
  if (...) {
      function n(...) {
      }
  }
  ...
}
}
})(OpenLayers)

The error here is that the function 'walk' is assigned an minified letter already used here 'n'.
Coordinator
Feb 4, 2013 at 1:22 PM
Yeah, sorry about that. I had already noticed and checked in a fix last week; it'll be in the next release.

On a side note, it is technically invalid JavaScript to put a function declaration anywhere but within a source-element level block, at least until ECMAScript 6 support is official - and when that happens, it's not going to behave as you might expect, because then those functions will be scoped only inside the if-block (which is why this issue is happening - I wrote the new scoping code to be ES6-compliant). And regardless of ES6, even though most browser implementations let you put a function inside blocks like that, browsers treat them differently, which can lead to very hard-to-find cross-browser issues. It's not a good idea at this point to put function declaration inside if-statements.
Feb 4, 2013 at 5:26 PM
Yes, You're right about the scoping issue, and it's what we expected was the reason for this (and because of that we fixed our code multiple places, so that's good.. :) ). The side effect here was third libraries that we don't always have quite so much control over that caused some extra issues. Will the fix be an option/switch so that we have the possibility to handle 3rd libraries differently from our own source code?
Coordinator
Feb 4, 2013 at 5:49 PM
The fix just makes sure a function defined in a lexical scope doesn't get renamed to collide with anything in the variable scope. But I was also going to throw a low-sev warning for the positioning, even though it's valid ES6, just because so many people tend to write code like this right now. You'll still be able to ignore the warning using the -ignore or -w switches if you need to.
Feb 4, 2013 at 6:22 PM
That's a good solution. Thanks, Ronlo.. :-) (and nice work by the way! :-) )
Feb 5, 2013 at 8:29 AM
By the way, I have a problem with minified code working on Firefox but not in Chrome her. It may very well be related to the topic already discussed here
(function (xx, yy){
  function private (arg1, arg2) {
    if (arg1...)
  }
   ...
  yy.register (arg1, arg2) {
    private(arg1, arg2)
  }
  ...
})(XX, YY);
This is translated to something like:
(function (a, b){
  function c(c, d) {
    if (c...)
  }
   ...
  b.register (e, f) {
    c(e, f)
  }
  ...
})(XX, YY);
The problem here is the private functions name, and the first parameter gets the same minified name. In Firefox this seems to work alright, the parameter 'overrides' the functions name, but in chrome the private functions name wins, so the if statement inside the function breaks down. As said this may be related to the same issue already discussed, since this worked on older versions (4.67).
Coordinator
Feb 5, 2013 at 2:50 PM
Fascinating! I'll run some experiments and see what I can find.
Coordinator
Feb 5, 2013 at 3:00 PM
Yeah, I think it might be something else. I created a web page containing this script:
    (function (a, b)
    {
        function c(c, d) 
        {
            console.log("inside: typeof c=" + typeof c + "; c=" + c);

            if (c)
            {
                d.write("<h1>"+c+"</h1>")
            }
        }

        console.log("outside: typeof c=" + typeof c + "; c=" + c);
        c(a,b);

    })("one", document);
and it works fine in all the browsers I tested (IE, Chrome, FF, Opera, Safari). I bet what you're seeing is c is getting corrupted by some other function declaration somewhere within the outer function, not from the function itself. It should be an AjaxMin error that will be corrected today with version 4.82.