react/prop-types - is missing in props validation - false/positive when using inner function

[jkrawczyk]

Hi, I came across the issue with react/prop-types rule.

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

const DisplayName = (props) => {
    const getNameDiv = () => {
        return <div>{props.name}</div>;
    };

    return getNameDiv();
};

DisplayName.propTypes = {
    name: PropTypes.string.isRequired,
};

gives me warning 'name' is missing in props validation react/prop-types. Same thing happens if I rewrite function to function getNameDiv() {...}

After inlining getNameDiv there is no warning:

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

const DisplayName = (props) => {
    return <div>{props.name}</div>;
};

DisplayName.propTypes = {
    name: PropTypes.string.isRequired,
};

I was looking for similar issues but I didn't find any. Maybe #1605 is connected?

I was using eslint-plugin-react: 7.11.1 but it's the same for 7.13.3.

Thanks!

[ljharb]

In this case, getNameDiv is actually being detected as a component (which it should be; prefer components to render functions).

I'm not sure how we could rule out getNameDiv as being a component without ruling out many valid use cases.

[jkrawczyk]

This is my use case:

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

const DisplayNames = (props) => {
    const namesTable = () => {
        return <table className="table">
            <thead>
            <tr>
                <th>Name</th>
            </tr>
            </thead>
            <tbody>
            {
                props.names.map((name, index) =>
                    <tr key={index}>
                        <td>{name}</td>
                    </tr>
                )
            }
            </tbody>
        </table>;
    };

    const noNames = () => {
        return <span>No names</span>;
    };

    return props.names.length > 0 ? namesTable() : noNames();
};

DisplayNames.propTypes = {
    names: PropTypes.array.isRequired,
};

I didn't want to extract additional components because for me it was integral part of the component I was creating. Function was extracted just for better readability.

I understand that this use case may be hard to recognise. I suppose I will leave it and just disable the rule for the file.

Thank you.

[ljharb]

Certainly that’s subjective; personally I’d find it much more readable for noNames to be inline jsx, and for namesTable to be a separate component.

Let’s leave this open in case there’s a way to handle it.

[jkrawczyk]

Certainly! 😃Thanks!

[dvyue]

I am having the same issue with inner function. My use case is overriding a component in material-table. I didn't want to extract the two-liner inner function into a separate component.

[ljharb]

Why not?

[jaydunning]

Because the purpose of PropTypes is to enable proper use of a module's interface, not to enforce a module implementation structure. This limitation is particularly burdensome when working with 3rd-party components where a one-line change turns into a massive refactoring to eliminate this error.

[ljharb]

@jaydunning you only lint your own code, not third-party code; i'm not clear on why this would be burdensome.

[jaydunning]

So I had 30 minutes to correct a bug at line 209 of material-ui-treeview. It's a simple, one-file module with a permissive license, so I copied it into my code base and made the (trivial) change, then spent the next 19 minutes puzzling over how to remove this lint error before I disabled the rule for the module, then 5 minutes on the comment above. I'm referencing the specific file because I think it may provide a good concrete example of what I meant by "burdensome". The location responsible for the errors (line 152) is a recursive function within a component; the function itself behaving as a component but needing its closure, and I couldn't think of a quick way to refactor it to satisfy this rule. Further, it seemed to me that the design was suited to purpose, and any refactoring to eliminate the rule error would degrade the module's design. This may be an exception case, and perhaps disabling the rule is the expected remedy, and I BTW agree that OP's example is not ideal and think that detection of the anti-pattern is a good thing, but I also believe there's a critical distinction between interface and implementation, and that the prop-types rule should be about interface, not implementation, because PropTypes is about interface, not implementation.

[ackvf]

Originally from https://stackoverflow.com/a/60510524/985454

I have discovered a strange behaviour. The only difference is in the return statement:

Now also the bar in propTypes is uncommented:

I think the problem is because the myFunc looks like a functional component and that eslint is picking it up wrongly.

Also, this is rather funny, if you simply rename the props to anything else, eslint goes silent :)

[ackvf]

@ljharb React itself differentiates that myFunc is a function and MyFunc is a component, so why not assume the same?

myFunc can't be used as a component anyway, this code is invalid <myFunc />.

[ljharb]

@ackvf react does runtime introspection; a linter can not.

[ackvf]

@ljharb myFunc vs MyFunc is known at compile time or am I missing something?