CoffeeScript compiling badly interacts with Babel

[mitar]

It seems there is a bad interaction with Babel and CoffeeScript. Recently, a change has been introduced to CoffeeScript compiling which allows Babel to process code inside backticks. The bad side-effect of this is that when Babel processes the code, it rewraps the list of defined variables, which breaks these assumptions. As a consequence, some exported variables cannot be defined because a local variable is defined.

The problem is even more problematic because even if a backtick is just inside a comment in the CoffeeScript (read: Markdown documentation) it triggers Babel processing, breaking exports.

This pull requests adds tests which break CoffeeScript in the above described way.

[benjamn]

As you probably realize, we can't merge this until it's fixed, though we do very much appreciate that you've provided these test cases (instead of just reporting the problem).

When we get around to fixing this, we'll open a pull request against mitar:coffeescript-babel, which you can review and merge, which will then update this PR, and hopefully the tests will pass here, and then we can merge this PR into devel. Sound good?

[mitar]

I can also just move the branch to this repository?

[mitar]

Or I can give permissions on that fork for whoever wants. But yes, pull request is also good.

[GeoffreyBooth]

Alternatively, now that CoffeeScript supports import and export, people could use that to share objects between files. Dropping support for shared variables would be a breaking change, though.

[GeoffreyBooth]

@benjamn I think this is actually a bug in the Babel compiler. So from this CoffeeScript file, with no backticks or modules and therefore no Babel postprocessing:

COFFEESCRIPT_EXPORTED = 123
COFFEESCRIPT_EXPORTED_ONE_MORE = 234
COFFEESCRIPT_EXPORTED_WITH_BACKTICKS = 345
class TestClass
class ClassExtending extends TestClass

we get this:

__coffeescriptShare = typeof __coffeescriptShare === 'object' ? __coffeescriptShare : {}; var share = __coffeescriptShare;
var ClassExtending, TestClass,                                                                                             
  extend = function(child, parent) { for (var key in parent) { if (hasProp.call(parent, key)) child[key] = parent[key]; } function ctor() { this.constructor = child; } ctor.prototype = parent.prototype; child.prototype = new ctor(); child.__super__ = parent.prototype; return child; },
  hasProp = {}.hasOwnProperty;                                          //
                                                                        //
COFFEESCRIPT_EXPORTED = 123;                                            // 1
                                                                        //
COFFEESCRIPT_EXPORTED_ONE_MORE = 234;                                   // 2
                                                                        //
COFFEESCRIPT_EXPORTED_WITH_BACKTICKS = 345;                             // 6
                                                                        //
TestClass = (function() {                                               // 13
  function TestClass() {}                                               //
                                                                        //
  return TestClass;                                                     //
                                                                        //
})();                                                                   //
                                                                        //
ClassExtending = (function(superClass) {                                // 15
  extend(ClassExtending, superClass);                                   //
                                                                        //
  function ClassExtending() {                                           //
    return ClassExtending.__super__.constructor.apply(this, arguments);
  }                                                                     //
                                                                        //
  return ClassExtending;                                                //
                                                                        //
})(TestClass);                                                          //

which works fine. Wrap the third line in backticks, though, to trigger Babel, and you get this:

__coffeescriptShare = typeof __coffeescriptShare === 'object' ? __coffeescriptShare : {}; var share = __coffeescriptShare;
var                                                                                                                //
    COFFEESCRIPT_EXPORTED_ONE_MORE,                                                                                //
    ClassExtending,                                                                                                //
    TestClass,                                                                                                     //
    extend = function extend(child, parent) {                                                                      //
  for (var key in meteorBabelHelpers.sanitizeForInObject(parent)) {                                                //
    if (hasProp.call(parent, key)) child[key] = parent[key];                                                       //
  }function ctor() {                                                                                               //
    this.constructor = child;                                                                                      //
  }ctor.prototype = parent.prototype;child.prototype = new ctor();child.__super__ = parent.prototype;return child;
},                                                                                                                 //
    hasProp = {}.hasOwnProperty;                                                                                   //
                                                                                                                   //
COFFEESCRIPT_EXPORTED = 123;                                                                                       //
                                                                                                                   //
COFFEESCRIPT_EXPORTED_ONE_MORE = 234;                                                                              //
                                                                                                                   //
COFFEESCRIPT_EXPORTED_WITH_BACKTICKS = 345;                                                                        //
                                                                                                                   //
TestClass = function () {                                                                                          //
  function TestClass() {}                                                                                          //
                                                                                                                   //
  return TestClass;                                                                                                //
}();                                                                                                               //
                                                                                                                   //
ClassExtending = function (superClass) {                                                                           //
  extend(ClassExtending, superClass);                                                                              //
                                                                                                                   //
  function ClassExtending() {                                                                                      //
    return ClassExtending.__super__.constructor.apply(this, arguments);                                            //
  }                                                                                                                //
                                                                                                                   //
  return ClassExtending;                                                                                           //
}(TestClass);                                                                                                      //

The important thing to note here is that COFFEESCRIPT_EXPORTED_ONE_MORE is added to the var statement (line 3 in the latter JavaScript block). This is just completely wrong, and this is Babel’s doing (or the doing of whatever Meteor package wraps it). It’s all the more perplexing because for whatever reason the other COFFEESCRIPT_EXPORTED variables don’t receive the same treatment. It’s triggered by the class extends; take out the second class statement, and the bug goes away.

Do you know where the code is that generates this var statement?

[GeoffreyBooth]

@benjamn I take it back. It’s not Babel’s fault. It’s compile-coffeescript.js’ stripExportedVars.

Or more specifically, it’s the fact that stripExportedVars was written expecting the output from the CoffeeScript compiler, not the output from the Babel compiler. CoffeeScript creates a var line near the top that is a long one-liner: var a, b, c;. Babel converts such a line to a multiline construction:

var a,
  b,
  c;

Or at least it does in the case of @mitar’s tests.

Rather than rewrite stripExportedVars to handle a multiline var statement, we can just reorder the steps so that stripExportedVars happens before the Babel compilation. So now it’s CoffeeScript compilation, then stripExportedVars, then Babel compilation. This new order passes @mitar’s tests.

I submitted the fix as mitar#1, per @benjamn’s instructions above. @mitar, after you accept the PR you should merge the latest upstream devel into your branch, to incorporate the updates from #7818. It might trigger a mild merge conflict, which should be easy to resolve.

[mitar]

Merged, updated, merged, and tests are running.

[mitar]

Updated once more.

[GeoffreyBooth]

@benjamn I think this should be ready for review.

[benjamn]

Great work @mitar and @GeoffreyBooth!

[mitar]

Can you bump the coffeescript Meteor package version please? ;-)

See my comment here: 8dbfd97

[mitar]

Can we make this 1.11.1_1?