gcanti / babel-plugin-tcomb

Babel plugin for static and runtime type checking using Flow and tcomb
MIT License
482 stars 22 forks source link

return types on fat-arrow functions lose access to `this` #134

Closed nrser closed 8 years ago

nrser commented 8 years ago

i'm using version 0.3.17 and seeing fat-arrow functions lose their this binding when they have return types declared.

source:

export class A {
  constructor() {
    this.x = 'ex';
  }

  f() {
    [1, 2, 3].forEach((n): string => {
      console.log(this.x);
    });
  }
}

const a = new A();
a.f();

compiled with skipAsserts = false:

// ...

var A = exports.A = function () {
  function A() {
    _classCallCheck(this, A);

    this.x = 'ex';
  }

  _createClass(A, [{
    key: 'f',
    value: function f() {
      [1, 2, 3].forEach(function (n) {
        var ret = function (n) {
          console.log(this.x);
        }.call(this, n);

        _assert(ret, _tcomb2.default.String, 'return value');

        return ret;
      });
    }
  }]);

  return A;
}();

var a = new A();
a.f();

// ...

running produces

          console.log(this.x);
                          ^

TypeError: Cannot read property 'x' of undefined

works fine with skipAsserts = true:

// ...

var A = exports.A = function () {
  function A() {
    _classCallCheck(this, A);

    this.x = 'ex';
  }

  _createClass(A, [{
    key: 'f',
    value: function f() {
      var _this = this;

      [1, 2, 3].forEach(function (n) {
        console.log(_this.x);
      });
    }
  }]);

  return A;
}();

var a = new A();
a.f();

// ...

notice the _this = this and _this.x that are missing from the top compilation.

for now i just set skipAsserts = true to mitigate it. thanks in advance, Neil.

nrser commented 8 years ago

just noticed that the return type on the fat arrow function doesn't make any sense but it shouldn't make any difference.

gcanti commented 8 years ago

Ah sure, thanks for reporting. Here https://github.com/gcanti/babel-plugin-tcomb/blob/master/src/index.js#L530 I wrongly used an identifier but there is a proper ast node: thisExpression. Will fix asap

gcanti commented 8 years ago

Released a fix

nrser commented 8 years ago

awesome, i'll upgrade and try it out