eslint-plugin-react: react/prefer-stateless-function false positive

Input

class Test extends React.Component {
  constructor() {
    super();
    this.dispatchThingViaRedux = () => this.props.dispatch(thing());
  }

  render() {
    return <button onClick={this.dispatchThingViaRedux}>Click me!</button>;
  }
}

This gives a lint error to prefer a stateless function. However, in this case, it is not appropriate. While you could make a stateless function and just create a dispatch function every time, the value for the button onClick handler is changed between updates (i.e. newProps.onClick. !== props.onClick). Hence DOM updates have to occur if a stateless function was used.

About this issue

  • Original URL
  • State: closed
  • Created 8 years ago
  • Comments: 29 (15 by maintainers)

Most upvoted comments

I’ve run into some false positives with classes that make use of onClick or onSubmit handlers to avoid recreating functions inside render. For example:

import React, { Component, PropTypes } from 'react';
import pureRender from 'pure-render-decorator';


@pureRender
export default class Example extends Component {
  static propTypes = {
    action: PropTypes.func.isRequired,
    id: PropTypes.number.isRequired,
  };

  handleClick = () => this.props.action(this.props.id);

  render() {
    return <button onClick={this.handleClick} />;
  }
}

While all the data is coming from props, I can’t see any way it could be rewritten as a stateless function without breaking the jsx-no-bind rule.

Following @ljharb advice’s I rewrote the rule and it should now better target the components that should be written as SFC.

The new version will ask you to write a component as a SFC if:

  • it has only displayName, propTypes, render and/or useless constructor (same detection as ESLint no-useless-constructor rule)
  • the only instance property referred to is this.props and/or this.context (destructuring is handled)
  • render() never returns anything than JSX (should cover all falsy values)

I also added much more tests.

Let me know I missed something.

@Robinnnnn

function MyComponent({ prop1, prop2 }) {
  return (
    <div>{prop1} {prop2}</div>
  );
}
MyComponent.propTypes = {
  prop1: React.PropTypes.node,
  prop2: React.PropTypes.node,
};

const mapStateToProps = ({ prop1, prop2 }) => ({
  prop1,
  prop2,
});

const mapDispatchToProps = dispatch => ({
  actions: bindActionCreators(actionCreators, dispatch),
});

export default connect(mapStateToProps, mapDispatchToProps)(MyComponent);

@Kerumen decorators are just functions so your code is equivalent to:

class Button extends Component {
  render() {
    <button styleName="button"></button>
  }
}

export default CSSModules(styles)(Button);

I’m not sure what your HOC does but technically it should be possible for Button to be a SFC like so:

function Button(props) {
  return <button styleName="button"></button>;
}

export default CSSModules(styles)(Button);

Things that should disqualify a component from being an SFC in this linter rule:

  • the presence of any instance methods
  • the presence of a non-useless constructor
  • any reference to this besides this.props or const { props } = this
  • any return null in render()

In other words, a class-based component should only be an SFC when:

  • it has solely a render method (ignoring useless constructors)
  • the only instance property referred to is this.props
  • render() never returns null (or false or undefined or return; or return '';)

Just double-checked about those stateless components and you guys are right, they aren’t pure. My world just shattered 😢

Thank you!!

@Robinnnnn It’s mapStateToProps - the stateless function receives props, not state - the store’s state is what is being mapped. You absolutely can (and should) convert MyComponent to an SFC.

@artisologic That would still re-create _handleClick every render. To quote the description of the jsx-no-bind rule:

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.

So even if the code doesn’t trigger jsx-no-bind, the performance penalty from creating functions inside render still exists.

As a side note, I like to use the no-use-before-define rule because I feel hoisting adds unnecessary complication, and breaks the top-to-bottom flow of logic in a file. Additionally, stateless components currently don’t support pure render / shouldComponentUpdate (Before now I assumed they were already pure by nature of being functional, but it appears I was wrong about that).