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

Yuglify fails to parse jquery.js #19

Open
jdufresne opened this issue Apr 12, 2014 · 22 comments
Open

Yuglify fails to parse jquery.js #19

jdufresne opened this issue Apr 12, 2014 · 22 comments

Comments

@jdufresne
Copy link

@jdufresne jdufresne commented Apr 12, 2014

Steps to reproduce:

$ rm -rf bower_components && bower cache clean && bower install jquery#~1.11
bower deleted       Cached package jquery: /home/jon/.cache/bower/packages/fe2fe255e91d251051d543998aa8327a/1.11.0
bower jquery#~1.11          not-cached git://github.com/jquery/jquery.git#~1.11
bower jquery#~1.11             resolve git://github.com/jquery/jquery.git#~1.11
bower jquery#~1.11            download https://github.com/jquery/jquery/archive/1.11.0.tar.gz
bower jquery#~1.11             extract archive.tar.gz
bower jquery#~1.11            resolved git://github.com/jquery/jquery.git#1.11.0
bower jquery#~1.11             install jquery#1.11.0

jquery#1.11.0 bower_components/jquery

$ yuglify bower_components/jquery/dist/jquery.js 
Compressing bower_components/jquery/dist/jquery.js...

/home/jon/node_modules/yuglify/bin/yuglify:111
                    throw(err);
                          ^
Error
    at new JS_Parse_Error (/home/jon/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:263:18)
    at js_error (/home/jon/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:271:11)
    at croak (/home/jon/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:733:9)
    at token_error (/home/jon/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:740:9)
    at unexpected (/home/jon/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:746:9)
    at /home/jon/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:1112:13
    at maybe_unary (/home/jon/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:1209:19)
    at expr_ops (/home/jon/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:1236:24)
    at maybe_conditional (/home/jon/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:1240:20)
    at maybe_assign (/home/jon/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:1264:20)
    at /home/jon/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:1278:20
    at vardefs (/home/jon/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:1065:32)
    at var_ (/home/jon/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:1077:26)

This failure to parse occurred some time between jQuery 1.10 and 1.11 as 1.10 parses fine:

$ rm -rf bower_components && bower cache clean && bower install jquery#~1.10
bower deleted       Cached package jquery: /home/jon/.cache/bower/packages/fe2fe255e91d251051d543998aa8327a/1.10.2
bower jquery#~1.10          not-cached git://github.com/jquery/jquery.git#~1.10
bower jquery#~1.10             resolve git://github.com/jquery/jquery.git#~1.10
bower jquery#~1.10            download https://github.com/jquery/jquery/archive/1.10.2.tar.gz
bower jquery#~1.10             extract archive.tar.gz
bower jquery#~1.10            resolved git://github.com/jquery/jquery.git#1.10.2
bower jquery#~1.10             install jquery#1.10.2

jquery#1.10.2 bower_components/jquery

$ yuglify bower_components/jquery/jquery.js 
Compressing bower_components/jquery/jquery.js...
Successfully generated JS file: bower_components/jquery/jquery.min.js

I submitted a bug to jQuery, but they believe this is a Yuglify bug. See: http://bugs.jquery.com/ticket/14998

audax added a commit to audax/pypo that referenced this issue Apr 21, 2014
@pmclanahan

This comment has been minimized.

Copy link

@pmclanahan pmclanahan commented May 1, 2014

Seeing same issue with jQuery 2.1.0.

@dorey

This comment has been minimized.

Copy link

@dorey dorey commented May 21, 2014

I'm finding it has to do with an exclamation mark at the beginning of a comment in the middle of a variable assignment. (Happens in jQuery when Sizzle is defined)

/*! this comment is okay */
X = /* and this is okay */ Y;
X = /*! <-- but the "!" here causes an error */ Y;
@dorey

This comment has been minimized.

Copy link

@dorey dorey commented May 22, 2014

It is fixed by @leik 's fix suggested in this pull request which fixes the case when an IIFE follows the comment.

On testing it again, it looks like it's not fixed by that pull request.

@wreiske

This comment has been minimized.

Copy link

@wreiske wreiske commented Jun 1, 2014

Still having an issue with this with Jquery-1.11.1.js

Compressing ./js/jquery-1.11.1.js...

/usr/local/lib/node_modules/yuglify/bin/yuglify:111
                    throw(err);
                          ^
Error
    at new JS_Parse_Error (/usr/local/lib/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:263:18)
    at js_error (/usr/local/lib/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:271:11)
    at croak (/usr/local/lib/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:733:9)
    at token_error (/usr/local/lib/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:740:9)
    at unexpected (/usr/local/lib/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:746:9)
    at /usr/local/lib/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:1112:13
    at maybe_unary (/usr/local/lib/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:1209:19)
    at expr_ops (/usr/local/lib/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:1236:24)
    at maybe_conditional (/usr/local/lib/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:1240:20)
    at maybe_assign (/usr/local/lib/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:1264:20)
    at /usr/local/lib/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:1278:20
    at vardefs (/usr/local/lib/node_modules/yuglify/node_modules/uglify-js/lib/parse-js.js:1065:32)

You can easily get this to work by removing the comment blocks including the /*! comments.

Beginning of the file, and where it shows "sizzle". Once they are removed yuglify works as normal.

@ewjoachim

This comment has been minimized.

Copy link

@ewjoachim ewjoachim commented Jun 12, 2014

I'm willing to downgrade my jquery and/or yuglify versions if that can make it work (its far easier for me than modifying the code of jquery, because we don't really include the code for thirdparties in our projects. Do anyone know what versions of jquery/yuglify should I use, while waiting for the fix ? Thank you !

@Matt-Deacalion

This comment has been minimized.

Copy link

@Matt-Deacalion Matt-Deacalion commented Jun 25, 2014

@ewjoachim works ok with jquery version 2.0.3.

@jensenbox

This comment has been minimized.

Copy link

@jensenbox jensenbox commented Jul 21, 2014

I can confirm that removing the two comment blocks starting with /*! resolves the compile issue.

@mrcoles

This comment has been minimized.

Copy link

@mrcoles mrcoles commented Oct 13, 2014

Considering that jQuery 2.x doesn't support IE8, lots of people will still be using jQuery 1.11+ for the near future, which still has this problem. I'm going to just use "uglify-js" in the meantime.

@Luis-Palacios

This comment has been minimized.

Copy link

@Luis-Palacios Luis-Palacios commented Feb 18, 2015

any updates on this one? with v2.1.3 the issue still remains, btw i can also confirm that by removing the two comment blocks starting with /*! resolves the compile issue.

@sassanh

This comment has been minimized.

Copy link

@sassanh sassanh commented Feb 22, 2015

any workaround except changing jquery's code?

@apendua

This comment has been minimized.

Copy link

@apendua apendua commented Mar 21, 2015

Same story, here. BTW, I am wondering why yuglify fails to show a comprehensive error message.

@apendua

This comment has been minimized.

Copy link

@apendua apendua commented Mar 21, 2015

Lemme explain why this is happening. When you look at the jquery source code at the beginning of Sizzle section, line 548 in my case, you will see this:

var Sizzle =
/*!
 * Sizzle CSS Selector Engine v2.2.0-pre
 * http://sizzlejs.com/
 *
 * Copyright 2008, 2014 jQuery Foundation, Inc. and other contributors
 * Released under the MIT license
 * http://jquery.org/license
 *
 * Date: 2014-12-16
 */
(function( window ) {
  // ...
});

which looks totally fine at first sight. However, during the compilation process yuglify decides to replace the block comment with the following code:

var Sizzle =
;"yUglify: preserved comment block";
(function( window ) {
  // ...
});

This fails to be a valid JavaScript.

So the question here is why do we want to replace a block comment with something which has a semantic meaning. I think everyone understand the reason something has to be done, because otherwise the minify process might break things. However, we could replace that block comment with something more like:

/* */

which should not break anything and it's shorter then the current solution.

@apendua

This comment has been minimized.

Copy link

@apendua apendua commented Mar 21, 2015

Ah! I can see what you did there

https://github.com/yui/yuglify/blob/master/lib/jsminify.js#L44-L50

I understand that this is a hack there to prevent uglify-js from removing those comments. Hmm ... I think that this issue shows that the hack is not good enough.

@apendua

This comment has been minimized.

Copy link

@apendua apendua commented Mar 21, 2015

Turns out, not having an ability to preserve "license" comments is a long existing issue in uglify-js.

@apendua

This comment has been minimized.

Copy link

@apendua apendua commented Mar 21, 2015

BTW, have you considered using

https://github.com/mishoo/UglifyJS2

instead?

@apendua

This comment has been minimized.

Copy link

@apendua apendua commented Mar 21, 2015

Ha! I've seen there's already an issue for this #7.

@jjmurre

This comment has been minimized.

Copy link

@jjmurre jjmurre commented May 6, 2015

@apendua: After hours of hair-pulling I came across this discussion. Just switched from Yugilfy to UglifyJS2, problem gone. Tx.

@jonykalavera

This comment has been minimized.

Copy link

@jonykalavera jonykalavera commented Jul 11, 2015

I'm just adding jQuery to a static files work-flow for my project, so I just changed to the already minified version of jQuery which doesn't have the evil comments discussed earlier. I realize this isn't even a workaround for the issue at hand but it might be helpful for some people.

@apendua

This comment has been minimized.

Copy link

@apendua apendua commented Jul 13, 2015

@jonykalavera Nice trick 👍

@mnazim

This comment has been minimized.

Copy link

@mnazim mnazim commented Jul 23, 2015

Using jquery.min.js instead of jquery.js solved the issue for me.

@jplehmann

This comment has been minimized.

Copy link

@jplehmann jplehmann commented Jul 29, 2015

I got burned by this problem this AM -- not from jquery but from the minified versions of bootstrap and parsley javascript. Recommend generalizing the name of this issue. +1 for some kind of fix.

Seems to happen for me only from terminal input though.

@tjwudi

This comment has been minimized.

Copy link

@tjwudi tjwudi commented Feb 29, 2016

@jonykalavera it works! thanks! :) I wonder if this issue is being working on or not?

DavidCain added a commit to DavidCain/mitoc-trips that referenced this issue May 28, 2016
Use Google's CDN to serve these incredibly common libraries, falling
back to our local minified versions if needed.

Yuglify can't compress the standard jQuery library (as noted
on yui/yuglify#19). As a workaround, we can
pass the minified version to Yuglify, as the minified version lacks
comments that are triggering the error.

Also, move the last bit of JavaScript into the minified project source.
natevw added a commit to natevw/yuglify that referenced this issue Oct 17, 2017
…ad of a custom regex one. this should fix a number of open issues (probably yui#19 and yui#20; not sure about yui#21)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.