yui / yuicompressor

YUI Compressor
http://yui.github.com/yuicompressor/
Other
3.01k stars 664 forks source link

conditional compilation problem #119

Open cherouvim opened 10 years ago

cherouvim commented 10 years ago

The compressor breaks on: if (/_@ccon!@/true) alert(2);

with: missing ) after condition

This kind of code exists in selectivizr.com

ibobo commented 10 years ago

I have this problem too

wolframarnold commented 10 years ago

Same here. Any update on this?

tml commented 10 years ago

I cannot reproduce this with version 2.4.8; which version, OS, and JRE are you guys using? Can one of you create a reproducible .css file and attach it here?

cherouvim commented 10 years ago

It breaks on this source http://pastebin.com/raw.php?i=ZGQ3cXEM if you save it as foo.js I use:

and call YUI via ant.

tml commented 10 years ago

Thanks, this reproduces the problem and I'm looking into a fix.

tml commented 10 years ago

I assume the expectation is that YUICompressor not strip these?

cherouvim commented 10 years ago

Yes. It shouldn't strip these. The sample code just does an alert, but the code in the selectivizr library probably does something useful. I guess it's for detecting the browser. Super ugly though.

tml commented 10 years ago

Well, I ask because that would (as far as I can tell,) make us the only JS minifier preserving these comments...not that this is a bad thing, just making sure I understand what our userbase wants.

tml commented 10 years ago

The next question is - is there a strong reason to support these conditional comments at arbitrary points in the code? Doing so will require the patch to touch all the possible parser states, whereas if we can sanely limit these to isolated locations, it becomes far less intrusive.

cherouvim commented 10 years ago

I don't know. I think most people (including myself and all my programmer friends) wouldn't write this kind of code, but selectivizr, for some reasons, seems to do so. Maybe these are needed for some obscure browser detection reasons. I'm not sure whether selectivizr will work if these are removed. The problem that made me start this thread is that the build process of my project broke because it includes yui compression in all css and js when we added selectivizr.

I'm not sure whether I am the right person to give you valuable insight on how to solve this (technically and politically). Maybe you could start a discussion with the selectivizr guys.

ibobo commented 10 years ago

Another project using this is Projekktor HTML5 video player: https://github.com/frankyghost/projekktor/blob/master/src/controller/projekktor.js Don't know if this is used in other points by people. Yes, it needs to be preserved, not stripped and I definitely would support it at least in if conditions, general expressions would be a plus.

sippsolutions commented 9 years ago

I can reproduce this. Is there any fix available?

    [apply]   32:25:missing ) after condition
    [apply] [ERROR] in selectivizr.js
    [apply]   1:0:Compilation produced 1 syntax errors.
    [apply] org.mozilla.javascript.EvaluatorException: Compilation produced 1 syntax errors.
    [apply]     at com.yahoo.platform.yui.compressor.YUICompressor$1.runtimeError(YUICompressor.java:172)
    [apply]     at org.mozilla.javascript.Parser.parse(Parser.java:396)
    [apply]     at org.mozilla.javascript.Parser.parse(Parser.java:340)
    [apply]     at com.yahoo.platform.yui.compressor.JavaScriptCompressor.parse(JavaScriptCompressor.java:315)
    [apply]     at com.yahoo.platform.yui.compressor.JavaScriptCompressor.<init>(JavaScriptCompressor.java:536)
    [apply]     at com.yahoo.platform.yui.compressor.YUICompressor.main(YUICompressor.java:147)
    [apply]     at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    [apply]     at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    [apply]     at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    [apply]     at java.lang.reflect.Method.invoke(Method.java:606)
    [apply]     at com.yahoo.platform.yui.compressor.Bootstrap.main(Bootstrap.java:21)

BUILD FAILED
tml commented 9 years ago

@sippsolutions selectivizr already provides you with a minified version in their zip file

sippsolutions commented 9 years ago

@tml Our build and deployment process automatically processes all js-files in the src folder, so this doesn't fix the problem.

tml commented 9 years ago

@sippsolutions Seems an easy enough work-around; set your build process/tool to skip files named "*.min", and put the minified version in your src folder instead of the un-minified.

I do not currently have a fix in the pipeline for the underlying issue, so I encourage you to find work-arounds in your environment until one is available.

ibobo commented 9 years ago

As a workaround I turned all conditional comments used in if conditions to a string before minification and then turning them back to normal after. Use this regex replacement: before: /\(([^\)]+\/\*@cc_on[^@]+@\*\/[^\)]+)\)/ -> (' + escape match + ') after: /\([\'"]([^\)\']+\/\*@cc_on[^@]+@\*\/[^\)]+)[\'"]\)/ -> ( + unescape match + )

This is the PHP code I use:

$code = preg_replace_callback('/\(([^\)]+\/\*@cc_on[^@]+@\*\/[^\)]+)\)/', function($match){
    return '(\''.preg_replace('/\s+/', '', addcslashes($match[1], '\'')).'\')';
}, $code);

... minifcation ...

$code = preg_replace_callback('/\([\'"]([^\)\']+\/\*@cc_on[^@]+@\*\/[^\)]+)[\'"]\)/', function($match){
    return '('.str_replace('\\\'', '\'', $match[1]).')';
}, $code);
sippsolutions commented 9 years ago

@tml I already thought about this. This IS an workaround but the issue should still get fixed in the near future. We have an automatic build system for over 100 packages, one subtask is minifying the js-files - so we need to be sure everything is fine, no matter which js-files a person adds to the package src directory.

marczych commented 9 years ago

I just ran into this as well and boiled it down to this example:

var x = /*@cc_on/*@if(@_jscript_version<=5.7)true@else@*/false/*@end@*/;
[mzych@home] - [~] - [Thu Oct 08, 04:58:31]
[N]> java -jar yuicompressor-2.4.8.jar --type js --charset utf-8  <(echo "var x = /*@cc_on/*@if(@_jscript_version<=5.7)true@else@*/false/*@end@*/;")
[ERROR] in /proc/self/fd/11
  1:63:missing ; before statement
[ERROR] in /proc/self/fd/11
  1:0:Compilation produced 1 syntax errors.
org.mozilla.javascript.EvaluatorException: Compilation produced 1 syntax errors.
    at com.yahoo.platform.yui.compressor.YUICompressor$1.runtimeError(YUICompressor.java:172)
    at org.mozilla.javascript.Parser.parse(Parser.java:396)
    at org.mozilla.javascript.Parser.parse(Parser.java:340)
    at com.yahoo.platform.yui.compressor.JavaScriptCompressor.parse(JavaScriptCompressor.java:315)
    at com.yahoo.platform.yui.compressor.JavaScriptCompressor.<init>(JavaScriptCompressor.java:536)
    at com.yahoo.platform.yui.compressor.YUICompressor.main(YUICompressor.java:147)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:606)
    at com.yahoo.platform.yui.compressor.Bootstrap.main(Bootstrap.java:21)
[mzych@home] - [~] - [Thu Oct 08, 04:58:45] - [2]
[I]> java -jar yuicompressor-2.4.8pre.jar --type js --charset utf-8  <(echo "var x = /*@cc_on/*@if(@_jscript_version<=5.7)true@else@*/false/*@end@*/;")
var x=
/*@cc_on/*@if(@_jscript_version<=5.7)true@else@*/
false
/*@end@*/;%                                                                                                                                                                                                                                                                                   [mzych@cominor] - [~] - [Thu Oct 08, 04:59:02]
[I]> java -jar yuicompressor-2.4.8pre.jar --version

Usage: java -jar yuicompressor-2.4.6.jar [options] [input file]

Global Options
  -h, --help                Displays this information
  --type <js|css>           Specifies the type of the input file
  --charset <charset>       Read the input file using <charset>
  --line-break <column>     Insert a line break after the specified column number
  -v, --verbose             Display informational messages and warnings
  -o <file>                 Place the output into <file>. Defaults to stdout.
                            Multiple files can be processed using the following syntax:
                            java -jar yuicompressor.jar -o '.css$:-min.css' *.css
                            java -jar yuicompressor.jar -o '.js$:-min.js' *.js

JavaScript Options
  --nomunge                 Minify only, do not obfuscate
  --preserve-semi           Preserve all semicolons
  --disable-optimizations   Disable all micro optimizations

If no input file is specified, it defaults to stdin. In this case, the 'type'
option is required. Otherwise, the 'type' option is required only if the input
file extension is neither 'js' nor 'css'.

Note that a similar code snippet is found in FileDrop.

Is there fix or workaround in the works?