Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JS code not being transpiled? #176

Open
mchenryc opened this issue Aug 5, 2018 · 8 comments
Open

JS code not being transpiled? #176

mchenryc opened this issue Aug 5, 2018 · 8 comments

Comments

@mchenryc
Copy link
Contributor

mchenryc commented Aug 5, 2018

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
Copy link
Contributor Author

mchenryc commented Aug 5, 2018

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

@andreasrosdal
Copy link
Contributor

samaxes/minify-maven-plugin#155

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

@lonemadmax
Copy link
Contributor

That's the raw code for local testing. The "production" host would serve webclient.min.js: a9cc731#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.

@mchenryc
Copy link
Contributor Author

mchenryc commented Aug 6, 2018

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 Trackjs token should be removed from source #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
Copy link
Contributor Author

mchenryc commented Aug 6, 2018

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
Copy link
Contributor Author

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
Copy link
Contributor Author

Closed by mistake.

@mchenryc mchenryc reopened this Aug 24, 2018
@andreasrosdal
Copy link
Contributor

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants