5

I have a container that I need to change the UI form showing the form or showing a success page.

The container has a state.showSuccess and I need the MyFormModule to be able to call the container to change the state.

The below code works but I'm getting the following warning:

JSX props should not use .bind()

How can I get this to work without using .bind()?

...
const myPage = class extends React.Component {
  state = { showSuccess: false };
  showSuccess() {
   this.setState({
      showSuccess: true,
    });
  }
  render() {
    const { showSuccess } = this.state;
    if (showSuccess) {...}
    ....
    <MyFormModule showSuccess={this.showSuccess.bind(this)} />
Penny Liu
  • 15,447
  • 5
  • 79
  • 98
AnApprentice
  • 108,152
  • 195
  • 629
  • 1,012

3 Answers3

18

You should first understand WHY this is a bad practice.

The main reason here, is that .bind is returning a new function reference.
This will happen on each render call, which may lead to a performance hit.

You got 2 options:

  1. Use the constructor to bind your handlers (this will run only once).

    constructor(props) {
      super(props);
      this.showSuccess = this.showSuccess.bind(this);
    }
    
  2. Or create your handlers with arrow functions so they will use the lexical context for this, hence you won't need to bind them at all (you will need a babel plugin):

    showSuccess = () => {
      this.setState({
        showSuccess: true,
      });
    }
    

You should not use this pattern (as others suggested):

showSuccess={() => this.showSuccess()}

Because this will as well create a new function on each render.
So you may bypass the warning but you are still writing your code in a bad practice design.

From the ESLint docs:

A bind call or arrow function in a JSX prop will create a brand new function on every single render. This is bad for performance, as it will result in the garbage collector being invoked way more than is necessary. It may also cause unnecessary re-renders if a brand new function is passed as a prop to a component that uses reference equality check on the prop to determine if it should update.

Sagiv b.g
  • 30,379
  • 9
  • 68
  • 99
  • You're right, it is a potential performance issue (as the docs admit), but in general, [it's probably okay](https://reactjs.org/docs/faq-functions.html#is-it-ok-to-use-arrow-functions-in-render-methods) – CRice Jan 05 '18 at 22:18
  • 2
    Yeah, if the OP has a linting rule that warns about "_JSX props should not use.bind()_" then we might want to explain why this warning exists and how to make the code better and not bypass the rule :) – Sagiv b.g Jan 05 '18 at 22:20
2

Use an arrow function when defining showSuccess

showSuccess = () => {
  this.setState({
    showSuccess: true,
  });
} 
Eric Hasselbring
  • 1,374
  • 1
  • 10
  • 18
0

Use an arrow function since they automatically inherit the this context of wherever they are defined.

showSuccess={() => this.showSuccess()}

Here is a link to the facebook documentation on this subject, which lists this method among others as a solution. Interestingly, they also list using .bind in the prop as one of the solutions, even though it produces a warning when actually used.

From that documentation, you'll note that this is a potential performance issue, since the function will be recreated on every render:

Note:

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

But also from the same link:

Is it OK to use arrow functions in render methods? Generally speaking, yes, it is OK, and it is often the easiest way to pass parameters to callback functions.

If you do have performance issues, by all means, optimize!

So I would say if your component will be re-rendering very frequently, you should use one of the other solutions: bind in the constructor, or define the method with an arrow function in the first place. But if not, use whatever method seems cleanest to you.

CRice
  • 29,968
  • 4
  • 57
  • 70
  • this is the same bad practice. it will create a new function instance same as `.bind` does. – Sagiv b.g Jan 05 '18 at 22:06
  • its an anonymous function, it will get created with every render – Eric Hasselbring Jan 05 '18 at 22:21
  • Yeah, if the OP has a linting rule that warns about "_JSX props should not use.bind()_" then we might want to explain why this warning exists and how to make the code better and not bypass the rule :) – Sagiv b.g Jan 05 '18 at 22:21