Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: react/prop-types false positive when component is a value of capitalized property in class #3534

Open
2 tasks done
Maxim-Mazurok opened this issue Feb 21, 2023 · 9 comments

Comments

@Maxim-Mazurok
Copy link

Maxim-Mazurok commented Feb 21, 2023

Is there an existing issue for this?

  • I have searched the existing issues and my issue is unique
  • My issue appears in the command-line and not only in the text editor

Description Overview

Weird error:
image

import React from 'react';

class SomeComponent {
	column = {
		Cell: ({ original }: { original: string }) => <span>{original === 'bla' ? 'bla' : 'bla'}</span>,
	};
}
error    'original' is missing in props validation  react/prop-types

eslint myfile.tsx

Expected Behavior

Not to throw error

When I change Cell to cell - the error is gone.
When I avoid using destructuring - the error is also gone.

eslint-plugin-react version

v7.32.2

eslint version

v8.34.0

node version

v18.14.1

@Maxim-Mazurok Maxim-Mazurok changed the title [Bug]: react/prop-types false positive when component is a value of capitalized property [Bug]: react/prop-types false positive when component is a value of capitalized property in class Feb 21, 2023
@ljharb
Copy link
Member

ljharb commented Feb 21, 2023

Cell here is a function, taking props, that renders JSX, and doesn't have propTypes. Why is it named with a component naming convention if it's meant to be a render function?

@Maxim-Mazurok
Copy link
Author

The reason it's named Cell is because it later passed to react-table@6 as part of the columns prop.

Here's a more complete example of how we use it:

import React, { Component } from 'react';
import ReactTable from 'react-table';

export default class MyTable extends Component {
	columns = [
		{
			Cell: ({ original }: { original: string }) => <span>{original === 'bla' ? 'bla' : 'bla'}</span>,
		},
	];
	render(): JSX.Element {
		return <ReactTable columns={this.columns} />;
	}
}

Note that we're using v6 of react-table which has a very different API from the latest version and no bandwidth to migrate.

@ljharb
Copy link
Member

ljharb commented Feb 23, 2023

and ReactTable requires it to be named "Cell"? is that because it uses it like a component?

@Maxim-Mazurok
Copy link
Author

and ReactTable requires it to be named "Cell"?

Yes, see:

Cell: JSX | String | Function // Used to render a standard cell, defaults to the accessed value

from docs
image

is that because it uses it like a component?

I think so...

@ljharb
Copy link
Member

ljharb commented Feb 25, 2023

right - so, it's a component. and components should have propTypes.

Is there a reason you can't do:

import React from 'react';

class SomeComponent {
	column = {
		Cell({ original }: { original: string }) { return <span>{original === 'bla' ? 'bla' : 'bla'}</span>; },
	};
}

?

Components shouldn't be arrow functions, and I think that should be recognized by the prop-types rule as having prop types.

(even better is to define it outside of SomeComponent and reuse it)

@Maxim-Mazurok
Copy link
Author

Maxim-Mazurok commented Feb 26, 2023

Is there a reason you can't do:

import React from 'react';

class SomeComponent {
	column = {
		Cell({ original }: { original: string }) { return <span>{original === 'bla' ? 'bla' : 'bla'}</span>; },
	};
}

?

Your example doesn't give errors, but if I add class SomeComponent extends Component - then it produces the same error:
image
error 'original' is missing in props validation react/prop-types

Components shouldn't be arrow functions

Surely this isn't a technical limitation (because our code works just fine), it's a stylistic preference, right?

@Maxim-Mazurok
Copy link
Author

Hi, @ljharb just curious if you had time to look into this or have any thoughts.

@ljharb
Copy link
Member

ljharb commented Mar 6, 2023

I haven't had time to look into it.

Yes, it's a stylistic preference, but it's more than just that, because an arrow function can't ever have an explicit name, and name inference isn't always intuitive.

@JMartinCollins
Copy link

JMartinCollins commented Mar 16, 2023

I think I've experienced a similar issue. In my situation I have an HOC:

const ModifiedComponent = withModifiedBehavior(Component, options);

The options object has the following type signature:

interface WithModifiedBehaviorOptions {
    // ...
    ModifiedBehaviorComponent?: React.ComponentType<{propWhichWillBePassed: () => void}>;
    // ...
}

options can take a property whose value is a React Component which will be rendered as part of the HOC modified behavior.

const ModifiedComponent = withModifiedBehavior(Component, {
    ModifiedBehaviorComponent ({ propWhichWillBePassed }) {
        return (<>
            <div>Custom Text</div>
            <button 
                onClick={
                    // custom logic
                    propWhichWillBePassed()
                } 
            />
        </>)
    }
});

In this case propWhichWillBePassed has a squiggly line saying 'propWhichWillBePassed' is missing in props validationeslint[react/prop-types](https://github.com/jsx-eslint/eslint-plugin-react/tree/master/docs/rules/prop-types.md)

The TS validation works perfectly fine here and it's able to infer the type and give all the necessary intellisense. I've tried declaring the ModifiedBehaviorComponent in the options object as a named function, like above, and also as arrow fn like this: ModifiedBehaviorComponent: ({ propWhichWillBePassed }) => { /* ... */ }

In this case I'm not sure if there's a method for adding prop types to a Function Component provided as an object property. Obviously I could export the type WithModifiedBehaviorOptionsComponentProps and use it every time the option is used, but that seems redundant when TS can figure out what it should be already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants