pthrasher / snockets

Sprockets-style script concatenation for Node
119 stars 39 forks source link

CoffeeScript classes result in verbose compilation #26

Closed knpwrs closed 11 years ago

knpwrs commented 12 years ago

Consider the following example:

Animal.coffee:

class Animal

Cat.coffee:

#= require Animal
class Cat extends Animal

Dog.coffee:

#= require Animal
class Dog extends Animal

app.coffee:

#= require Cat
#= require Dog
cat = new Cat()
dog = new Dog()

The following is the output of compiling app.coffee:

(function() {
  var Animal;

  Animal = (function() {

    function Animal() {}

    return Animal;

  })();

}).call(this);

(function() {
  var Dog,
    __hasProp = {}.hasOwnProperty,
    __extends = 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; };

  Dog = (function(_super) {

    __extends(Dog, _super);

    function Dog() {
      return Dog.__super__.constructor.apply(this, arguments);
    }

    return Dog;

  })(Animal);

}).call(this);

(function() {
  var Cat,
    __hasProp = {}.hasOwnProperty,
    __extends = 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; };

  Cat = (function(_super) {

    __extends(Cat, _super);

    function Cat() {
      return Cat.__super__.constructor.apply(this, arguments);
    }

    return Cat;

  })(Animal);

}).call(this);

(function() {
  var cat, dog;

  cat = new Cat();

  dog = new Dog();

}).call(this);

There are two issues with the way this is compiled:

  1. First, Cat, Dog, and Animal are out of scope of each other so inheritance doesn't work and they are defined outside the scope of the compiled app.coffee so they cannot be instantiated.
  2. Second, CoffeeScript is littering the output of each compiled file with their own __extends and __hasProp functions. Even if each file is compiled individually with --bare a minifier would not know that only one of these declarations is necessary (I have tested uglifyjs and closure, they both overwrite the extension functionality for each declared class).

My proposed solution is to concatenate CoffeeScript files in the proper order and then compile them. Following that process manually we end up with the following output:

(function () {
  var Animal, Cat, Dog, cat, dog,
    __hasProp = {}.hasOwnProperty,
    __extends = 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; };

  Animal = (function() {

    function Animal() {}

    return Animal;

  })();

  Cat = (function(_super) {

    __extends(Cat, _super);

    function Cat() {
      return Cat.__super__.constructor.apply(this, arguments);
    }

    return Cat;

  })(Animal);

  Dog = (function(_super) {

    __extends(Dog, _super);

    function Dog() {
      return Dog.__super__.constructor.apply(this, arguments);
    }

    return Dog;

  })(Animal);

  cat = new Cat();
  dog = new Dog();
}).call(this);

Now we only have one declaration of __extends and __hasProp and everything is in proper scope. I would be happy to go ahead and code this up and send a pull request but I just wanted to see if you had any concerns first.

adriantoine commented 12 years ago

Hi, I've been having exactly the same issue. I think that it compiles Coffee files to JS first (each is wrapped in a private scope anonymous function), then concatenate the JSs files. What we need is an option to concatenate the Coffee files first, then compile the big coffee file to a JS one. It would be a really great improvement.

bernatfortet commented 11 years ago

Same here :+1:

pthrasher commented 11 years ago

This is an interesting idea.

Not sure if, or how I'd implement this, but I'll definitely consider it. Probably won't make it into the upcoming 2.0 release, but perhaps a release after.

pthrasher commented 11 years ago

After further thought, this would be incredibly tricky to implement. Mainly due to snocket's mixed-asset nature. This becomes difficult or impossible as soon as there's a non-coffee file in the chain. If you can come up with a way around it, I'm open to pull requests. I'm working on the new 2.0.0 branch in #43 -- feel free to work off of that. It currently has source map support, and I've upgraded coffee-script and uglifyjs to latest.

For now, I'm closing this issue. Thanks a ton for submitting this, and I look forward to any solutions you may have to offer.

knpwrs commented 11 years ago

Cheap hack, but what if you just wrapped the contents of the js files in backticks? This is how embedded JavaScript works in CoffeeScript.

Consider the following files:

Animal.coffee:

class Animal

Cat.coffee:

class Cat extends Animal

Dog.coffee:

class Dog extends Animal

init.js

//= require Cat
//= require Dog
var kitty = new Cat(), puppy = new Dog();

app.coffee

#= require init
console.log kitty
console.log puppy

That would result in a concatenation similar to the following:

class Animal
class Cat extends Animal
class Dog extends Animal
`
var kitty = new Cat(), puppy = new Dog();
`
console.log kitty
console.log puppy

Which, when run through coffee, compiles to the following:

// Generated by CoffeeScript 1.6.2
(function() {
  var Animal, Cat, Dog, _ref, _ref1,
    __hasProp = {}.hasOwnProperty,
    __extends = 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; };

  Animal = (function() {
    function Animal() {}

    return Animal;

  })();

  Cat = (function(_super) {
    __extends(Cat, _super);

    function Cat() {
      _ref = Cat.__super__.constructor.apply(this, arguments);
      return _ref;
    }

    return Cat;

  })(Animal);

  Dog = (function(_super) {
    __extends(Dog, _super);

    function Dog() {
      _ref1 = Dog.__super__.constructor.apply(this, arguments);
      return _ref1;
    }

    return Dog;

  })(Animal);

var kitty = new Cat(), puppy = new Dog();
;

  console.log(kitty);

  console.log(puppy);

}).call(this);

It results in an extra semi-colon but that doesn't matter once run through uglifyjs.