prefer-stateless with event handlers is problematic

[kesne]

I noticed with the latest version of the config that I get a lot of prefer-stateless warnings on components that just use props and event handlers. However, this is hard to do when the react/jsx-no-bind rule is enabled, which stops you from making functions for event handlers in the render path.

For example, here's a component that only uses props and an event handler that the linter will tell me to turn into a stateless component.

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

    this.handleClicked = this.handleClicked.bind(this);
  }

  handleClicked() {
    this.props.onClicked(this.props.name);
  }

  render() {
    return (
      <button onClick={this.handleClicked}>Click Me</button>
    );
  }
}

However, in the attempt to do so, you need to create a function in the render path, which causes the react/jsx-no-bind lint rule to blow up on you.

function MyComponent({ onClicked, name }) {
  return (
    <div onClick={() => onClicked(name)}>Blow Me Up</div>
  );
}

You can work around this by defining the function outside of the JSX, but at that point you're just working around the linter.

[ljharb]

This is a current bug in eslint-plugin-react - components with instance methods should not be made stateless, because they have state - this is their state.

For now, I'd just disable the rule.

[ljharb]

This should be fixed now in eslint-plugin-react. The next release of the linter config will definitely include it (if the current one doesn't already)

[barrymichaeldoyle]

You could have set it up like this:

const blowMeUp = (onClicked, name) => () => onClicked(name);

function MyComponent({ onClicked, name }) {
  return (
    <div onClick={blowMeUp(onClicked, name)}>Blow Me Up</div>
  );
}

[ljharb]

@barrymichaeldoyle while that's fine for that use case (passing it into a div), for a custom component that would be bad because it'd create a new function every render - the only option there is a class-based component.

[barrymichaeldoyle]

Ah right, good point.

[obouchari]

@ljharb I'm new to react, still trying to learn the ins and outs of react. Could you elaborate on your response with an example? That will be greatly appreciated :) thanks

[ljharb]

@obouchari you can see in the example here that every render - every time MyComponent gets called - blowMeUp will be called, and will create a brand new function, that will be !== to the one passed on the previous render. For HTML elements like div, it doesn't matter, but if instead of a div it was a class Foo extends React.PureComponent, then the PureComponent optimization (ie, not rerendering when the props are all === from the previous render) would be bypassed, and you'd get an unnecessary rerender. These rerenders can add up to big performance problems on a larger page.

[obouchari]

Thanks for the awesome explanation. Correct me if I'm wrong. So in the case of using a custom child component. Every time MyComponent rerenders the child component will rerender as a result since the returned function will be different every time. And that will cause a performance issue? What about creating a brand new function every time we render MyComponent, isn't that a problem as well? Just curious 😄

[ljharb]

@obouchari if you create it on every render then that’s a problem; but the constructor of a class component isn’t called on every render.

[obouchari]

Got you! Thanks, that was really helpful.