172

I'm running lint with my React app, and I receive this error:

error    JSX props should not use arrow functions        react/jsx-no-bind

And this is where I'm running the arrow function (inside onClick):

{this.state.photos.map(tile => (
  <span key={tile.img}>
    <Checkbox
      defaultChecked={tile.checked}
      onCheck={() => this.selectPicture(tile)}
      style={{position: 'absolute', zIndex: 99, padding: 5, backgroundColor: 'rgba(255, 255, 255, 0.72)'}}
    />
    <GridTile
      title={tile.title}
      subtitle={<span>by <b>{tile.author}</b></span>}
      actionIcon={<IconButton onClick={() => this.handleDelete(tile)}><Delete color="white"/></IconButton>}
    >
      <img onClick={() => this.handleOpen(tile.img)} src={tile.img} style={{cursor: 'pointer'}}/>
    </GridTile>
  </span>
))}

Is this a bad practice that should be avoided? And what's the best way to do it?

KadoBOT
  • 2,944
  • 4
  • 16
  • 34

10 Answers10

252

Why you shouldn't use inline arrow functions in JSX props

Using arrow functions or binding in JSX is a bad practice that hurts performance, because the function is recreated on each render.

  1. Whenever a function is created, the previous function is garbage collected. Rerendering many elements might create jank in animations.

  2. Using an inline arrow function will cause PureComponents, and components that use shallowCompare in the shouldComponentUpdate method to rerender anyway. Since the arrow function prop is recreated each time, the shallow compare will identify it as a change to a prop, and the component will rerender.

As you can see in the following 2 examples - when we use inline arrow function, the <Button> component is rerendered each time (the console shows the 'render button' text).

Example 1 - PureComponent without inline handler

class Button extends React.PureComponent {
  render() {
    const { onClick } = this.props;
    
    console.log('render button');
    
    return (
      <button onClick={ onClick }>Click</button>
    );
  }
}

class Parent extends React.Component {
  state = {
    counter: 0
  }
  
  onClick = () => this.setState((prevState) => ({
    counter: prevState.counter + 1
  }));
  
  render() {
    const { counter } = this.state;
    
    return (
      <div>
        <Button onClick={ this.onClick } />
        <div>{ counter }</div>
      </div>
    );
  }
}

ReactDOM.render(
  <Parent />,
  document.getElementById('root')
);
<script crossorigin src="https://unpkg.com/react@16/umd/react.production.min.js"></script>
<script crossorigin src="https://unpkg.com/react-dom@16/umd/react-dom.production.min.js"></script>
<div id="root"></div>

Example 2 - PureComponent with inline handler

class Button extends React.PureComponent {
  render() {
    const { onClick } = this.props;
    
    console.log('render button');
    
    return (
      <button onClick={ onClick }>Click</button>
    );
  }
}

class Parent extends React.Component {
  state = {
    counter: 0
  }
  
  render() {
    const { counter } = this.state;
    
    return (
      <div>
        <Button onClick={ () => this.setState((prevState) => ({
          counter: prevState.counter + 1
        })) } />
        <div>{ counter }</div>
      </div>
    );
  }
}

ReactDOM.render(
  <Parent />,
  document.getElementById('root')
);
<script crossorigin src="https://unpkg.com/react@16/umd/react.production.min.js"></script>
<script crossorigin src="https://unpkg.com/react-dom@16/umd/react-dom.production.min.js"></script>
<div id="root"></div>

Binding methods to this without inlining arrow functions

  1. Binding the method manually in the constructor:

    class Button extends React.Component {
      constructor(props, context) {
        super(props, context);
    
        this.cb = this.cb.bind(this);
      }
    
      cb() {
    
      }
    
      render() {
        return (
          <button onClick={ this.cb }>Click</button>
        );
      }
    }
    
  2. Binding a method using the proposal-class-fields with an arrow function. As this is a stage 3 proposal, you'll need to add the Stage 3 preset or the Class properties transform to your babel configuration.

    class Button extends React.Component {
      cb = () => { // the class property is initialized with an arrow function that binds this to the class
    
      }
    
      render() {
        return (
          <button onClick={ this.cb }>Click</button>
        );
      }
    }
    

Function Components with inner callbacks

When we create an inner function (event handler for example) inside a function component, the function will be recreated every time the component is rendered. If the function is passed as props (or via context) to a child component (Button in this case), that child will re-render as well.

Example 1 - Function Component with an inner callback:

const { memo, useState } = React;

const Button = memo(({ onClick }) => console.log('render button') || (
  <button onClick={onClick}>Click</button>
));

const Parent = () => {
  const [counter, setCounter] = useState(0);
  
  const increment = () => setCounter(counter => counter + 1); // the function is recreated all the time
  
  return (
    <div>
      <Button onClick={increment} />
      
      <div>{counter}</div>
    </div>
  );
}

ReactDOM.render(
  <Parent />,
  document.getElementById('root')
);
<script crossorigin src="https://unpkg.com/react@16/umd/react.development.js"></script>
<script crossorigin src="https://unpkg.com/react-dom@16/umd/react-dom.development.js"></script>

<div id="root"></div>

To solve this problem, we can wrap the callback with the useCallback() hook, and set the dependencies to an empty array.

Note: the useState generated function accepts an updater function, that provides the current state. In this way, we don't need to set the current state a dependency of useCallback.

Example 2 - Function Component with an inner callback wrapped with useCallback:

const { memo, useState, useCallback } = React;

const Button = memo(({ onClick }) => console.log('render button') || (
  <button onClick={onClick}>Click</button>
));

const Parent = () => {
  const [counter, setCounter] = useState(0);
  
  const increment = useCallback(() => setCounter(counter => counter + 1), []);
  
  return (
    <div>
      <Button onClick={increment} />
      
      <div>{counter}</div>
    </div>
  );
}

ReactDOM.render(
  <Parent />,
  document.getElementById('root')
);
<script crossorigin src="https://unpkg.com/react@16/umd/react.development.js"></script>
<script crossorigin src="https://unpkg.com/react-dom@16/umd/react-dom.development.js"></script>

<div id="root"></div>
Ori Drori
  • 183,571
  • 29
  • 224
  • 209
  • 3
    How do you achieve this on stateless components? – lux Apr 17 '16 at 15:04
  • 4
    Stateless (function) components don't have `this`, so there's nothing to bind. Usually the methods are supplied by a wrapper smart component. – Ori Drori Apr 17 '16 at 15:06
  • Makes sense. I have some stateless components that dispatch actions to redux, but via arrow funcs in the component, not via props passed in. Seems like I could refactor to pass in the function instead and gain some perf. Thanks. – lux Apr 17 '16 at 15:10
  • Welcome. Use react-redux and `mapDispatchToProps` in the smart component created by `connect` to create the functions you pass to props. – Ori Drori Apr 17 '16 at 15:12
  • I do actually, but come to think of it, I never just provide the callback in the `onClick`, I always bind to an arrow function out of habit, thinking it would be invoked immediately otherwise. Thanks for the tip. – lux Apr 17 '16 at 15:15
  • 55
    @OriDrori: How does that work when you need to pass data in the callback? `onClick={() => { onTodoClick(todo.id) }` – adam-beck May 12 '16 at 20:14
  • 6
    @adam-beck - add it inside the callback method definition in the class `cb() { onTodoClick(this.props.todo.id); }`. – Ori Drori May 12 '16 at 22:54
  • 1
    This is the lowest level I have and it's just a stateless component. Is this what you meant? https://gist.github.com/adam-beck/686bd0e4f495abcbbdcd0fab41f68eb6 – adam-beck May 12 '16 at 23:35
  • Use a class component, and use one of it's methods as the callback. The CB then will be able to use one of the class properties as id. – Ori Drori May 14 '16 at 20:35
  • In my test right now class instance arrow functions are not bound to `this`. – Fabian Zeindl Sep 09 '16 at 13:11
  • Indeed they are not. You should bind them manually by using one of the methods in the answer. – Ori Drori Sep 09 '16 at 13:14
  • 1
    would `constructor(props) { this.onTodoClick = () => this.props.onTodoClick(this.props.id) }` do work also? – philk Oct 24 '16 at 21:31
  • @philk - Indeed. It's equivalent to declaring Class Instance Fields. – Ori Drori Oct 25 '16 at 04:39
  • 1
    @OriDrori but is it considered bad style, or why I never see that in tutorials and examples? – philk Oct 28 '16 at 12:40
  • No idea, but maybe class properties, and bind in the constructor preserve the class format, ie methods in the class body, while the method you presented moves everything to the constructor. However, I'm just guessing. – Ori Drori Oct 28 '16 at 13:04
  • "Rerendering many elements might create jank in animations." Does this apply to animations done using JS only or also to CSS animations? – Devashish Feb 13 '19 at 12:39
  • Garbage collection might effect both types of animations as far as I know. – Ori Drori Feb 13 '19 at 13:32
  • @adam-beck - I've added a solution using hooks that addresses the passing data in the callback question. – Ori Drori Jun 24 '19 at 17:23
  • 2
    @adam-beck I think this is how to use `useCallback` with dynamic value. https://stackoverflow.com/questions/55006061/react-hooks-usecallback-with-parameters-inside-loop – Shota Tamura Aug 24 '19 at 04:13
  • You can check list of items section here: https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-no-bind.md#lists-of-items. Gives a nice example of how to proceed when there is a need to pass data – Kiran Nov 09 '20 at 11:42
  • This is a major generalization, inline functions as props are not inherently bad as Ryan Florence illustrates in this post https://medium.com/@ryanflorence/react-inline-functions-and-performance-bdff784f5578 – Steve Dec 03 '21 at 18:58
19

Using inline functions like this is perfectly fine. The linting rule is outdated.

This rule is from a time when arrow functions were not as common and people used .bind(this), which used to be slow. The performance issue has been fixed in Chrome 49.

Do pay attention that you do not pass inline functions as props to a child component.

Ryan Florence, the author of React Router, has written a great piece about this:

https://reacttraining.com/blog/react-inline-functions-and-performance/

sbaechler
  • 1,329
  • 14
  • 21
  • Can you please show how to write a unit test on components with inline arrow functions? ˆˆ – krankuba Apr 09 '19 at 15:46
  • 1
    @krankuba This is not what this question was about. You can still pass in anonymous functions that are not defined inline but are still not testable. – sbaechler Aug 20 '19 at 08:36
  • This is link https://reacttraining.com/blog/react-inline-functions-and-performance – Hussam Khatib Aug 31 '22 at 06:58
13

Why shouldn't JSX props use arrow functions or bind?

Mostly, because inline functions can break memoization of optimized components:

Traditionally, performance concerns around inline functions in React have been related to how passing new callbacks on each render breaks shouldComponentUpdate optimizations in child components. (docs)

It is less about additional function creation cost:

Performance issues with Function.prototype.bind got fixed here and arrow functions are either a native thing or are transpiled by babel to plain functions; in both cases we can assume it’s not slow. (React Training)

I believe people claiming function creation is expensive have always been misinformed (React team never said this). (Tweet)

When is the react/jsx-no-bind rule useful?

You want to ensure, that memoized components work as intended:

  • React.memo (for function components)
  • PureComponent or custom shouldComponentUpdate (for class components)

By obeying to this rule, stable function object references are passed. So above components can optimize performance by preventing re-renders, when previous props have not changed.

How to solve the ESLint error?

Classes: Define the handler as method, or class property for this binding.
Hooks: Use useCallback.

Middleground

In many cases, inline functions are very convenient to use and absolutely fine in terms of performance requirements. Unfortunately, this rule cannot be limited to only memoized component types. If you still want to use it across-the-board, you could e.g. disable it for simple DOM nodes:

rules: {
  "react/jsx-no-bind": [ "error", { "ignoreDOMComponents": true } ],
}

const Comp = () => <span onClick={() => console.log("Hello!")} />; // no warning
ford04
  • 66,267
  • 20
  • 199
  • 171
  • I'd suggest changing the wording for the second last sentence. It gives the impression that you're talking about the concept itself, not the implementation of the eslint rule. If I totally missed your point, it again proves that your intention is not clear. – sayandcode Oct 17 '22 at 12:43
12

This is because an arrow function apparently will create a new instance of the function on each render if used in a JSX property. This might create a huge strain on the garbage collector and will also hinder the browser from optimizing any "hot paths" since functions will be thrown away instead of reused.

You can see the whole explanation and some more info at https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-no-bind.md

Karl-Johan Sjögren
  • 16,544
  • 7
  • 59
  • 68
  • 1
    Not only that. Creating the new function instances each time means the state is modified and when a component's state is modified it will be re-rendered. Since one of the main reasons to use React is to only render elements that change, using `bind` or arrow functions here is shooting yourself in the foot. It is **not** well documented though, especially in the case of working with `map`ping arrays within Lists, etc. – hippietrail Sep 23 '17 at 02:07
  • "Creating the new function instances each time means the state is modified" what do you mean by that? There's no state in at all in the question – apieceofbart Jun 04 '18 at 11:30
4

To avoid creating new functions with the same arguments, you could memoize the function bind result, here is a simple utility named memobind to do it: https://github.com/supnate/memobind

supNate
  • 432
  • 3
  • 10
3

You can remove this error by wrapping the function inside useCallback.

fruitloaf
  • 1,628
  • 15
  • 10
2

For those wondering when you need to pass data in the callback. Ex.:

const list = todos.map((todo, index) => (
  <Todo
    onClick={() => { onTodoClick(todo.id, todo.title, index) }
  />
));

Solution

According to the official documentation, you should do:

  1. Move the function arguments into the children component:
const list = todos.map((todo, index) => (
  <Todo
    onClick={onTodoClick}
    todoId={todo.id}
    todoTitle={todo.title}
    indexOnList={index}
  />
));
  1. In the children component (<Todo />), pass the arguments in the call:
function Todo(props) {
  // component properties
  const { onClick, todoId, todoTitle, indexOnList } = props;

  // we move the call from the parent to the children
  const handleClick = useCallback(() => {
    onClick(todoId, todoTitle, indexOnList);
  }, [todoId, todoTitle, indexOnList]);

  return (
    <div onClick={handleClick}>
    {/* the rest of the component remains the same */}
    </div>
  );
}

Is this the best solution?

I dislike this solution. You end up with parent's data and logic in the children component. This makes the children component dependent on the parent component, breaking the dependency rule.

That's a big no-no for me.

What I do is just disable this rule. According to Ryan Florence (React Router author), this is not a big deal anyway: https://medium.com/@ryanflorence/react-inline-functions-and-performance-bdff784f5578


Daniel Loureiro
  • 4,595
  • 34
  • 48
2

The new (in beta, jan 2023) React tutorial uses both functions and arrow functions as JSX props. This hints strongly at this not being a major concern.

akafred
  • 21
  • 1
0

You can use arrow functions using react-cached-handler library, no need to be worried about re-rendering performance :

Note : Internally it caches your arrow functions by the specified key, no need to be worried about re-rendering!

render() {
    return (
        <div>
            {this.props.photos.map((photo) => (
                <Photo
                    key={photo.url}
                    onClick={this.handler(photo.url, (url) => {
                        console.log(url);
                    })}
                />
            ))}
        </div>
    );
}

Other features:

  • Named handlers
  • Handle events by arrow functions
  • Access to the key, custom arguments and the original event
  • Component rendering performace
  • Custom context for handlers
Guilherme Samuel
  • 459
  • 6
  • 11
Ghominejad
  • 1,572
  • 16
  • 15
-2

You may also see this this error if the function you are using in your onClick handler is a regular (non-inline) function defined outside of the render method but using the function keyword.

enter image description here

Declare your handler function as an arrow function using const, outside of you render method, and React will stop complaining...

enter image description here

Roger Heathcote
  • 3,091
  • 1
  • 33
  • 39
  • I don't think that's true. I get the same error message when declaring my handler as an arrow function. The way to go in this case is to use the `useCallback` hook. – Rodolphe Jun 24 '22 at 08:53
  • Well, I'm sorry but here'e your code in my IDE: https://ibb.co/7p1c4Cs And here's the error: https://ibb.co/QHqZJpr Plus many others have clearly stated that `useCallback` is the way to go. – Rodolphe Jun 25 '22 at 10:00
  • The way you did it, the `nextPage` function is recreated at every render. Which is obviously not good performance-wise. – Rodolphe Jun 25 '22 at 10:03