Unbound methods, interest in autobinding?

[marionebl]

I noticed methods on all objects are unbound, e.g.

// https://tonicdev.com/marionebl/5755b0178b908f130020fd9d
var Github = require('github-api');
var github = new Github();
var getRepo = github.getRepo;

var repo = getRepo();
// => Cannot read property '_getFullName' of undefined

Of course this can be fixed easily by changing it to:

// https://tonicdev.com/marionebl/5755b4e28b908f130020fedb
var Github = require('github-api');
var github = new Github();
var getRepo = github.getRepo.bind(github);

var repo = getRepo();

This is rather clumsy and moves the work to bind the methods to their respective host objects to users although it could easily be done by the library.

1. Are you interested in a PR that auto binds all methods?
2. Which would be the preferred way to do it?

• Assignment in constructor:

// https://tonicdev.com/marionebl/5755b612c91caa1300f72617
...
this.bar = this.bar.bind(this);
...

• Tool-facilitated assignment in constructor

https://tonicdev.com/marionebl/5755b612c91caa1300f72617
import autobind from '...';
...
autobind(this);
...

• Class properties (needs additional experimental babel plugins)

...
  bar = () => {
    return this.__barMessage;
  }
...

• An autobind decorator (needs experimental/disabled decorator babel plugin)

@autobind()
class Foo {
...
}

[clayreimann]

While the issue you document will currently occur–Javascript in general behaves in this manner–is there a reason to have everything bound by default?

Could you elaborate on your use case for having methods bound? Why not just pass around the objects that the library returns to you?

[marionebl]

The reason for binding methods on the returned objects is purely to provide a more convenient developer interface. An example use case would be e.g. conversion to promise based / async-await control flows:

import denodeify from 'denodeify';
import Github from 'github-api';

const github = new Github();
const getRepo = denodeify(github.getRepo);

async function main() {
 const repo = await getRepo();
 return repo;
}

main()
  .then()
  .catch(error => {
     // barfs with:
     // => TypeError: Cannot read property '_request' of undefined
  });

This is fixed with a simple function.prototype.bind call, but I'd argue this is inconvenient for library users and adds to general boilerplate code needed.

[clayreimann]

Why not:

import Github from 'github-api';

const github = new Github();

async function main() {
 const repo = await github.getRepo('michael', 'github');
 return repo;
}

main()
  .then(repoJson => {
    // do stuff
  }).catch(error => {
  });

You're going to need to pass things around (whether they're functions or the github instance) so I don't see any advantage to pulling instance methods off of the instance.

I guess to answer the question you asked I don't think that this is something we'd like to support by default. However, if you submit a PR that enables this behind a flag or option upon instantiation then I guess I don't have a problem with it. It just doesn't feel very javascript-y.

See React's reasoning for dropping support for auto-bound methods like this as they transition to ES6+ usage.

[marionebl]

Thanks for the clarification and fast response. Makes a lot of sense to me. 👍