Compile to ECMASCRIPT5 when using closure compiler

[muzuiget]

fix this https://bugs.dojotoolkit.org/ticket/16601, a bug 3 years ago.

the ticket above mark as a duplicate of https://bugs.dojotoolkit.org/ticket/16196, but this not right, if we write the optimizeOptions as

optimizeOptions: {
    languangeIn: 'ECMASCRIPT5',
},

then closure will throw an error

Done (compile time:0.098s). OPTIMIZER FAILED: InternalError: Java class "com.google.javascript.jscomp.CompilerOptions" has no public instance field or method named "languangeIn".

because this how dojo-util create the closure option object with this

    var options = new jscomp.CompilerOptions();
    for(var k in optimizeOptions){
        options[k] = optimizeOptions[k];
    }

the closure docs http://javadoc.closure-compiler.googlecode.com/git/com/google/javascript/jscomp/CompilerOptions.html#setLanguageIn%28com.google.javascript.jscomp.CompilerOptions.LanguageMode%29 says it should be call setLanguageIn(), and pass a typed-object to it. There noway to do this in optimizeOptions.

We don't need to find a new way to pass the option, because the dojo have code not compatible with ES3 now (at lease at v1.10), it can not complie to ES3

dojo.js.uncompressed.js:2033: ERROR - Parse error. IE8 (and below) will parse trailing commas in array and object literals incorrectly. If you are targeting newer versions of JS, set the appropriate language_in option.
    'dojo/aspect',
    ^    

dojo.js.uncompressed.js:85075: ERROR - Parse error. missing name after . operator
                return this.transformer.in(value);
                                        ^

dojo.js.uncompressed.js:94967: ERROR - Parse error. invalid property id
                var widget=new BorderContainer({gutters:false, class:"singleDecorator"});
                                                               ^

3 error(s), 0 warning(s)
Done (compile time:0.938s)

So, ES5 is only option.

[dylans]

I think two things should be fixed here:

1. The things that are invalid ES3 should be fixed for Dojo 1.x (as the errors listed above are not ES5 specific features, but usage of reserved words and a trailing comma)
2. ES5 should be a compiler/config option

[dylans]

@muzuiget do you have a CLA on file? Please see the guidelines at https://github.com/dojo/util/blob/master/CONTRIBUTING.md#contributor-license-agreement if you do not.

[gitgrimbo]

@muzuiget - Are you saying that Dojo itself has code that the closure compiler fails to compile?

If so, in the following lines of your uncompressed dojo layer, what are the module(s) to which these lines are associated? Can we determine that they are Dojo modules or your own application's modules?

dojo.js.uncompressed.js:2033 dojo.js.uncompressed.js:85075 dojo.js.uncompressed.js:94967

I ask because I haven't experienced Dojo modules being responsible for syntax errors during my own builds.

[dylans]

Yeah, from what I can tell, the first one is somewhere with a list of AMD dependencies, the second one is using the reserved word in, and the third one should put the word class in quotes in a constructor. From what I can tell from the limited info provided, the first and third are easily fixable (it's possible the first one is a regression in Dojo), and I have no idea where the second one is coming from.

[gitgrimbo]

An issue with the bug is that languangeIn is a typo, and leads to the error message provided:

Done (compile time:0.098s). OPTIMIZER FAILED: InternalError: Java class "com.google.javascript.jscomp.CompilerOptions" has no public instance field or method named "languangeIn".

When this is fixed (to languageIn), the actual error will be:

exception: (compile time:0.093s). OPTIMIZER FAILED: InternalError: Cannot convert ECMASCRIPT5 to com.google.javascript.jscomp.CompilerOptions$LanguageMode

So the main issue is probably that the Java type for languageIn (CompilerOptions$LanguageMode) is not a straight forward translation from any JS type. There are probably lots of optimizeOptions for which the type translation is simple (boolean, number, string, etc), and we probably don't see any issues for these.

[muzuiget]

Oh, the typo is mistake, I actually correct it and tried again, but I copy and paste the wrong output code when create an issue.

And the ES3-compatible code is not Dojo, it a thirdparty library https://github.com/stemey/dojo-generate-form, sorry.

How about make whitelist to hardcode translate the option value type? I have update my code.

PS: @dylans Do I need to fill the form http://dojofoundation.org/about/claForm here, even required street address, It just one line code guy, I don't care my PR merge or not, just hope fix this issue in official code base.

[dylans]

@muzuiget you're right in that we don't need a CLA for just a one line patch, but given my notes above to make it a flag rather than a hardcoded change, I think the eventual patch will need to be a bit more involved, hence my suggestion of getting a CLA in place now. The purpose of asking for a street address is to prove that you're a real human, we don't use the information we collect via the CLA process for any other purpose.

[gitgrimbo]

Changes made in this area will also have to be applied twice.

Once to optimizeRunner.js (used in a 'Node build' when spawning a JVM to run closure): https://github.com/dojo/util/blob/1.10.4/build/optimizeRunner.js#L86

And once to closure.js (used in a 'Java build'): https://github.com/dojo/util/blob/1.10.4/build/transforms/optimizer/closure.js#L31

These two pieces of code (that are responsible for orchestrating the closure compiler) are almost identical, and could (should?) probably be refactored into a single module to be shared.

[dylans]

Closed via 72cb7fd and 9d92e70. This could definitely use some additional testing (it passes for me).