Problem handling function expressions.

[diegocr]

See this code:

(function(view) {

    alert(typeof Test);

    view.test = function Test() {
        console.log(Test);
    };
})(this);

It's minimized to:

(function (a) {
    alert(typeof b);
    a.test = function b() {
        console.log(b)
    }
})(this);

That's wrong, a named function expression must be obfuscated within the function itself only.

[tml]

Can you help me understand exactly what you were expecting as output from this?

[diegocr]

(function (a) {
    alert(typeof Test);
    a.test = function b() {
        console.log(b)
    }
})(this);

I think that's self-explanatory.

[tml]

I apologize, but it's not self-explanatory to me. Why are you expecting 'Test' to not get munged in the 'typeof' expression? I could perhaps understand if you expected:

(function (a) {
    alert(typeof b);
    a.test = function c() {
        console.log(c)
    }
})(this);

or something, but that obviously doesn't fix the problem you're bringing over from Blob.js.

[eligrey]

Why are you expecting it to be munged? Named function expressions in this situation do not get hoisted to the scope outside of themselves.

I hope this code helps illustrate the issue to you, and also explains why named function expession's names shouldn't be munged:

(function() {
    alert(typeof Blob);
    var not_blob = function Blob(){ return Blob; };
    alert(Blob === not_blob); // false, ^ that didn't redefine Blob
    alert(not_blob() === Blob); // false
}());

[diegocr]

at that point on the execution the typeof of Test is checked against the global scope, because our local Test is a named function expression which isn't yet declared.

If that were a normal function it'd be ok, since the JS engine does function hoising, I.e:

(function(view) {

    alert(typeof Test);

    function Test() {
        console.log(Test);
    };
})(this);

However, that isn't the case and therefore the YUI compressor is obviously buggy.

[diegocr]

Oh, didn't saw Eli post - well said man :)

However, (Blob.name === not_blob.name) may not always return true.

[tml]

Like I said: I can see an argument that they should be munged to different values. I cannot see an argument that the first Test should not be munged. Nothing in what either of you have said here have clarified for me why you think

(function(view) {
  alert(typeof Blob);
})(this);

should refuse to munge Blob (which is what @diegocr suggested in his first response).

[diegocr]

Let's say you have to think like the JS engine parsing the code. At runtime - like i said - the typeof Test is checked from the global scope because within that function there is no other code processed before the alert() call.

I'm not sure how to clarify it better.. any O'Reilly book on the matter could help you understand it deeply :)

[tml]

YUICompressor doesn't just "think like the JS engine parsing the code", it IS a JS engine parsing the code. The global scope in YUICompressor is the script you fed into YUICompressor.

[diegocr]

Then, it's a bogus JS engine :)

In any case, I meant it's you who have to think like a JS engine, to be able to understand what we're talking about here, with all due respect.

[tml]

I AM thinking like a JS engine - namely, Mozilla's Rhino JS engine. If your problem is with their JS engine, you can file a bug with upstream. :)

I agree that there is a bug here on the named function expression not generating a new scope (causing it to re-use the same munge set as the previous instance of Test), and am working on a fix for that now. Expecting YUICompressor to not consider the script given it to be the definition of the global scope of the javascript that will be run is a fundamental change in the behaviour of YUICompressor. You can get your desired behaviour by telling YUICompressor not to munge Test via the "nomunge" hint:

(function(view) {
    "Test:nomunge";
    alert(typeof Test);

    view.test = function Test() {
        console.log(Test);
    };
})(this);

[diegocr]

I'll be pleased to file a bug with Mozilla if we prove that's the origin of the problem :)

[gerardpaapu]

For this code:

(function g(a, b, v) {
    var f = function g() {};
    console.log(g.length);
}());

yui-2.4.8 emits code equivalent to the following (I've only added whitespace).

(function g(d,c,e){
    var i = function h(){};
    console.log(h.length); // h is not defined in this scope
}());

reference error: h is not defined

I don't have any knowledge of yui-compressor's internals, but it appears to be confusing or conflating function expressions and function declarations, i.e. function expressions are affecting their containing scope.

(function () {
    // declared foo in this scope
    function foo() {
    }

    // this is a function expression that has no effect on its containing scope
    (function bar() { 
        // but it IS declared inside the function body
        return bar;
    });

    foo(); // this is legit
    bar(); // ReferenceError: bar is not defined
}());

[tml]

Yes, Rhino doesn't make a distinction between the two (function expressions and function declarations) - they're the same from the POV of the de/compiler. I have been trying to make Rhino aware of the difference, but it's not a trivial problem, and it's behind a few other high-profile issues. If one of you has the time and inclination, a Pull Request would be welcome. :)

[diegocr]

Sorry, but they don't look the same on my end. See this code:

function g() {}
var f = function g() {};

whose AST representation for the relevant part is as follow:

            "type" : "FunctionDeclaration",
            "id" : {
                "loc" : null,
                "type" : "Identifier",
                "name" : "g"
            },
....
                        "type" : "FunctionExpression",
                        "id" : {
                            "loc" : null,
                            "type" : "Identifier",
                            "name" : "g"
                        },
....

Their type is clearly different.

/me just trying to bring some light :-)

[tml]

At the stage that YUICompressor hooks into it today, Rhino's decompiler has neither the concept of "FunctionDeclaration" nor of "FunctionExpression", only "Token.FUNCTION"; both declarations and expressions get lumped into the same token at decompile. Making the right part of the decompiler aware of this distinction is the aforementioned work.

[diegocr]

Interesting, I was sure i must have been missing something. Well, perhaps for - let's say - version 3.0 it should be considered changing how it hooks into it, doing so in a way that is no longer error prone.