[BUG] react/prop-types - false positive in functional components

[andrmoel]

When I use props inside a function of a functional component. The eslint PropTypes validation fails. Which is wrong.

My eslintrc.json:

{
  "settings": {
    "react": {
      "createClass": "createReactClass",
      "pragma": "React",
      "version": "16"
    }
  },
  "parser": "babel-eslint",
  "parserOptions": {
    "ecmaVersion": 6,
    "sourceType": "module",
    "ecmaFeatures": {
      "jsx": true
    }
  },
  "extends": [
    "plugin:react/recommended"
  ],
  "plugins": [
    "babel"
  ],
  "rules": {
    "react/prop-types": ["error"]
  }
}

Example with bug

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

const MyComponent = (props) => {
    function render() {
        return <test>{props.text}</test>;
    }

    return render();
};

MyComponent.propTypes = {
    text: PropTypes.string.isRequired,
};

export default MyComponent;

Error messge: 6:29 error 'text' is missing in props validation react/prop-types

---

If I don't use a function inside my functional component. The eslint rule validates my code correctly.

Working example:

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

const MyComponent = (props) => {
    return <test>{props.text}</test>;
};

MyComponent.propTypes = {
    text: PropTypes.string.isRequired,
};

export default MyComponent;

[ljharb]

Why would you use a function like that? render, in this case, is a component. Defining it newly each time inside your component is wasteful.

[andrmoel]

@ljharb Is this information important for the given bug?

There are many reasons why you should structure your code into small functions. Using the Single Responsibility Principle or Top-Down design is state of the art nowadays.

const MyComponent = (props) => {
    const render = () => (
        <div>
            {props.text === 'A' ? _renderA() : _renderB()}
        </div>
    );

    const _renderA = () => <A/>;

    const _renderB = () => <B/>;

    return render();
};

(Bug occurs in this code also)

[ljharb]

It's a bad idea to pass the props object around anywhere, so even if you wanted to do that, you'd always destructure props directly in the component, and pass them to the function you wanted.

If you do that, the linter will work correctly as well.

There's nothing about that code that's "state of the art", imo - render should take a text argument, and all three of those functions should be defined outside of MyComponent so you're not creating them anew on every render.

[tusharshuklaa]

What if I want to break my component into smaller components within same file and then call them as needed. For Example:

// is missing in props validation eslint(react/prop-types)
const  MyComp1 = ({ title, description }) => {
  return (
    <h4>{title}</h4>
    <p>{description}</p>
  )
};

// is missing in props validation eslint(react/prop-types)
const  MyComp2 = ({ name, address }) => {
  return (
    <h4>{name}</h4>
    <p>{address}</p>
  )
};

// No error in this case
function MyComponent ({isAddress, name, address, title, description}) {
    const CompToShow = isAddress ? <MyComp2 name={name} address={address} /> : <MyComp1 title={title} address={description} />
    
    return(<CompToShow />);
};

MyComponent.propTypes = PropTypes.shape({
    isAddress: PropTypes.string,
    name: PropTypes.string,
    address: PropTypes.string,
    title: PropTypes.string,
    description: PropTypes.string,
});

MyComp1.propTypes = PropTypes.shape({
    title: PropTypes.string,
    description: PropTypes.string,
});

MyComp2.propTypes = PropTypes.shape({
    name: PropTypes.string,
    address: PropTypes.string,
});

Now let's just skip the code quality thing for a moment but the above example gives me false error as well.

[ljharb]

The propTypes object must not be a shape, it must be an object literal. Remove the PropTypes.shape wrapper from all three of those, and a) your propTypes won't be broken, and b) the lint rule that's just saved you from your broken code will be satisfied.

[golopot]

This bug is similar to #2135, #2171, #2352, #2196. Our plugin think props.text is an undeclared prop usage in the inner component render, but ideally the plugin should note that the variable props comes from the outer component, therefore it is a prop usage in the outer component.

[jzabala]

With the improvement in function component detection proposed in PR #2699, this error is gone since the render function is not recognized as a component anymore. The problem that I see is that not now or with the PR MyComponent will be recognized as a component since it doesn't return JSX or null. For example this is ok:

const MyComponent = (props) => {
  function render() {
    return <test>{props.hello}</test>;
  }
  console.log(props.hello) // no error here since MyComponent is not recognized as a component
  return render();
};
MyComponent.propTypes = {
  text: PropTypes.string.isRequired,
};

[ljharb]

That's great to hear.

I'm not too worried about this edge case - that pattern seems nonsensical to me.