jsx-eslint / eslint-plugin-react

React-specific linting rules for ESLint
MIT License
8.92k stars 2.76k forks source link

(react props) is missing in props validation ERROR #2777

Open S-Amin opened 4 years ago

S-Amin commented 4 years ago

I just tested the version 7.20.6 and I got the error ... is missing in props validation. It's a Nextjs project and I want to add the linting now but I got the error on my components this is my package dependency

        "eslint": "^7.7.0",
        "eslint-config-airbnb-typescript": "^9.0.0",
        "eslint-config-prettier": "^6.11.0",
        "eslint-plugin-import": "^2.22.0",
        "eslint-plugin-jsx-a11y": "^6.3.1",
        "eslint-plugin-prettier": "^3.1.4",
        "eslint-plugin-react": "^7.20.6",
        "eslint-plugin-react-hooks": "^4.0.8",

this is the eslint consfig file


{
    "env": {
        "browser": true,
        "es2020": true
    },
    "extends": [
        "airbnb-typescript",
        // "eslint:recommended",
        // "plugin:react/recommended",
        "plugin:@typescript-eslint/recommended",
        "prettier",
        "prettier/react",
        "prettier/@typescript-eslint",
        "plugin:prettier/recommended"
    ],
    "parser": "@typescript-eslint/parser",
    "parserOptions": {
        "project": "./tsconfig.json",
        "ecmaFeatures": {
            "jsx": true
        },
        "ecmaVersion": 12,
        "sourceType": "module"
    },
    "plugins": ["react", "@typescript-eslint"],
    "rules": {
        "prettier/prettier": ["error"],
        "react/react-in-jsx-scope": "off",
        "import/prefer-default-export": "warn",
        "no-underscore-dangle": "off",
        "no-nested-ternary": "off",
        "@typescript-eslint/no-empty-interface": "warn",
        "@typescript-eslint/naming-convention": [
            "error",
            {
                "selector": "interface",
                "format": ["camelCase", "UPPER_CASE", "PascalCase"]
            }
        ]
    }
}

and there are multiple components that eslint threw this error ... is missing in props validation

ljharb commented 4 years ago

Please provide exact component code that's being warned on, and the exact text of the warnings.

What's likely happening is that a bunch of missing propTypes are being properly detected now that component detection has been improved to catch a new class of cases.

S-Amin commented 4 years ago

One of the components is this:

interface Props {
    posts: string[];
}
const Home: NextPage<Props> = ({ posts }) => {
    function onChange(date, dateString) {
        console.log(date, dateString);
    }

    function check() {}

    return (
        <div style={{ marginTop: 100 }}>
            <Form layout="horizontal">
                <DatePicker onChange={onChange} picker="week" />
                <Button onClick={check}>check Token</Button>
                <FormItem
                    style={{ marginTop: 48 }}
                    wrapperCol={{ span: 8, offset: 8 }}
                >
                    <Button size="large" type="primary">
                        +
                    </Button>
                    <Button size="large">-</Button>
                </FormItem>
            </Form>
            <Card
                title="Default size card"
                extra={<a href="/">More</a>}
                className={sass.test}
            >
                {posts.map((item, i) => (
                    <p key={i}>{item}</p>
                ))}
                <p>Card content</p>
                <p>Card content</p>
            </Card>
        </div>
    );
};

export async function getServerSideProps(ctx: NextPageContext) {
    return { props: { posts: ["test", "test2"] } };
}

export default Home;

and the error is :

 14:34  error    'posts' is missing in props validation       react/prop-types
 43:24  error    'posts.map' is missing in props validation   react/prop-types
ljharb commented 4 years ago

what's NextPage? React components usually aren't typed like that.

S-Amin commented 4 years ago

this is a page in nextjs and the same error appear in my components too. the problem is that eslint can't understand the prop interface when I'm using arrow function.

ljharb commented 4 years ago

Ah. That's a limitation in the TS parser; the type is associated with the variable and not with the arrow function

Components shouldn't be arrow functions anyways - make them proper named functions, and the types will work properly too.

S-Amin commented 4 years ago

I can't see why I shouldn't use arrow functions. just because a bug in TS or a package?

ljharb commented 4 years ago

In general, because arrow functions don't have explicit names, it makes them worse for debuggability. That the TS eslint parser can't properly associate type information to them is just another reason on top of the regular JS one.

S-Amin commented 4 years ago

So is there any plan to fix this bug or not?

ljharb commented 4 years ago

I'd like it fixed, despite the arrow function advice, but I believe the issue is in the typescript eslint parser. @bradzacher, can you confirm?

iow, if there's something we can do here to associate the right type information with arrow function components, I'm happy to make the change here, but I think to be practical the parser would have to attach it to the arrow function AST node.

bradzacher commented 4 years ago

In general, because arrow functions don't have explicit names, it makes them worse for debuggability

JS Engines (at least chrome..) actually do implicit naming for arrow functions based on the name of the variable: image

So I don't think there's a difference between function decls and arrow functions in this regard.


but I believe the issue is in the typescript eslint parser

In this case, no - the AST is working as expected: the type annotation is on the variable declaration, not the function argument. You will get roughly the same AST shape with babel parsing TS, or any flow parser as well.

This plugin would have to traverse the parent in order to find the annotation, and then do some handling of the type to find the prop type.

Common cases it could handle:

The "handling" would essentially be: 1) annotation = id.typeAnnotation.typeAnnotation 2) if (annotation.type !== 'TSTypeReference' && annotation.typeParameters == null) return; (not a generic type with type params) 3) if (!allowedTypes.has(context.getSourceCode().getText(annotation.typeName).replace(/ /g, ''))) return (note an allowed generic type) 4) follow the reference from the generic parameter to the props def. 5) validate prop type against prop def

ljharb commented 4 years ago

@bradzacher yes, the spec has inferred names, but they don't always apply intuitively (so it's easy for devs to think they're inferred when they're not) and when debugging in browsers that lack name inference, the name might not show up at all.


In that case it seems like a flaw in TS itself :-/ it seems like we need a helper in our component detection that can get the type (when it doesn't exist on the component, only on the variable that holds it). PRs welcome.

bradzacher commented 4 years ago

In that case it seems like a flaw in TS itself

For function expressions (arrow or otherwise) assigned to a variable declaration, parameter declaration, returned from a function or assigned to a property, both flow and TS will infer function parameter types for you based on the contextual type (i.e. the annotated type of the variable, parameter, function signature return, or object property, respectively).

This inference greatly simplifies DevX without sacrificing any readability or usability. It does however mean analysis tools have to take on more burden to analyse the code.

Note that the @typescript-eslint tooling will include some limited type information here. But depending on the user's setup it might be unreliable.

We provide two flavours:

I figured this plugin wouldn't want to add a type-aware lint rule (as it restricts the rule's usability a bit), so to make this work you'd have to do this entirely based on the AST analysis.

ljharb commented 4 years ago

I guess I'd expect inferred types to show up directly on the nodes they're inferred on :-/

I agree that we wouldn't want the "complete project types" option by default. However, I think we'd always want to incur same-file cost for figuring out the type of a component, specifically (as opposed to everything else).

kaiyoma commented 3 years ago

Also just started running into this issue when upgrading from 7.20.6 to 7.21.4. "Don't use arrow functions for components" isn't a reasonable request - mainly because types require it - so we'll also be waiting for a proper fix for this. Happy to stay on 7.20.6 until that happens.

soullivaneuh commented 3 years ago

Agreed with @kaiyoma, I don't think arrow function is the issue here.

However, I found an another way to make this plugin not yelling:

Instead of:

const ButtonWrapper: FunctionComponent<ButtonWrapperProps> = ({ title, variant }): ReactElement => {}

I wrote:

const ButtonWrapper = ({ title, variant }: PropsWithChildren<ButtonWrapperProps>): ReactElement => {

Not sure it's the best practice, but it give me the same auto-completion results.

You may also keep both:

const ButtonWrapper: FunctionComponent<ButtonWrapperProps> = ({ title, variant }: PropsWithChildren<ButtonWrapperProps>): ReactElement => {

Works as well, but maybe a bit overkill.

EDIT: I understand now why FunctionComponent is prefered. It's avoid me to write the return type of my function. Example:

const CenterView: FunctionComponent<{}> = ({ children }) => <View style={styles.main}>{children}</View>;
IronSean commented 3 years ago

Is this issue related to why I can't use

const MyComponent: React.FC<{myProp: string}> = ({myProp}) => {
  return <p>{myProp}</p>
}

Without getting the same errors? I'm tempted to just disable this rule because with typescript the type checking validates the props anyway.

kaiyoma commented 3 years ago

I was also running into this issue for a while and actually ran an old version of eslint-plugin-react for several months to avoid the problems with upgrading. Today, on a whim, I tried upgrading all my eslint-related packages to their latest versions and this problem is gone. Not only that, but eslint is now spotting this particular error (correctly) in a few more places! I guess it got fixed somewhere along the line? 🤷‍♂️

ljharb commented 3 years ago

@kaiyoma it's possible v7.22.0 fixed it - you can try downgrading to v7.21 and see if the problem recurs, then we could close this with confidence :-)

ljharb commented 3 years ago

I'll close this now, but reopen if that's not the case. cc @S-Amin to confirm.

andreidmt commented 3 years ago

The issue is still present using "eslint-plugin-react": "^7.23.1"

focus_2021-April-07-Wednesday-16:36:31

ljharb commented 3 years ago

This still isn’t fixed for arrow functions and typescript, because we haven’t added the code required to connect the variable type to the untyped arrow function. The better fix is to change it to a normal function in your code; but I’ll reopen.

kaiyoma commented 3 years ago

The better fix is to change it to a normal function in your code; but I’ll reopen.

This is not always an option since function types (like FC in the screenshot above) are sometimes used. We have similar types in our codebase too, so our function components must be defined with const and fat arrow.

rafaelgrilli92 commented 3 years ago

it didn't use to complain before, but yea, now it only works if you create it as a normal function instead of an arrow function, which is really annoying as the entire project was written using arrow function for functional components.

joshmedeski commented 2 years ago

So the react/prop-types rule is still failing when I define my components this way:

const Component: React.FC<{value: string}> = ({value}) => {return <span>{value}</span>}

I see 'value' is missing in props validation.

Is this syntax going to be supported by this plugin any time soon? What can the community do to help?

ljharb commented 2 years ago

@joshmedeski https://github.com/yannickcr/eslint-plugin-react/issues/2777#issuecomment-683944481 someone's welcome to make a PR implementing this rough plan.

janpe commented 7 months ago

Here's something I noticed. The type of these components is identical but the plugin fails to recognize the latter two ways of using the type

image
ljharb commented 5 months ago

@janpe eslint uses static analysis, so unless the typescript eslint parser can provide the type information (and, we know it's there and use it), we don't know the types.