Skip to content

Switch to ES 6#617

Merged
k4b7 merged 3 commits intomasterfrom
es6
Jan 14, 2017
Merged

Switch to ES 6#617
k4b7 merged 3 commits intomasterfrom
es6

Conversation

@gagern
Copy link
Collaborator

@gagern gagern commented Jan 10, 2017

With 0.7.0 released, it might be a good time to do the planned switch to ES6 now. And this PR here is the first step in that direction.

  1. It adds babelify to the build process, translating ES6 to ES5. The eslint configuration is adapted to cover es6, too.
  2. It changes occurrences of var to const or let as appropriate. eslint only ever allows one of these two alternatives. The main purpose of this step is demonstrating that the transformations actually get applied as intended.
  3. It changes the live server to serve either babelified or unbabelified files. The former is required for browsers with lacking ES6 support, which at the moment includes Firefox. The latter is faster so in combination with a compatible browser it avoids slowing down development.

@gagern
Copy link
Collaborator Author

gagern commented Jan 12, 2017

Rebased onto the old and new trailing comma convention. Having this in would help testing on IE9 thanks to the browserify step.

This allows using ES6 syntax in our code, while maintaining backwards
compatibility for the generated file.
As babelify is slow, it may be desriable to not run it during development.
This is OK if the browser is recent enough to understand ES6 natively.
(This does not include current Firefox due to it having problems with
for(const … in …), https://bugzilla.mozilla.org/show_bug.cgi?id=1094995.)
For older browsers, or to check issues possibly introduced by babelify,
adding /babel as the first component of the path will switch to a version
which has been processed by babelify.  This is also used for screenshots.

function twoBrowserified(url, file, standaloneName) {
app.get(url, serveBrowserified(file, standaloneName, false));
app.get("/babel" + url, serveBrowserified(file, standaloneName, true));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this out and there's a noticeable lag... thanks for setting this up a separate route.

if (!katexURL) {
katexIP = req.query.ip;
katexURL = "http://" + katexIP + ":" + katexPort + "/";
katexURL = "http://" + katexIP + ":" + katexPort + "/babel/";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

process.exit(1);
} else {
console.log("OK");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the fix for the node version check?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this version should work on all reasonable versions of GNU make, contrary to a version I had before some fixup.

@k4b7
Copy link
Member

k4b7 commented Jan 14, 2017

This is the future step in modernizing the codebase. I'm excited. Thanks @gagern.

@gagern gagern deleted the es6 branch January 15, 2017 15:57
@gagern
Copy link
Collaborator Author

gagern commented Jan 17, 2017

I realized that this does slow down the screenshotter immensely. I guess we should try to come up with a solution to do screenshot tests with a cached version. Or perhaps even with the build version, as present on the file system. Something like that.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants