Switch from CoffeeScript to modular ES2015

[valscion]

Hi there!

First of all, huge thanks for this gem, it has proven to be very useful to us!

I would really love this gem to work also for more complex and modern JavaScript use cases than simply using Sprockets to concatenate all JS source files together to a single blob.

Let me tell you about

1. How we use this gem
2. What issues the current implementation causes us
3. What is my proposal on how to do things differently

Rest assured that I'm not asking you to do these changes that I am proposing. I'm mainly interested in hearing your thoughts on whether you'd be willing to accept a big overhaul for the JS code of this gem as a future pull request, if our team could make this a reality.

I'd also be very interested to hear whether anyone else using this gem finds this issue interesting and the proposed solution something worth doing.

How we use this gem (our use case)

First we generate the routes programmatically for each locale we support to app/assets/javascripts/routes/ directory, using routes-<LOCALE>.js as the filename. (e.g. routes-fi.js for Finnish routes). A snippet from our rake task looks like this:

ROUTES_DIR = File.join('app', 'assets', 'javascripts', 'routes')
FileUtils.mkdir_p(Rails.root.join(ROUTES_DIR))

Site.each do |site|
  I18n.with_locale(site.locale) do
    file_name = File.join(ROUTES_DIR, "routes-#{I18n.locale}.js")
    JsRoutes.generate!(file_name)
  end
end

This is because we translate our routes to get prettier URLs in each localized version of our site. This means that e.g. our feedback path in Finnish site could be https://venuu.fi/palaute and on English site it could be https://venuu.com/feedback.

The toolchain we use to get the routes working on JS side as well:

• We use Babel to transpile this modern JavaScript to a version that can be run on IE>=9 browsers.
• We bundle our JavaScript modules together by leveraging webpack as our module bundler.
• We build separate JS bundles for every locale we support
• We have mapped routes name to locale specific file with a webpack config:

  config.alias.routes = `routes/routes-${locale}`

Then we have multiple modern JS files importing various routes from that file:

import { feedbackPath } from 'routes';

console.log(`The feedback path is: ${feedbackPath()}`);

This code ends up as as part of our bundle/main-fi.js file, with the final console.log possibly evaluating to

var _routes = __webpack_require__(123 /* routes module ID */);

console.log("The feedback path is: " + _routes.feedbackPath());
// => The feedback path is: /palaute

Issues the current implementation causes us

We've got a few issues the CoffeeScript approach used by this gem causes us:

The built JS cannot be imported directly

The CoffeeScript compiler generates output to the end of routes.js that only works in global scope, but doesn't work in a function scope:

// ...all the other code...

}).call(this);

We have had to add this to our rake task generating the routes to make the routes.js file importable via webpack:

# Remove the part from the end of generated file which causes the file to
# be unimportable via webpack.
File.open(Rails.root.join(file_name), 'r+') do |routes_file|
  src = routes_file.read
  src.gsub!(/(\}\)\.call\(this\);)\Z/, '})();')
  routes_file.rewind
  routes_file.truncate(0)
  routes_file.write(src)
end

The routes always leaks to globals

I might be wrong, but at least what we've struggled with was that there is no easy way to avoid to avoid importing this code changing global JS variables on the page.

Even if we use a JS bundler so that we could split our JS code to smaller modules, when we import our generated route file there will sadly be side-effects that we cannot get rid of.

The generated code can't benefit from tree shaking

The JS tools are heading to a future where we can only ship the smallest amount of JavaScript required for a page to function. One amazing tool for this is tree shaking, which effectively removes the unused exports at build time.

The generated bundle cannot be statically analyzed

The JS community has come up with lots of tools that allow JS code to be statically analyzed, enabling discovery of typos and bugs early in writing code.

One tool we use is ESLint and eslint-plugin-import to validate that the imports we're using are actually there and not undefined.

The way the bundle is generated right now breaks tools like the eslint-plugin-import and we cannot find out whether the route imports we've written actually even exist.

The whitelist approach for generating routes is not enough

We are generating multiple different bundles of our JavaScript, and our goal is to only ship the JS necessary for a single page to function. The different JS files might need different parts of the routes, and it would be error-prone for us to whitelist for all the possible routes we use.

We would very much like our tools to strip out the unused routes automatically, based on their usages. I know that this is possible to do with the correct tools.

The generated routes file is big

In our case, we're currently generating over 140 route declarations and I can only see this number going up. We've analyzed the sizes of our JS bundles and found out that our routes are over 10% of our main bundle sizes, which is unnecessarily big.

We really want to send less JavaScript to our clients, so that they could load the site faster and we could provide better service to them.

The current CoffeeScript approach is hard to contribute to

I'd argue that the current approach of having only one CoffeeScript file for the JS part makes it unnecessarily difficult to contribute changes back to this repository. Given the increasing prevalence of ES2015+ code being used in favour of CoffeeScript, I fear that there are less and less people interested in writing CoffeeScript and to fix bugs or propose new useful features for this gem.

There are a lot of posts on the Internets where people are encouraging switching away from CoffeeScript to ES6/ES2015. One good question was over at Meteor forums: Use ES2015 or CoffeeScript?

My proposal

Refactor the JS code to be:

• converted from CoffeeScript to ES2015
• modular
• export each of the generated routes explicitly

The build pipeline would generate a similar routes.js file that would work identically to the current implementation. However, it could also generate files that could be consumed by tools like Webpack 2 that could leverage tree shaking to strip out the routes that are never used in the codebase.

We could also allow cherry-picking the routes from separate files like Lodash does:

import feedbackPath from 'routes/feedbackPath';

This way even if the module bundler used wouldn't support tree shaking, we could still get smaller bundles because only the used routes would get inside the bundle. Also, if I'd accidentally import a route with an incorrect name, it wouldn't even pass our build, instead of it blowing up in runtime or even in production.

I don't yet know what the final output would exactly look like, but I would tackle this by creating a new build chain for this gem:

• Split the current routes.js.coffee to multiple source files that are written with modern JavaScript
• Use Babel to transpile those source files to JS that older browsers can run, too
• Bundle the results of those transpiled files to consumable files, similar to the current routes.js being built from the CoffeeScript sources
• Expose the generated bundle as consumable by CommonJS, AMD and browser global environments by leveraging webpack's libraryTarget option, exposing the generated code with Universal Module Definion (UMD) code.

This refactoring would not change the current functionality. It would still be possible to require the generated routes bundle file via Sprockets as before. This would just supercharge the module bundler usage to a completely new level, allowing us to leverage the best parts of modern JavaScript tooling.

TL;DR:

Switch from CoffeeScript to modular modern JavaScript (ES2015) to

• make this more friendly for modern JS tooling
• avoid unnecessarily large route files
• avoid issues with modern JS bundlers
• enable static analysis of route imports
• make this awesome gem easier to contribute back to

---

What do you think? Am I just insane to propose a change like this? Do you see this proposal as a valid approach to make sure this tool works better even when using a module bundler like webpack or browserify? What knee-jerk reactions does this bring to you? I'm open for any comments regarding this approach.

[le0pard]

Hello. Thanks for proposal. I need to clarify such info:

The built JS cannot be imported directly

But if look deeper, you can see, what router is friendly for amd and module systems:

https://github.com/railsware/js-routes/blob/v1.3.2/lib/routes.js#L409-L436

because root defined here: https://github.com/railsware/js-routes/blob/v1.3.2/lib/routes.js#L11

So to use it just need:

const Routes = require('./routes').Routes;

The routes always leaks to globals avoid issues with modern JS bundlers

I am not sure how this can happen, if we support amd and commonjs systems.

avoid unnecessarily large route files

Need to get more info how exactly ES6 (babel) will make this happen (not uglifyjs or some another stuff). As I know only rollup can do this, even webpack (only 2 version can do this, but still not stable) blindly get all file to build, which of course build systems, not ES6 modules.

[le0pard]

I think, that we can improve export system for current routes like this:

module.exports = {
  default: Routes
}

In this case you can use it in this way:

import Routes from './routes'

Or import each route separate, so in this case you can use destruction. But all this changes can be done without converting to ES6.

[valscion]

Unfortunately Webpack 2 cannot use tree shaking unless the exports are using ES6 modules and not CommonJS:

export const feedbackPath = /* .... */;
export const myOtherPath = /* .... */;

// Or even:

export function feedbackPath(/* params */) {
  /* implementation */
}

The reason is that CommonJS modules are not statically analyzable, whereas ES6 modules are.

avoid issues with modern JS bundlers

I am not sure how this can happen, if we support amd and commonjs systems.

Unfortunately the CoffeeScript compiler adds the }).call(this); to the end of the file. That .call(this); breaks webpack as the this referenced in here is undefined.

I can look more into what exactly breaks and how, but that is only a subset of the issues we've faced.

Or import each route separate, so in this case you can use destruction. But all this changes can be done without converting to ES6.

We can already use destructuring, our imports actually look like

import { feedbackPath, usersPath } from 'routes';

But that is only syntactic sugar. All the unused route declarations are still there.

The built JS cannot be imported directly

But if look deeper, you can see, what router is friendly for amd and module systems:

https://github.com/railsware/js-routes/blob/v1.3.2/lib/routes.js#L409-L436

because root defined here: https://github.com/railsware/js-routes/blob/v1.3.2/lib/routes.js#L11

Yes, but this is the part that is confusing to me: https://github.com/railsware/js-routes/blob/v1.3.2/lib/routes.js#L422

root.NAMESPACE = ROUTES;

I'd only need that to be root = ROUTES; without the NAMESPACE in there at all.

avoid unnecessarily large route files

Need to get more info how exactly ES6 (babel) will make this happen (not uglifyjs or some another stuff). As I know only rollup can do this, even webpack (only 2 version can do this, but still not stable) blindly get all file to build, which of course build systems, not ES6 modules.

Maybe if we'd have this kind of file tree generated:

// routes/utils.js

var Utils = {
  default_serializer: function(object, prefix) {
    /* ... */
  }
};

module.exports = Utils;

// routes/feedbackPath.js

const Utils = require('./utils');

// feedback => /feedback(.:format)
// function(options)
var feedbackPath = Utils.route([], ["format"], [2,[7,"/",false],[2,[6,"feedback",false],[1,[2,[8,".",false],[3,"format",false]],false]]], arguments);

module.exports = feedbackPath;

...then the imports could be directly to the actual route files, kinda like "manual tree shaking" because what we don't import will never get put to the bundle:

import feedbackPath from 'routes/feedbackPath';

webpack (only 2 version can do this, but still not stable)

For these fairly trivial use cases I'd say that webpack 2 would work. But yes, it isn't actually released yet so there might still be some quirks left... but the release will likely be soon: https://github.com/webpack/webpack/issues/1545#issuecomment-264121567

[valscion]

But all this changes can be done without converting to ES6.

I'm curious as what do you think about how easy it is to contribute to this repository. Do you think having the codebase written with ES6 would help with getting contributions compared having it in CoffeeScript?

I'd argue that the current approach of having only one CoffeeScript file for the JS part makes it unnecessarily difficult to contribute changes back to this repository. Given the increasing prevalence of ES2015+ code being used in favour of CoffeeScript, I fear that there are less and less people interested in writing CoffeeScript and to fix bugs or propose new useful features for this gem.

There are a lot of posts on the Internets where people are encouraging switching away from CoffeeScript to ES6/ES2015. One good question was over at Meteor forums: Use ES2015 or CoffeeScript?

Do you believe that this is an unnecessary fear? It very well might be, who knows ¯\_(ツ)_/¯

[le0pard]

The reason is that CommonJS modules are not statically analyzable, whereas ES6 modules are.

CommonJS modules are statically analyzable, if they doesn't contain code execution (like function calls). This mean, it should be statical. Our implementation is not support this, but we can fix this.

Unfortunately the CoffeeScript compiler adds the }).call(this); to the end of the file. That .call(this); breaks webpack as the this referenced in here is undefined.

We can add --bare option to coffescript compiler to compile JavaScript without this safety wrapper.

All the unused route declarations are still there.

it is how works webpack builder.

I'd only need that to be root = ROUTES; without the NAMESPACE in there at all.

If you are using es6 modules - yes, but our library should support usage with any conditions - global load in browser, amd and commonjs ways. So any solution should not break any another. NAMESPACE is a feature for users, which use global load in browser.

...then the imports could be directly to the actual route files, kinda like "manual tree shaking" because what we don't import will never get put to the bundle:

How this scripts suppose to work with systems, which not contain ES6 modules support (like amd, sprockets)? Because require will be undefined in this systems.

I'm curious as what do you think about how easy it is to contribute to this repository. Do you think having the codebase written with ES6 would help with getting contributions compared having it in CoffeeScript?

It is only depend from developer. If developer cannot learn another language and ES6 for them only source of truth for any problem - it is only this developer limits. We can also rewrite this to Typescript, Dart or even Ruby (http://opalrb.org/), but it still be compiled in JS code (mostly in ES5 version). BTW, many developers each year say, that Java will die, but it still one of top language in world - http://www.tiobe.com/tiobe-index/ and many cool projects still created on it.

I think good idea to fix call(this) and make export statically analyzable, but I still cannot find reason, what benefits will provide for us ES6 (I see only one - more developers know ES6, but I think good developers know more, than only ES6 - so it is even good).

[le0pard]

Also little problem, that right now we can build project by using only Ruby system. With ES6 we also should add Node.js and npm as dependency for building gems, so we will fight with additional level of abstraction: wrong Node.js version break build, babel config need to be updates, babel build scripts in invalid way. Like it was in version 5 - it was built default in root scope of module.exports (module.exports = DefaultExport), in babel 6 it was fixed and now it build in default scope of module.exports (module.exports = { default: DefaultExport })

[valscion]

How this scripts suppose to work with systems, which not contain ES6 modules support (like amd, sprockets)? Because require will be undefined in this systems.

Yeah, maybe this would just work for the special case of always using a JS bundler.

CommonJS modules are statically analyzable, if they doesn't contain code execution (like function calls). This mean, it should be statical. Our implementation is not support this, but we can fix this.

Huh, this could be interesting. I wonder what that would look like, though. Could be something worth looking into.

I agree that maybe these sorts of changes are really out of scope for this gem. It would, without a doubt, add much complexity to the build pipeline of this gem. Such pipeline could be hidden from the gem user, though.

Thank you for the conversation! I'll close this issue as I got great answers from you regarding my points.

[le0pard]

@valscion I propose to open new issue "Fix ES6 module integration". This is important for us, so we can by its remove call(this) and make exports statically analyzable. Thanks.

[valscion]

@valscion I propose to open new issue "Fix ES6 module integration". This is important for us, so we can by its remove call(this) and make exports statically analyzable. Thanks.

Seems like this had been an issue in the past but maybe some new versions of webpack has fixed it, or maybe we had configured things weirdly. I uncommented our special fix and all worked anyway.

[valscion]

Yeah, seems like some newer version of webpack had fixed any issues raised by that .call(this) — in fact, this is how it looks like in the generated bundle now.

      if (true) {
        !(__WEBPACK_AMD_DEFINE_ARRAY__ = [], __WEBPACK_AMD_DEFINE_RESULT__ = function () {
          return createGlobalJsRoutesObject();
        }.apply(exports, __WEBPACK_AMD_DEFINE_ARRAY__), __WEBPACK_AMD_DEFINE_RESULT__ !== undefined && (module.exports = __WEBPACK_AMD_DEFINE_RESULT__));
      } else {
        createGlobalJsRoutesObject();
      }
    }).call(undefined);

[le0pard]

@valscion Hm, better remove in lib call(this), because system expected, what in browser it will be window. But rollup and webpack make this in global scope equal undefined. So no benefits from this call(this)

[valscion]

Could be :). I don't know for sure how it behaves in the browser build, so I feel a bit uneasy if I'd open an issue for that case.