Introduce a --no-bound-method-check flag

[Goues]

Hello, per my comment in #4561, there is an issue with boundMethodCheck and scope. It's added on top of the file, which pollutes global scope when using Google Closure Compiler and the check doesn't work. If the project doesn't need the check, it could be turned off.

[GeoffreyBooth]

I’m sorry, but it’s a project design decision that we’re not adding any further flags that influence compilation output. This was discussed during the CoffeeScript 2 project, and the consensus was that people should be able to look at CoffeeScript source and know how it will be output without needing to also need to inspect how it’s being compiled (i.e. the opposite of TypeScript, where seeing TypeScript source doesn’t tell you all you need to know without also consulting the relevant tsconfig.json).

[Goues]

I understand. Is there any way I could make CoffeeScript output valid while complying with these decisions? At the moment, the output is invalid as soon as I use any class A extends B {} with a bound method.

Consider this. File A:

goog.require 'goog.ui.Component'
goog.provide 'namespace.Parent'

class namespace.Parent extends goog.ui.Component

    constructor: ->
        something.addEventListener 'event-a', @onEventA

    onEventA: ->
        console.log this

File B:

goog.require 'namespace.Parent'
goog.provide 'namespace.Child'

class namespace.Child extends namespace.Parent

    constructor: ->
        something.addEventListener 'event-b', @onEventB

    onEventB: ->
        console.log this

If I run compilation for this, it puts

var ref,
    boundMethodCheck = function (instance, Constructor) {
  if (!(instance instanceof Constructor)) {
    throw new Error('Bound instance method accessed before binding');
  }
};

and also does

ref = namespace.Parent

and

ref = namespace.Child

in them respectively. This means that when this gets bundled, ref is always equal to namespace.Child and any call to Parent.onEventA fail on the boundMethodCheck.

Before you say that bare solves this, it's invalid because goog.require and goog.provide work like native import/export, so they cannot be inside a closure.

This means we're stuck in a place where we cannot use neither bare nor not-bare.

So I am willing to explore ideas that would be mergeable.

For example, CS handles import/export, could goog.require/provide be put on the same level so that if the output has a wrapper, the module system stays on top level? What about using a different variable name, now everything is just ref, so last one out wins, if the variable was unique, it wouldn't be a problem either.

The output of CS 2 is about 20 % smaller just because it uses ES6, so it's a huge benefit that is at the moment, only spoiled by this single check.

Thanks you for any advice.

[GeoffreyBooth]

Individual files are inherently scoped. A var ref = in one file has no relation to var ref = in another file. This sounds more like an issue with how you’re bundling multiple files together. Perhaps you need a more sophisticated bundler that renames variables as needed when things like a module-scoped ref or boundMethodCheck would collide because two module files are being merged into one.

[Goues]

Personally, I wouldn't say that google closure compiler is not sophisticated enough, but I guess we will just have to stick with v1. :(

Thank you for your time.

[ch1c0t]

@Goues Can you replace goog.require and goog.provide with something else?

You can create your own macros to handle dependencies. I do this to implement window.FROM.

I use the AST from CoffeeScript's compile, and then prepare a file declaring dependencies.

[Goues]

The entire stack is google-based, so it's google closure compiler with google closure library and coffeescript thrown into it. It's a java bundler with java tooling that is built around goog.require, goog.provide and namespaces, everything is defined in global space, which is why a new variable ref put at the top level of a file is global - in google closure, it should be namespaced.

There are definitely other approaches (goog.module), but they are probably not worth the effort compared to a micro patch like the one here.

If I was to use AST, I would use it to remove the injected check as well instead of trying to work around the top-level scope.

Anyway, this setup is a very niche corner of the internet and I don't expect anyone else to have it, so please don't spent too much time on this, a single review and feedback is all I could hope for 👍

[GeoffreyBooth]

boundMethodCheck isn’t the only helper that CoffeeScript adds to the tops of files. See https://github.com/jashkenas/coffeescript/blob/master/lib/coffeescript/rewriter.js#L10-L11 for example. If your build process is losing these as part of bundling, there are possibly other things broken with your final output, even in v1.

Another thing to consider is whether you need to merge the files together at all. Keeping all the ESM files separate would also avoid this issue, and with HTTP/2 commonplace nowadays there might not be a need to create a single bundle.

[Goues]

I started looking around for existing work and I found couple projects for coffeescript v1, one of which we are using.

There used to be https://www.npmjs.com/package/coffee2closure, the source code is now gone, but since we're actually using this with CS1, we have a private fork of this.

There is also a fork of CS itself in https://github.com/hleumas/coffee-script/wiki that introduces a compiler flag to make output compatible with google closure. And even an older attempt in http://bolinfest.com/coffee/.

I guess I was too optimistic thinking that as CS2 can output ES6 classes and is in general more compatible with JS, we could get rid of any custom post-transformation (like actually making the classes compatible). From my initial attempts at upgrading, it looked like boundMethodCheck was the only issue, but I now have an understanding that there may, and probably will, be a cascade of issues down the road.

All projects for google closure compatibility with coffeescript were sunset a long time ago, which should tell us how legacy the stack can be considered in the first place.

[GeoffreyBooth]

CoffeeScript just outputs JavaScript, and CoffeeScript 2 is much more standard JavaScript. Any project that prepares JavaScript for Closure should do the trick; you could look for one that processes TypeScript output to prepare it for Closure compilation, as TypeScript would present many of the same issues with helper functions and generated variables.