api-console.js and api-console-vendor.js cannot be minified using YUICompressor due to usage of reserved keywords

[ravikiranj]

The generated api-console.js and api-console-vendor.js use 'enum', 'for', 'boolean', 'default'. See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Lexical_grammar for list of reserved keywords.

$ java -jar yuicompressor-2.4.8.jar -v --type js api-console.js -o minified.js
 1:0:Compilation produced 123 syntax errors.
org.mozilla.javascript.EvaluatorException: Compilation produced 123 syntax errors.

$ java -jar yuicompressor-2.4.8.jar -v --type js api-console-vendor.js -o minified.js 
[ERROR] in api-console-vendor.js
  1:0:Compilation produced 408 syntax errors.
org.mozilla.javascript.EvaluatorException: Compilation produced 408 syntax errors.

The correct way to refer the reserved keywords would be to wrap them in quotes such as obj["enum"] instead of obj.enum

[blakeembrey]

@ravikiranj It could be an issue with your compiler. Using reserved words as properties is completely valid ES5 code. That said, it probably couldn't hurt to support this. Is there a mode on yuicompressor to support ES5 to ES3 output?

Edit: Looks like your using the only engine that doesn't support it according to https://kangax.github.io/compat-table/es5/#Reserved_words_as_property_names

[ravikiranj]

Also, there are reserved keywords (double, byte, char) being assigned values. My understanding was using reserved keywords as identifiers was a no go, please correct me if I'm wrong.

[blakeembrey]

As variables, yes, as properties, no.

[blakeembrey]

You can verify by typing JavaScript and running ({}).enum vs var enum = 'test'.

[ravikiranj]

At this point, my only viable alternative is to manually fix the properties and rename the variable names. You can close out this issue in that case. Having said that, it would be nice if RAML-only JS was Rhino compatible.

[blakeembrey]

The variable names won't be broken otherwise this would have already been an issue, as noted above. Is there no option for yiucompressor to support ES5 or is it literally using Rhino to parse?

We can accept a PR if it works for you. I'd start by looking at either disabling ES5 in our linting or using something like https://github.com/spicyj/es3ify so this doesn't happen again. However, you might end up running into more issues with any dependencies that aren't written using ES3 too (not sure how likely, ES5 has been around for a while). The alternative is to use a compressor that works for ES5, which should be pretty much all of them. If you're using Java, you could try using https://github.com/google/closure-compiler which appears to work fine with ES5 (using an option).

My knowledge of Rhino about ends there, so you might have to investigate what the issue with the compiler is if you want to use yuicompressor (relevant issues yui/yuicompressor#183, yui/yuicompressor#47, many others, and yui/yuicompressor#41 (comment) which was a couple of years ago stating it'd be fixed by updating Rhino).

Edit: First paragraph is irrelevant now I went digging through yuicompressor issues.

[ravikiranj]

I manually fixed all the references but then I noticed that the angularJS code in api-console.js totally breaks when you minify and obfuscate it via uglifyJS or yuicompressor. Has this code been tested with minification + obfuscation ? I found https://docs.angularjs.org/tutorial/step_05#a-note-on-minification that explains how to resolve it but the change required is way too much for my liking.

[ravikiranj]

@blakeembrey I managed to patch api-console.js and made it compatible with uglifyJS and YUICompressor for minification with/without munge/obfuscation. The patch looks like http://pastebin.com/qG21sLGw . Is this something you'd be interested in accepting a pull request ?

[blakeembrey]

@ravikiranj Yes, variable mangling causes issues with right now which is why we'd need to use the array syntax. We should really be doing that already, so if you want to PR that it'd be pretty amazing. You'd have to modify the original source code though, not the combined file. As for the ES5 properties, it'd be good to have another PR for that (just make sure you enable es5: false in JSHint so we don't break this in the future).

Edit I think raml-validate might need some changes to support this too, which is a bit of a pain.

[ravikiranj]

I submitted a PR at #170

[jcenturion]

@ravikiranj, I've already merged it.