Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

YUI compression broken #31

Open
eddiefletchernz opened this issue Jul 1, 2014 · 9 comments
Open

YUI compression broken #31

eddiefletchernz opened this issue Jul 1, 2014 · 9 comments

Comments

@eddiefletchernz
Copy link

The following two snippets are located in the same function:

if (view.Blob && view.URL) {
        try {
            new Blob;
            return;
        } catch (e) {}
    }
view.Blob = function Blob(blobParts, options) {
        var type = options ? (options.type || "") : "";
        var builder = new BlobBuilder();
        if (blobParts) {
            for (var i = 0, len = blobParts.length; i < len; i++) {
                builder.append(blobParts[i]);
            }
        }
        return builder.getBlob(type);
    };

When run through a minification tool (I am using the latest YUI), the first function (which checks for the presence of Blob in the browser) is compressed as:

if(a.Blob&&a.URL){try{new d;return}catch(c){}}

This is correct minification, as further down the file, a named function "Blob" is defined in the local scope (and is minified to the name "d")

Because "new Blob" is minified as "new d", an error will occur regardless of the browser's Blob support, thereby overwriting it with the shim instead of using the native implementation.

You can run an online version of the YUI compressor here:
http://refresh-sf.com/yui/

To fix this issue, change:

view.Blob = function Blob(blobParts, options) {

to:

view.Blob = function (blobParts, options) {

I don't know if this will break anything else though. As far as I can tell it still works when you change this.

@eligrey eligrey closed this as completed in d71ed78 Jul 2, 2014
@diegocr
Copy link
Contributor

diegocr commented Jul 19, 2014

FYI: That is NOT correct minification. In fact, if there were any real issue on that, the non-minimized code would cause some problem as well. At run time "Blob" is checked from the global scope since the polyfill isn't yet created and therefore any minificator should take that into account.

My advice, switch to a more robust and well tested minification tool.

@eligrey eligrey changed the title Minifying Blob.js results in different functionality due to "view.Blob = Blob function (...)" YUI compression broken Jul 19, 2014
@eligrey
Copy link
Owner

eligrey commented Jul 19, 2014

Please file a bug with the YUI compressor and link it here if you want to resolve the root issue. I will revert to the previous named function code once this issue is fixed upstream with YUI.

@diegocr
Copy link
Contributor

diegocr commented Jul 19, 2014

yui/yuicompressor#155

I think there's no need to revert d71ed78, though.

@eligrey eligrey reopened this Jul 20, 2014
@eligrey
Copy link
Owner

eligrey commented Jul 20, 2014

Ok, thanks. I've subscribed to that issue.

@tml
Copy link

tml commented Jul 30, 2014

Because YUICompressor is implemented by compiling all JavaScript provided down to an AST (using Mozilla Rhino), there's no way to prevent YUICompressor from minifying browser-implementation objects like Blob short of using a parser hint, as mentioned in yui/yuicompressor#155. Some other JS compression engines do not actually compile the JS themselves, so they may not have this problem.

@eligrey
Copy link
Owner

eligrey commented Jul 30, 2014

Sounds like Rhino's AST representation is broken or YUI's handling of it, imo. The first Blob it runs into would be a different variable and scope from the next Blob.

@tml
Copy link

tml commented Jul 30, 2014

It's no different than the AST of any other javascript engine; node or spidermonkey would do the same things in this case.

@diegocr
Copy link
Contributor

diegocr commented Jul 30, 2014

For this code as posted on yui/yuicompressor#155

(function(view) {

    alert(typeof Test);

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

This is the relevant part of the AST representation:

"type" : "UnaryExpression",
"operator" : "typeof",
"argument" : {
    ...
    "type" : "Identifier",
    "name" : "Test"
},
...
"type" : "FunctionExpression",
"id" : {
    "loc" : null,
    "type" : "Identifier",
    "name" : "Test"
},

So, i highly doubt that's a bug on Rhino, but rather on YUICompressor wrongly interpreting the data.

@jimmywarting
Copy link
Contributor

close?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants