-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
(react props) is missing in props validation ERROR #2777
Comments
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. |
One of the components is this:
and the error is :
|
what's |
this is a page in |
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. |
I can't see why I shouldn't use arrow functions. just because a bug in TS or a package? |
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. |
So is there any plan to fix this bug or not? |
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 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. |
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. |
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). |
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. |
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 const CenterView: FunctionComponent<{}> = ({ children }) => <View style={styles.main}>{children}</View>; |
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. |
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? 🤷♂️ |
@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 :-) |
I'll close this now, but reopen if that's not the case. cc @S-Amin to confirm. |
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. |
This is not always an option since function types (like |
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. |
So the const Component: React.FC<{value: string}> = ({value}) => {return <span>{value}</span>} I see Is this syntax going to be supported by this plugin any time soon? What can the community do to help? |
@joshmedeski #2777 (comment) someone's welcome to make a PR implementing this rough plan. |
@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. |
What am I doing wrong? how to fix the eslint violation? I tried to turn
|
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 componentsthis is my package dependency
this is the
eslint
consfig fileand there are multiple components that
eslint
threw this error... is missing in props validation
The text was updated successfully, but these errors were encountered: