pilif / sacy

Smarty Asset Compiler (warning: all branches but master are subject to force pushes)
http://pilif.github.com/sacy
MIT License
34 stars 12 forks source link

jsmin.php and jquery-ui > 1.9 #17

Open cpoppema opened 11 years ago

cpoppema commented 11 years ago

In the jquery ui library there exists a function called uniqueId - which I personally do not use, however it will cause a syntax error (in Chrome and Firefox, but Firefox breaks all javascript on the page) near the +++n

uniqueId:function(){return this.each(function(){this.id||(this.id="ui-id-"+ ++n)})}

This will be minified by jsmin.php to:

uniqueId:function(){return this.each(function(){this.id||(this.id="ui-id-"+++n)})}

My current workaround is to simply remove + ++n by rewriting this function like so:

uniqueId:function(){return this.each(function(){if(!this.id){n+=1;this.id="ui-id-"+n;}})}

Testcase:

$js = "var n=100;for(var i=0;i<10;i++){console.log('nr. '+ ++n);}";
print JSMin::minify($js);

will output:

var n=100;for(var i=0;i<10;i++){console.log('nr. '+++n);}

Since JSMin is no longer maintained, I created this issue here. Maybe you're able to solve this issue, either by patching jsmin.php or maybe replacing it with a different minifier?

Love sacy otherwise, eventhough I'm only using the CSS and JS minifying part :)

pilif commented 11 years ago

I don't quite like shipping patched versions of code, though it might be needed here.

In the mean time, as a workaround, you could use uglyify.js by defining SACY_COMPRESSOR_UGLIFY and setting it to the path to the uglifyjs binary.

In my setups, I'm using all external tools because the PHP ports are outdated and kinda buggy at times.

cpoppema commented 11 years ago

I guess there is no port or an alternative for uglifyjs? It wouldn't be a problem to work with it on my workstation, but unfortunately my VPS server has no option to install nodejs, it only allows ASP(.NET), Perl, CGI and PHP. Seems like I'll stick with patching jquery ui for now.

cpoppema commented 11 years ago

Well, after I've finally setup UglifyJS on my local development, I found out that UglifyJS seems to have the same issue as JSMin regarding this particular statement! So it seems to be a non-specific issue for JSMin. (JSMin also mentions this particular issue on their website):


http://www.crockford.com/javascript/jsmin.html Use parens with confusing sequences of + or -. For example, minification changes

a + ++b

into

a+++b

which is interpreted as

a++ + b

which is wrong. You can avoid this by using parens:

a + (++b)

When using external libraries, we obviously prefer to leave them untouched. A solution might work where you can define a function or a regex that acts as a filter: which would be able to filter out .min.js and .min.css. This filter would influence whether or not a file (defined by a filename parameter) would be minified or not. If this filter would trigger false, SACY would simply concat the file ot the rest of the output instead of the minified version (by either JSMin or UglifyJS) for this file. This should work for every library that provides a minified version by themselves already and should prevent any errors JSMin or UglifyJS might inject in the final output because the source is untouched. Probably the only extra check would be to check if there exists an semicolon between the two parts you are about to concat.

Does that sound like something you want in SACY? Of course you can always choose to not put these files in the {asset_compile} tags, but then you would only partially reduce the number of HTTP-requests.

ball6847 commented 10 years ago

It would be great if you update jsmin-php to this version https://github.com/mrclay/minify/blob/master/min/lib/JSMin.php, sorry for referencing other repo ... but this is the most uptodate version of jsmin-php i can found on github.

I tried it on my local copy and it worked without any errors.

Thanks for a great plugin.