react/jsx-no-bind false positive?

[bcutler-work]

Example snippet:

import React from 'react';
import PropTypes from 'prop-types';

class FalsePositive extends React.PureComponent {
  static propTypes = {
    list: PropTypes.arrayOf(PropTypes.node).isRequired,
  };

  render() {
    return (
      <div>
        {this.props.list.map(this.wrap.bind(this, 'span'))}
      </div>
    );
  }

  wrap(Tag, item, i) {
    return <Tag key={i}>{item}</Tag>;
  }
}

export default FalsePositive;

this.wrap.bind triggers the rule, even though the bound function isn't the actual prop being passed, but is being used during the render to build the children, so it shouldn't break purity.

[ljharb]

Indeed, that should not complain because it's not passing the new function as a prop anywhere.

[jackyho112]

I will give this one a look. 😄

[jackyho112]

@ljharb

Seems like the rule needs a bit more fixing than I thought it would to only check for bind calls in JSX props. If that was its purpose, this test and this one should not warn but they are currently invalid tests.

The current logic in the rule checks for all bind calls in all JSX methods and warn each of them. And along with the documentation in that file, I am given the impression that this rule does check for bind calls in a JSX method instead of only those in JSX attributes.

I understand that based on the rule doc, it should only check for JSX props. However, based on the purpose of such checks, wouldn't it make sense to check for all bind calls in a method that returns JSX? Because all of those functions will be created again on every render like the case mentioned by the issue opener and that is the undesirable outcome the rule is trying to prevent.

This is just my 2 cents. If we have to write the rule to fulfill exactly its purpose as documented, I will probably have to refactor a portion of its logic, have some new logic to deal with aliases like what no-unused-state is doing, get rid of wrong tests and add new tests.

If my opinion seems reasonable, we would only have to improve the docs.

Let me know your thoughts. 😄

[ljharb]

The rule is to avoid new functions being created and passed as props to an element (it should really apply not just to jsx, but to createElement calls as well).

[ljharb]

It's hard to evaluate without an actual PR; I'd love to look at the test case and docs changes in particular that you're suggesting.

[jackyho112]

@ljharb

Thanks for replying! I think I know the direction I'm going with to fix this issue now. Will open a PR soon. 😄