Porting code to ES6

[ninjasort]

ES6 has a number of advantages and could be utilized in both tests and source. What are your thoughts on porting some of the protoype-style classes over to actual ES6 classes using the babelify transform and running it over the lib code?

You can also use it for the admin React components as well.

[JedWatson]

@cameronjroe We're using babelify transform with browserify to build the new Admin UI code, and have adopted ES6 extensively for that code (see the elemental-integration branch)

I haven't started using it for node because it seems like more trouble than it's worth to transpile javascript for the back-end... on projects where I publish src and lib builds, things can get confusing fast for contributors and you're doubling the codebase, and it complicates the development process considerably.

Unless there's a strong argument for it and a really simple developer experience associated with building Keystone's node files with babel, I'm inclined to wait a bit longer then simply start adopting ES6 features that ship in new versions of iojs/node (which are nearly merged again)

Using it for tests is a good idea, since babel is a simple plugin for the test runner, that sounds like a good area to update next.

[ninjasort]

Hey @JedWatson

I would say that using ES6 does indeed propose a strong argument for implementing across the stack. Even with the iojs/node merge coming up, the syntax won't be changing. In that case, it's really a matter of just delaying the rewrite. However, I do think it would help tremendously for a few reasons:

• significantly reduce bugs and codebase size
• generators available for potentially migrating to koa
• ES7 features with babel-runtime for async functions
• decorators for potential jwt auth in the future
• faster ramp up time for contributors (ES6 is easier to read in my opinion)

[ninjasort]

Also, in terms of transpiling/bundling, (and maybe there's a better issue for this) I think that although webpack is confusing at first, it has a number of advantages for the build process including development/production configs that make it easier to configure bundle environments. I have also seen some people using koa-proxy for running a webpack-dev-server along-side a node server.

[JedWatson]

Just to be clear, I'm loving ES6 and a huge fan of babel. And you're right, we're limited to the speed at which V8 ships new Javascript syntax and features. However, the upside is that when we get that, it will be stable and fully supported.

significantly reduce bugs and codebase size

I don't think that running our node code from a separate build folder really does this. It doubles its size, confuses contributors and makes debugging more challenging. Several of my react packages have had this problem, especially the "confusing contributors" one.

We're looking into koa support anyway, ES6 isn't a blocker for this - see #1586

I'm not happy to extend the node runtime (e.g by requiring feature flags or loading babel) at the Keystone level. Developers are free to do this in projects using Keystone if they wish, but there's no way we should be enforcing it at a framework level.

Transpiling / bundling is taken care of already in the Admin UI by a custom implementation of browserify that works beautifully. It supports watching and debugging, and processing string input which we'll use to significantly optimise our client side payloads in the next big release, which isn't something we can achieve with Webpack.

Unfortunately, we are in a state where there are two code standards in Keystone - ES6+ (or more specifically, Babel supported) for the Admin UI, and ES5+ (or more specifically, node.js 0.10.x supported) for the Core.

While this could be a bit confusing for contributors, I think it's the best middle ground for now, and at least there's a clear distinction between the two.

[ninjasort]

Understandable. The last point I'll mention though is that with babel running on the node side, you can use isomorphic routing with react-router. This would eliminate the need for jade templates, because you could precompile React components, written in ES6, keeping all the code in JS/ES6.

Also, as far as the setup for running it, all you would need is an entry index (written in ES5) for the server to kick off babel: beyond that point, you don't need to require babel and can write in ES6

'use strict';

// Install `babel` hook for ES6
require('babel/register');

// Start the server
require('./koa.js');

Overall, I understand if it's not feasible right now, but hopefully with the iojs/node merge that will spark a more serious conversation.

[ninjasort]

Also, here's a snippet of using elegant isomorphic routing: (this does however use Flux/Alt)

root

import React from 'react';
import {RouteHandler} from 'react-router';
import css from '../path/to/css-module'; // extreme example here, but could easily just link to css file as well

export class Root extends React.Component {
  constructor(props) {
    super(props);
  }
  render() {
    var initialProps = {
      __html: this.props
    }

    return (
      <html>
        <head>
          <title>{this.props.title}</title>
          <style dangerouslySetInnerHTML={{ __html: css }} />
        </head>
        <body>
          <RouteHandler {...this.props} />
          <script
            id='initial-props'
            type='application/json'
            dangerouslySetInnerHTML={initialProps} />
          <script src='/bundle.js' />
        </body>
      </html>
    )
  }
}

routes

import React from 'react';
import {Route}, Router from 'react-router';
import Home from '../components/home';
import Admin from '../components/admin';

export default (
  <Route handler={Root}>
    <Route path='/' handler={Home} />
    <Route path='/keystone' handler={Admin} />
  </Route>
);

server

import React from 'react';
import alt from './alt';
import Iso from 'iso';
import Router from 'react-router';
import routes from './routes';

// isomorphic middleware
app.use(function(req, res) {
  alt.bootstrap(JSON.stringify(res.locals || {}));

  var iso = new Iso();

  const router = Router.create({
    routes: routes,
    location: req.url,
    onAbort: (redirect) => {
      return res.redirect('/login'); // auth from decorator within flux store
    }
  });

  router.run(function (Handler, state) {
    var html = React.renderToString(React.createElement(Handler));
    iso.add(html, alt.flush());

    res.send('<!DOCTYPE>' + html);
  });
});

[ninjasort]

So that res.send actually isn't totally true. It actually wraps the html in a div.__html__. But I could potentially create a fork for iso if that was something you'd be interested in. I think the idea is pretty slick, but it would require using flux+react-router which might be a different conversation entirely.

[JedWatson]

The last point I'll mention though is that with babel running on the node side, you can use isomorphic routing with react-router.

Actually, I'm about to include react-router for the Admin UI and rip out the jade templates entirely. Going to be fun :grinning:

Let me know if you're interested in helping with this, you've obviously got your head around it and I'd love some more help in that area.

I don't see any value in having the Admin UI isomorphically rendered - not like it will run without javascript on the client (technically we could target a downgraded experience for that, but the cost/benefit ratio is way out for me, given other areas we want to invest effort in Keystone)

This:

// Install `babel` hook for ES6
require('babel/register');`

installs babel globally on the node process by modifying require. As it notes in the Babel docs:

Not suitable for libraries

The require hook automatically hooks itself into all node requires. This will pollute the global scope and introduce conflicts. If you're writing an application, it's completely fine to use. If, however, you're writing a library then you should compile your library and depend on the babel-runtime.

Keystone should absolutely not implement this - it would affect every project it is used for.

Which is why I'm keen to stick "official / vanilla" javascript for the back-end, including the feature flags to enable generators for loa, etc. Keystone should really be as unintrusive on the project runtime as possible, so as to cater for the widest possible audience.

[ninjasort]

Good catch. Didn't see that with the babel/register hook. Isomorphic isn't necessarily needed, but maybe down the line it could be something to look into, considering it does improve performance quite a bit (initial page load not needing js)

Would love to help out with the routing. Are you planning on using v0.13.3 or v1.0.0-beta for the router?

[ericelliott]

I haven't started using it for node because it seems like more trouble than it's worth to transpile javascript for the back-end...

I'm a huge advocate of Universal JavaScript, and there are significant advantages to using ES6 everywhere. Specifically:

• Share all the exact same code for all views, all routing logic, all application state handling.
• With Redux + Redux DevTools, get some amazing time travel debugging capabilities on both client and server
• MUCH less code. I'm seeing 40% - 60% code size savings just by using Universal JavaScript for the entire app architecture.

[ericelliott]

I'll be looking at universal routing in the next couple days. Open to discussion & help @cameronjroe, if you're interested. =) https://github.com/keystonejs/keystone/issues/1941

[ninjasort]

Wow, sorry I missed this. Would love to loop back in on this. I'm definitely up for helping where I can. I've been setting up babel quite often for a ton of projects so I can definitely get that integrated if it's something @JedWatson decides is cool to move forward with. Love the direction of getting more React+Redux integrated.

[ericelliott]

@cameronjroe As a first step, I've got some ES6 + universal render + universal routing going in a universal keystone boilerplate project I'm actively working on. I'd love contributions. =)

[ninjasort]

Porting the code to ES6 is probably okay at this point. Babel is in a good place with the presets and it would reduce code bloat to have ES6 classes.

As for React/Redux, I think it would be a fantastic idea adopt it for the admin panel. Let me know where I can help, I've been playing around with React/Redux a lot recently.

[JedWatson]

@cameronroe we're going to launch the next version (0.4) with no babel build process for the server-side code (for simplicity), then drop support for node < 4 in one of the next significant versions so we can use ES6 natively. IMO the new versions of node with ES6+ support have been out long enough for that to be reasonable.

On a related note, 0.4 has ended up basically a complete rewrite, which has given us a much more robust architecture as well as the new React Admin UI, while at the same time we've been held back from making some big calls because I want to preserve as much backwards compatibility at the same time (not wanting to introduce unnecessary breaking changes AND rewrite in parallel, or we'll make upgrading too risky).

Once we're through this, the next few versions should happen much more quickly.

[ninjasort]

@JedWatson Awesome, thanks for the update. That all sounds great. I'm starting to review keystone again and really want to jump in when I have some time. I'll look out for that migration and see what help is needed!

[mxstbr]

I'll close this issue since Jed outlined everything perfectly above :+1: