freeciv-web icon indicating copy to clipboard operation
freeciv-web copied to clipboard

JS code not being transpiled?

Open mchenryc opened this issue 7 years ago • 8 comments

Freeciv-web needs to be transpiled to run on MSIE, but when I run a fresh build from master on MSIE-11.0.9600.19080, I see:

SCRIPT1014: Invalid character File: webclient.js, Line: 31831, Column: 28

which points to:

  model.traverse((node) => {

...an IE6 arrow-function, which Closure should transpile.

Hopefully this is just a configuration issue, or perhaps I'm building incorrectly somehow.

mchenryc avatar Aug 05 '18 04:08 mchenryc

At minimal, closure needs to be updated, as many improvements have been made, many to handle ES6 features, since the currently used v20170521 release.

mchenryc avatar Aug 05 '18 05:08 mchenryc

https://github.com/samaxes/minify-maven-plugin/pull/155

Perhaps you can try adding support for transpiling using https://babeljs.io ?

andreasrosdal avatar Aug 05 '18 07:08 andreasrosdal

That's the raw code for local testing. The "production" host would serve webclient.min.js: https://github.com/freeciv/freeciv-web/commit/a9cc731a561cd8f44c32bc319b4c2b14fa51e53b#diff-8d3042c0d730641af06832fd05e11539, that is transpiled.

I'm keeping this open with the more general issue of sending modern JS to modern browsers, as discussed in #174.

lonemadmax avatar Aug 05 '18 09:08 lonemadmax

TL;DR: I was looking at the right file - but hadn't realized the non-minified file is downloaded when hosted locally - IE was failing no matter what I did. I then began investigating closure-compiler's latest version, which does not transpile when using certain settings. Closure-compiler v20180716, using <closureLanguageIn>ECMASCRIPT_2017</...> works fine (as do our current settings w/ current version of CC).

Takeaway:

  1. We shouldn't test hostname to decide what script to load, but rather against some config setting specific to which build configuration should be used. This is related to #175 (in that it tests against hostname as well).

    <% if (request.getServerName().equals(fcwHost)) { %>

  2. I'll submit a patch for updating to closure-compiler's latest release. Sounds like there are a number of bugfixes.

The tangential rabbit hole (for posterity): upgrading closure-compiler for their bug-fixes:

For starters, our pom.xml has <closureLangauge> in the minify config, which is no longer valid - not a obvious problem, as it uses the default ECMASCRIPT6 as input language. However, minify (no longer maintained) is built with an old version of closure-compiler, and because CC has since removed the language constant ECMASCRIPT6, when upgrading CC, <closureLanguageIn> must be specified (ECMASCRIPT6 was replaced with ECMASCRIPT_2015, ECMASCRIPT_2016, ...etc).

Note that this "bad tag" in our config is the only reason we get transpilation from ES6 now. using CC-v20170521 with our current <closureLanguageIn>ECMASCRIPT5</...> give warnings about unsupported language features!

Upgrading to CC-v20180716 and setting <closureLanguageIn> to ECMASCRIPT_2015, ...2016, or ...2017 works fine. Using ECMASCRIPT_2018 though, CC will only transpile if <closureCompilationLevel> is set to WHITESPACE_ONLY, but not with the default SIMPLE_OPTIMIZATIONS (which our configuration of minify was using, as the default).

mchenryc avatar Aug 06 '18 06:08 mchenryc

Perhaps you can try adding support for transpiling using https://babeljs.io ?

I might look into this. minify is no longer maintained, and the silliness above w/ closure-compiler was just frustrating. I'm thinking that leaving javascript manipulation to the javascript ecosystem makes sense.

mchenryc avatar Aug 06 '18 06:08 mchenryc

I began a spike using babel with the maven build, which is, as might be expected, even more involved than using closure compiler plugin. More so, once the src-dir cleanup lands.

I want to do another spike on a far more involved change - but check openness to the idea first, that being: to separate the javascript code from the java server entirely, into a freeciv-client sub-project.

Freeciv-client would be a completely node based sub-project, generating static files that would be served either by nginx directly, or again through tomcat. This would allow us to use now-standard javascript tooling for this javascript application. The dev server would provide live updates, and proxy all requests to nginx or dev tomcat server. The build itself would have the latest tooling available.

My expectation is that this would take a good deal of time and a several iterations before it would be even ready for testing, but that the client code itself would not fundamentally need to change. It would, however, make client specific development far easier.

@andreasrosdal @lonemadmax, would love to get some early thoughts on this - ping other relevant contributors as needed.

mchenryc avatar Aug 24 '18 05:08 mchenryc

Closed by mistake.

mchenryc avatar Aug 24 '18 05:08 mchenryc

@mchenryc I'm very positive to the idea of separating the Javascript from Java webapp. There is definitively a trend going in that direction.

That being said, I also hope that you improve other things in the game which has a direct impact on impoving the game-play experience for players. That could be things like fixing bugs, creating new features etc.

andreasrosdal avatar Aug 24 '18 05:08 andreasrosdal