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 · 23 comments
Open

Yuglify fails to parse jquery.js #19

jdufresne opened this issue Apr 12, 2014 · 23 comments

Comments

@jdufresne
Copy link

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
Copy link

Seeing same issue with jQuery 2.1.0.

@dorey
Copy link

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
Copy link

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
Copy link

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
Copy link

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
Copy link

@ewjoachim works ok with jquery version 2.0.3.

@jensenbox
Copy link

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

@mrcoles
Copy link

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
Copy link

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
Copy link

sassanh commented Feb 22, 2015

any workaround except changing jquery's code?

@apendua
Copy link

apendua commented Mar 21, 2015

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

@apendua
Copy link

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
Copy link

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
Copy link

apendua commented Mar 21, 2015

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

@apendua
Copy link

apendua commented Mar 21, 2015

BTW, have you considered using

https://github.com/mishoo/UglifyJS2

instead?

@apendua
Copy link

apendua commented Mar 21, 2015

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

@jjmurre
Copy link

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
Copy link

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
Copy link

apendua commented Jul 13, 2015

@jonykalavera Nice trick 👍

@mnazim
Copy link

mnazim commented Jul 23, 2015

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

@jplehmann
Copy link

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.

@meetwudi
Copy link

@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)
@BarnabasSzabolcs
Copy link

BarnabasSzabolcs commented Apr 23, 2020

I thought it was still an issue with jQuery 3.4.1 and 3.5.0. minified and non-minified both.

I was using django-compressor, and it turns out, this is the right config:

COMPRESS_JS_FILTERS = [
  #'compressor.filters.jsmin.JSMinFilter', # < have to skip this (noob I know :o)
  'compressor.filters.yuglify.YUglifyJSFilter',
]

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