1.6.0 Broke overwriting a bound method

[ansman]

Consider this code:

class Foo
  someMethod: => alert('Foo')

class Bar extends Foo
  someMethod: => alert('Bar')

new Bar().someMethod()

In 1.6.0 it alerts Foo while in 1.5.0 it alerts Bar.
I think that this is not the intended behaviour, how does one overwrite a method now?

It seems to be caused by the super constructor running last in the child's constructor, shouldn't it be the other way around?

[michaelficarra]

This has nothing to do with bound methods. It's because the default constructor in subclasses has changed. Now it is returning the instance returned by calling the superclass's constructor.

[ansman]

Well, in 1.5 this was done in the constructor when you bind methods:

function Foo() {
  this.someMethod = __bind(this.someMethod, this);
}

While in 1.6 this is how it looks:

function Foo() {
  _this = this;
  this.someMethod = function() {
    return Foo.prototype.someMethod.apply(_this, arguments);
  };
}

[michaelficarra]

Yeah, and the only affect that has is a proper arity and respecting prototype changes. The important line you're looking for is in the `Bar` constructor:

return Bar.__super__.constructor.apply(this, arguments);

[epidemian]

@michaelficarra, i think @ansman is right. The change of the added return has no effect here, as the superclass constructor returns undefined.

Compare the 1.4.0 output:

// Generated by CoffeeScript 1.4.0
(function() {
  var Bar, Foo,
    __bind = function(fn, me){ return function(){ return fn.apply(me, arguments); }; },
    __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; };

  Foo = (function() {

    function Foo() {
      this.someMethod = __bind(this.someMethod, this);

    }

    Foo.prototype.someMethod = function() {
      return alert(Foo);
    };

    return Foo;

  })();

  Bar = (function(_super) {

    __extends(Bar, _super);

    function Bar() {
      this.someMethod = __bind(this.someMethod, this);
      return Bar.__super__.constructor.apply(this, arguments);
    }

    Bar.prototype.someMethod = function() {
      return alert(Bar);
    };

    return Bar;

  })(Foo);

  new Bar().someMethod();

}).call(this);

With the 1.6.0 one:

// Generated by CoffeeScript 1.6.0
(function() {
  var Bar, Foo,
    _this = this,
    __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; };

  Foo = (function() {

    function Foo() {
      var _this = this;
      this.someMethod = function() {
        return Foo.prototype.someMethod.apply(_this, arguments);
      };
    }

    Foo.prototype.someMethod = function() {
      return alert(Foo);
    };

    return Foo;

  })();

  Bar = (function(_super) {

    __extends(Bar, _super);

    function Bar() {
      var _this = this;
      this.someMethod = function() {
        return Bar.prototype.someMethod.apply(_this, arguments);
      };
      return Bar.__super__.constructor.apply(this, arguments);
    }

    Bar.prototype.someMethod = function() {
      return alert(Bar);
    };

    return Bar;

  })(Foo);

  new Bar().someMethod();

}).call(this);

The difference is that CS used to pass this.someMethod to __bind, thus binding this to the someMethod of the last prototype in the chain. Both the superclass and subclass constructors did the same thing basically (because this.someMethod was the same function), so it didn't matter in what order they did it.

Now that this is bound to the current constructor's prototype's someMethod, which will defer from superclass and subclass in this case, the order in which the constructors bind the method matter. The subclass constructor should bind the methods after the superclass constructor.

Shouldn't be a difficult fix i think.

[jashkenas]

Pushed a fix that should do the trick. But there may still be subtleties. Take a peek and let me know what you think.

[michaelficarra]

@epidemian: Ah, yes, my mistake. I was confused. So in which way do we want to fix it? We could either apply the superclass constructor before any bound method overrides or add some condition around the assignment of the bound method.

edit: Looks like Jeremy was too fast. Re-ordering it is.

[jashkenas]

I'm still concerned about cases where either the subclass constructor, or the superclass constructor want to use one of these bound functions themselves.

[michaelficarra]

@jashkenas: That's a valid concern. Then move the bound constructor assignment to the top with a guard like this:

edit: Never mind! I need to go to bed.

[jashkenas]

I'm pretty sure that won't work. If super is called after the method is bound, it patches over it with the parent's implementation. That's the original bug.

[michaelficarra]

Whoops!

[shesek]

Couldn't it be fixed by using the methods from this.constructor.prototype? That way, it'll take the prototype from the constructor of the dynamically bound this, rather than the constructor of the class that the fat-arrow method is defined in. Something like that:

function A() {
  var _this = this, _proto = _this.constructor.prototype;
  this.fat = function() {
    return _proto.fat.apply(_this, arguments);
  };
}

[michaelficarra]

@shesek: That's a good idea. CoffeeScriptRedux compiles in this way:

Bar = function (super$) {
  extends$(Bar, super$);
  function Bar() {
    var instance$;
    instance$ = this;
    this.someMethod = function () {
      return Bar.prototype.someMethod.apply(instance$, arguments);
    };
  }
  Bar.prototype.someMethod = function () {
    return alert('Bar');
  };
  return Bar;
}(Foo);

I'm thinking your way is better, since you have a reference to the actual [[Prototype]] object, rather than the current prototype property of the constructor. That is, of course, assuming A.prototype.constructor still points to A. I would recommend you pull _proto directly from the constructor: _proto = A.prototype.

[shesek]

@michaelficarra I was using this.constructor.prototype so that you would always get the prototype of the child class being constructed, rather than that of the parent class where the fat-arrow method was defined. Using A.prototype directly could still cause the original issue (that is, without the re-ordering that was committed).

It is a bit problematic to rely on A.prototype.constructor to still point to A, as naive implementations of inheritance could mess that up. A better solution would be using Object.getPrototypeOf, but it wouldn't work well on all browsers.

Perhaps a utility function that uses getPrototypeOf when available, than falls back to using .constructor.prototype (something like __getProto = Object.getPrototypeOf or (o) -> o.constructor::) can be used instead? The drawback is that the behavior across different browsers would be different - it might work on one browser with getPrototypeOf, but fail in another because the .constructor property is incorrect.

edit: actually, since this is only relevant in classes defined using coffee's class syntax, the constructor property is guaranteed to be correct, so it should be fine.