jsheroes/community-help

Class autobind - what's the cleanest way?

alexnm opened this issue ยท 8 comments

So I'm thinking of switching from React.createClass to the ES2015 class syntax (for various reasons) and I definitely don't want to manually wire each function inside each component, so I read a couple of articles on autobinding solutions and I'm curious which one do you use? (if you use)

A few links:

My current idea is to go for something like this: https://github.com/cassiozen/React-autobind as I don't want to introduce syntax from ES next into the project.

For a non-React project we're using this simple function to bind methods to the instance:

function bindToInstance(instance, methods){
  for (var i = 0; i < methods.length; i++) {
    instance[methods[i]] = instance[methods[i]].bind(instance);
  }
}
// then:
bindToInstance(this, [
  "mouseDownHandler",
  "mouseMoveHandler",
  "mouseUpHandler",
  ...
]);

i.e. bind an explicit list of methods to the object rather than autobind everything. Keeping the methods as strings has the disadvantage of hindering some types of code obfuscation (you can't rename the methods if you have string references to them), and it also might hinder types of automatic code refactoring -- these two issues don't affect us, but to solve it you'd indeed need to get more verbose (this.mouseDownHandler = this.mouseDownHandler.bind(this), etc).

If you don't mind autobinding all methods, I can't think of a cleaner approach than with the utility you shared. I prefer explicitly binding because it gives me a sense of clarity (I can tell how much event handling is going on in the object) and zero magic.

Edit: And yes, binding in the constructor sounds like the best idea out of the alternatives (fat arrow functions, binding in render, etc.)

I'm using class instance properties, e.g.:

class A extends Component {
  handleClick = (event) => {
    // ...
  }
}

or you could bind them in constructor if you don't like the idea of using unreleased language features.

So no auto-binding, just bind what you know it will be invoked in another context but it uses this keyword.

After a second thought, I think the best approach is to start with manual bind in constructor, since I like the idea of "seeing" the magic. After all, having more than 3-4 bindings might be a code smell for that component

@alexnm I came to the same conclusion, even on complex components only a few methods need binding.

I have a question of my own: is there a way to avoid creating new functions in render() if you have to have a handler on each item in a list? e.g.

render() {
   ...
   {
      items.map(function(item) {
        return <ListItem key={item.id} onClick={ (e) => { this.doSomethingWith(item) } }/>
      })
    }
}

or, in general, smarter ways of handing events on each item in a list? In a vanilla project I'd have a single handler on the list itself.

or is it just overkill?

I generally add a method that creates the handler by returning a new function that will handle the event

createDoSomethingHandler = (item) => (event) => {
  // ... you have item and event available 
};

render() {
   ...
   {
      items.map(function(item) {
        return <ListItem key={item.id} onClick={this.createDoSomethingHandler(item)}/>
      })
    }
}

Thanks to everyone that joined the conversation! A lot of good ideas were shared

We've created https://www.npmjs.com/package/react-class - and it serves us pretty well, and seems to be pretty popular :)