I am trying to use reactjs in my language wiki now that it can be used without classes using their brand new hook API (and here is a demo if you are curious https://fa.wikipedia.org/?withJS=MediaWiki:Experimental-react-demo.js not using ES6 so being compatible with our modules system) however resource loader sees something incompatible apparently when I issue mw.loader.using('ext.gadget.experimental-reactjs') which essentially tries to minify https://fa.wikipedia.org/wiki/%D9%85%D8%AF%DB%8C%D8%A7%D9%88%DB%8C%DA%A9%DB%8C:Gadget-experimental-react.js and gives JavaScript parse error: Parse error: Unexpected token; token 3 expected in file 'MediaWiki:Gadget-experimental-react.js' on line 27 which doesn't make sense as the source doesn't use ES6 features unconditionally. Can you see what is going on? Thanks!
Description
Details
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| resourceloader: Make JSMinPlus allow reserved words as property name (ES5) | mediawiki/core | master | +51 -3 |
Related Objects
Event Timeline
This would be T75714: Update JavaScript syntax checker for gadgets and user-scripts for ES6 and later, unfortunately. Patches are very welcome, but it's a seriously hard problem; our ES3/5 PHP minifier is astonishingly quick, and experiments making it compatible with ES6 syntax have each made it unreasonably slow.
I can't see any ES6 specific syntax here https://fa.wikipedia.org/wiki/%D9%85%D8%AF%DB%8C%D8%A7%D9%88%DB%8C%DA%A9%DB%8C:Gadget-experimental-react.js can you recheck?
The 0x notation is what it is failing on. That's a relatively new syntax feature, but indeed unlike the binary and octal number literals, the hex literals existed in ES5 as well.
We probably overlooked this when auditing for ES5 features. Will fix.
Thank you Krinkle. I've converted hex numbers to decimals but I still see some issue. Ideally I want to apply such fixes locally for a while and being able to not care about those fixes on updating.
I mean apparently line is "var hasSymbol = typeof Symbol === 'function' && Symbol.for;" which is weird. Sorry that I don't have a local installation of MediaWiki right now to check this better, I am trying to get jsminplus.php and run it independently to see whether I can improve it or fix the script issues.
Great, I was able to fix all the issues here https://fa.wikipedia.org/w/index.php?title=%D9%85%D8%AF%DB%8C%D8%A7%D9%88%DB%8C%DA%A9%DB%8C:Gadget-experimental-react.js&diff=25445722&oldid=25445607 the issue is jsmin raise error when using reserved keywords as property, like .delete .default .for and others. Please make that work also if possible. Thanks
Just to note use of reversed words is ES5 not ES6 as https://stackoverflow.com/a/17911822 In ECMAScript, starting from ES5, reserved words may be used as object property names "in the buff". This means that they don't need to be "clothed" in quotes when defining object literals, and they can be dereferenced (for accessing, assigning, and deleting) on objects without having to use square bracket indexing notation.
Thanks for all your helps. Next I should develop some gadget with reactjs now I've imported with lots of hassle :) (just to note, I keep try to avoid ES6 as I want to keep it RL compatible)
@Ebraminio Thanks, I'm familiar with that being part of ES5 and we did actually add support for that a while ago.
Examples from the unit tests:
var a = { true: 12 }; a.true = 12;
However, it would seem we missed a few cases. Will look into it! Also, if you're comfortable contributing to the PHP code base, patches welcome!
Lowering priority for the time being given other commitments and priorities. If and when resourcing is available, will probably go into T75714 instead. However, I'll keep this task open for anyone to work when they have time.
Ok. I experienced a bit with it and it looks fun so let me claim it :)
Interestingly if I give this as you said
var a = { true: 12 }; a.true = 12;Or run this on a temp folder,
wget https://github.com/wikimedia/mediawiki/raw/master/includes/libs/jsminplus.php
php -r 'require("jsminplus.php"); echo JSMinPlus::minify("var a = { true: 12 }; a.true = 12;");'It fails actually even the fact we see that on the unit test. Do you know what has went wrong here @Krinkle?
Change 491883 had a related patch set uploaded (by Ebrahim; owner: Ebrahim):
[mediawiki/core@master] Make JSMinPlus allow reserved words as property name (ES5)
@Ebraminio We have indeed, unfortunately, two parsers/minifiers currently.
We use JavaScriptMinifier for actual minification of all code served to users.
We use JSMinPlus only to validate (not minify) code submitted by users in the form of gadgets and user scripts. If code appears invalid, it is replaced with a simple "mw.log.error(errorMessage)" statement, so that it can still be safely concatenated and bundled with other code.
The reason we have both, is because JavaScriptMinifier is fast and can scale to what we need for all JavaScript code we serve (only a small portion is user-generated). JSMinPlus, on the other hand, is slow, but provides much better error messages. So we only use it for user-script validation.
Ideally, JSMinPlus would be removed in favour of JavaScriptMinifier being able to serve both needs in a way that won't compromise its speed.
Change 491883 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Make JSMinPlus allow reserved words as property name (ES5)
My new tiny tool based on react (didn't go rewrite my old ones yet), https://fa.wikipedia.org/wiki/%D9%85%D8%AF%DB%8C%D8%A7%D9%88%DB%8C%DA%A9%DB%8C:Massblock.js having jsx around could be huge boost, still, is nice to have react around.