Eslint-plugin-react: Returning A Function Fails PropTypes

[blainemuri]

Version

7.12.1 - 7.12.4 (haven't checked earlier versions)

Problem

Component where the return method calls a function instead of directly returning a jsx component will fail proptypes validation, even if the proptypes are properly declared.

Example

Broken

const SomeComponent = (props) => {
  function renderComponent() {
    return <div>{props.name}</div>
  }

  return renderComponent();
}

SomeComponent.propTypes = {
  name: PropTypes.string
}

The following error will occur:
'name' is missing in props validation react/prop-types

Working Example
No errors occur in the following case

const SomeComponent = (props) => {
  function renderName() {
    return <div>{props.name}</div>
  }

  return (
    <div>{renderName()}</div>
  );
}

SomeComponent.propTypes = {
  name: PropTypes.string
}

[ljharb]

renderComponent here effectively is a component, and so it should have propTypes.

Either way, instead of defining it on every render, you should define it outside of SomeComponent, and pass name into it as an argument.

[blainemuri]

By that definition, any function that returns jsx can be considered a "component" technically, but abstracting them out of the actual component doesn't necessarily solve the problem.

Props are inherently defined in the parent class/file and thus the context would never change going to the children on what the props will be (children in this case being just the function). Additionally the props should be scoped to the rendered React element, which in this case is SomeComponent. renderComponent is not a react element. Expecting someone to double define proptypes in this case is a bit much.

The better solution is to exclude this case, or at a minimum give better logging as to where the proptypes need to be defined. Currently the error message will say that SomeComponent is missing propTypes, but it is not.

[ljharb]

Yes, it's (or should be) either a render prop or a component if it returns jsx.

Passing around a props object is a massive antipattern; using it via closure is the same.

I think if you insist on using this style, you'll have to use an override comment to disable the rule.

[blainemuri]

Then at a minimum the logging should be fixed. eslint-plugin-react, if it's assuming that it's a new component there, should log accordingly. It should not say that SomeComponent is missing proptypes, that's just incorrect

[blainemuri]

This isn't an issue of whether or not the coding style is correct, it's an issue of me having to dig into why the the plugin was pointing out missing props in the logging in a place where props weren't missing

[ljharb]

I agree that there's a bug here; the plugin should be able to figure out that renderComponent isn't using the props argument - ie, the warning i'd expect here is that SomeComponent isn't using the "name" prop (as opposed to the one you're getting).

[ljharb]

cc @alexzherdev for props detection

[blainemuri]

The curious thing is why this issue would happen only when returning a function directly after the render. One would think that this error would also occur when a nested render is called (like in my second example) if it considers anything returning jsx to hold its own proptypes

[jzabala]

@ljharb this one will also be solved be #2699. Do you want me to add it to the list or would you prefer to use this one to improve the log message to include the "SomeComponent" as stated in some comments: https://github.com/yannickcr/eslint-plugin-react/issues/2196#issuecomment-473400981 https://github.com/yannickcr/eslint-plugin-react/issues/2196#issuecomment-473410079?

[ljharb]

@jzabala glad to hear that it'll be solved; would it be possible to improve the log message in #2699 as well?

[jzabala]

@jzabala glad to hear that it'll be solved; would it be possible to improve the log message in #2699 as well?

I could but I think it would be better to use another issue for that since the two tasks wouldn't have anything in common.