React jsx suggestion: avoiding `this.method.bind(this)`

In many of our components, we see a pattern of binding the this context on event-handlers

Example:

class MyCounter extends React.Component {
  constructor(props) {
    super(props);

    this.state = { count: 0 };

    helpers.bindMethods(this, ['handleClick']);
  }

  handleClick(event) {
    this.setState({ count: this.state.count + 1 });
    event.preventDefault();
  }

  render() {
    return <div>
      {this.state.count}
      <button onClick={this.handleClick}>+1</button>
    </div>;
  }
}

I know this approach is presented everywhere when you are learning about React, but it starts to feel like an anti-pattern for me.

I would prefer to take the advantage of jsx and write those components as well:

class MyCounter extends React.Component {
  constructor(props) {
    super(props);

    this.state = { count: 0 };
  }

  handleClick(event) {
    this.setState({ count: this.state.count + 1 });
    event.preventDefault();
  }

  render() {
    return <div>
      {this.state.count}
      <button onClick={(e) => this.handleClick(e)}>+1</button>
    </div>;
  }
}

It also helps me when I can see from far away all the events-handler in my renderer.
I don’t believe we should rush and start updating it everywhere but would love to see the jsx syntax more.

  • Agree
  • Disagree

0 voters

Cheers!

I have no strong opinion about this. I’m ok with standardizing on usage of the jsx wrapper method from this point on. But as you write, I’m against re-writing of what we already have.

No strong opinion either, but the current pattern looks easier to read to me. Moreover if it’s recommended in many documentation/blogs it means new contributors will be used to read that code, so there’s less cognitive burden trying to understand our code.

Why do you consider it an anti-pattern?

We had to use binding before es6 and the new arrow functions syntax.
Today, since we do have the arrow synntax, it feels like an anti-pattern.

At those days we use to do:

// doSomethinng exist only one time in your memory
function MyClass(props) {}
MyClass.prototype.doSomething = function(args) {};
// each time you creating a new instance you creating new function in the memory
// should not do that without a real use-case
function MyClass(props) {
  this.doSomething = function (args) {};
}

Combining both approaches make it even weirder,
first, you have a prototype function,
then, you create a new function for each instance that runs the prototype function with the current this as the context.

And you are basically don’t need to do that, the this is already your context natively.
You will lose the this context only when you will treat this function as a callback.

So in the old days:

MyClass.prototype.doSomething = function(args) { this.doSomethinngElse(); };

var instance = new MyClass();

// because i am not use `this.` i am loosing the this
function callCallback(callback) { callback() }

// so this will break
callCallback(instance.doSomething);

// This will work
callCallback(instance.doSomething.bind(instance));

// today can be much more elegant so we don't need to work hard and overthinnking about binding
// my rule never passes a callback anymore (unless it make sense), always use arrow syntax
callCallback(() => instance.doSomething());

I don’t think bind function is an anti-pattern, plus I think it’s readable too.
In this post you can find some options to bind `this’ in react.
This post mentions performance issues with arrow functions in render.

notice that React.createClass binds this automatically, though it’s not ES6.

In addition, the bind function (in the constructor) approach is suggested by react as well
while using arrow functions in render function comes with this warning:

Using an arrow function in render creates a new function each time the component renders, which may have performance implications (see below).

OK, you convinced me here, let’s keep it as it now to keep consistency.
I don’t believe it can actually affect the performance except for some unique-cases.

I still feel it’s more reasonable to understand:

<button onClick={(e) => this.handleClick(e)} />

then:

<button onClick={this.handleClick} />