Eslint-plugin-react: Missing prop validation in React.FunctionComponent

[dchebakov]

Tell us about your environment

• ESLint Version: v6.0.1
• Node Version: v10.16.0
• npm Version: v6.9.0

Configuration

module.exports = {
  parser: '@typescript-eslint/parser',
  parserOptions: {
    project: './tsconfig.json',
  },
  plugins: ['@typescript-eslint'],
  extends: [
    'plugin:react/recommended',
    'plugin:@typescript-eslint/recommended',
  ],
}

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

import * as React from 'react';
interface PersonProps {
    username: string;
}
const Person: React.FunctionComponent<PersonProps> = (props): React.ReactElement => (
    <div>{props.username}</div>
);

npx eslint src --ext .tsx

What did you expect to happen?
eslint show no errors, because type definition actually present in generic type React.FunctionComponent<PersonProps>

What actually happened? Please include the actual, raw output from ESLint.

error    'username' is missing in props validation       react/prop-types

[ljharb]

Technically you typed the variable and not the function; what happens if you use a normal non-arrow function for your component?

[dchebakov]

Technically you typed the variable and not the function; what happens if you use a normal non-arrow function for your component?

For non-arror function component:

function Person(props: PersonProps): React.ReactElement {
    return (
        <div>{props.username}</div>
    )
}

there are no errors, but I want to use arrow func with generic type FunctionComponent. For that case, type of props should be inferred from React.FunctionComponent<PersonProps>

[ljharb]

That’s something typescript does, but unless the typescript eslint parser passes those types to this plugin, we have no way of leveraging them.

It’s a better practice to use non arrows for SFCs anyways, so they have a proper reliable name for debugging.

[dchebakov]

Should I create an issue for typescript parser?

[ljharb]

I’m not sure. You could certainly try a PR here to improve the type detection - before filing an issue even on the typescript parser you’d need to have tested and figured out what was needed.

The simplest fix, though, is to follow what the most popular styleguide recommends and use normal functions for components.

[tx-steven]

I'm having this same error but with functional components. Can't seem to find any good reason why it's happening. I have a functional component. With a typescript props interface. When I access a property off of the prop. I get <property> is missing in types validation.

[ljharb]

@tx-steven is it an arrow function or a normal function?

[skube]

Adding : PersonProps (somewhat redundantly) seems to work...

const Person: React.FunctionComponent<PersonProps> = (props: PersonProps): React.ReactElement => (
    <div>{props.username}</div>
);

[ljharb]

That’s not redundant; in your case the first one is typing the variable, not the function.

[skube]

I guess but I thought TypeScript infers the types — suggesting that it's not required to do both left and right operands.

[shulcsm]

It does, this rule is redundant for arrowfunc: React.FC<> case since props are checked by TS

[ljharb]

TypeScript may, but if the eslint typescript parser doesn’t, there’s nothing this plugin can do.

[Rey-Wang]

@skube it works

[aaronlna]

@skube A small formality, but the inferred type is more specifically React.PropsWithChildren<PersonProps>. Although if you aren't using any of its (PropsWithChildren) attributes, PersonProps would be sufficient.

If this isn't an eslint issue would creating an option to ignore this case be worthwhile?

[kristojorg]

This is quite annoying, and I don't believe there is any value in using non arrow functions for function components.

[ljharb]

it has lots of value; explicitly that you aren’t relying on name inference, which is unintuitive.

[mohsinulhaq]

It’s a better practice to use non arrows for SFCs anyways, so they have a proper reliable name for debugging.

One advantage of using arrow function is the children typing you get for free. For example:

interface CompProps {
    prop1: number
}

const Comp: React.FC<CompProps> = ({ children, prop1 }) => {...} // children inferred from React.FC

function Comp({ children, prop1 }: CompProps) {...} // where to put React.FC?

ref: TypeScript#16918

It works in TSLint but not in ESLint.

[skube]

function Comp({ prop1 }: CompProps) {...} // where to put React.FC?

Can't you do? :

function Comp({ prop1 }: CompProps): React.ReactElement {...} 

[mohsinulhaq]

function Comp({ prop1 }: CompProps) {...} // where to put React.FC?

Can't you do? :

function Comp({ prop1 }: CompProps): React.ReactElement {...} 

Sorry I missed destructuring children in the second component. Added it now. After that, you face the same problem.

[xjamundx]

I found this to be an interesting case

// fails with prop-types
export const BlockBad: React.FC<BlockProps> = ({ attributeName }) => {
  return <div data-attributeName={attributeName} />;
};

// does not fail with prop-types
export const BlockGood: React.FC<BlockProps> = ({ attributeName }) => {
  if (attributeName) {
    return <div data-attributeName={attributeName} />;
  }
};

One workaround is to double-type your components, but it's definitely not behaving consistently as-is

export const BlockBad: React.FC<BlockProps> = ({ attributeName }: BlockProps) => {

For those seeking ideas about how to type these things. I found both of these resources helpful:

• https://github.com/typescript-cheatsheets/react-typescript-cheatsheet#function-components
• https://github.com/piotrwitek/react-redux-typescript-guide/blob/master/README.md#function-components---fc

Basically one suggests to use function and the other suggests the approach being used by a lot of people in this thread const val: React.FC = () => {}

[leonardorb]

@xjamundx very nice. This will work without warnings with the latest versions:

import React from 'react'

type CellProps = {
  value: string
}

const Cell: React.FunctionComponent<CellProps> = ({ value }: CellProps) => {
  return <div className="item">{value}</div>
}

export default Cell

[saranshkataria]

@leonardorb that will work, but as @xjamundx pointed out, you are typing it twice. It should have been inferred automatically instead.

Another potential way of doing it would be:

const Cell = ({ value }: CellProps): JSX.Element => {
  return <div className="item">{value}</div>
}

But not sure what the ideal statement should be.

[AlaaZorkane]

Any updates on this? Or at least a best practice, I don't see how typing twice is acceptable in this case...

[saranshkataria]

I have manually turned off the react/prop-types rule in my config for now. Not sure if there is a better way to handle it.

[tx-steven]

Best practice is to just not use arrow functions so I don't think you guys are going to find an easy/simple work around for this if you want to use arrow functions for react components

[TeoTN]

I think the rule is pretty useless in the current shape - it's supposed to check if props are typed, not to enforce whether the project should use arrow functions or not. It's not in scope of its responsibility.

[ljharb]

@TeoTN unfortunately by using arrow syntax it seems TS eslint parsers are incapable of attaching the type to an arrow function, so here we are :-/ happy to review a PR that can improve the situation.

[TeoTN]

I don't have a solution at hand. If it's unfeasible at the moment, it should be probably removed from the default set of rules.

[ljharb]

What is "it" here? There's a number of reasons why components shouldn't be arrow functions; it's totally fine if that truth is revealed by various rules.

[abmantis]

@ljharb Why do you say components shouldn't be arrow functions? I haven't found the reasons yet (not saying they don't exist).

[ljharb]

@abmantis arrow functions don't have explicit names, only sometimes-inferred ones. An explicit name is critical for debuggability and greppability.

[TeoTN]

Arrow functions don't hold names by default, but for debugging purposes they can be assigned a displayName field, which enables finding that name in React Profiler, if you need it.
"Greppability" is not a great argument either, you can grep for the name of the variable holding it, also any IDE will find usages of arrow function by its variable.

My key point here is that react/prop-types rule that is by default enabled in the preset, is supposed to check if prop types are provided, not someone is using arrow functions. Enforcing debatable code style is not within the scope of the rule intent, is it?
If the rule is not fulfilling its purpose, because it doesn't work with one of the most common ways of writing components, it shouldn't be a part of recommended linting rules for React. Probably we shouldn't enforce code style, there's prettier for that, there should be only genuine issues detected.

[ljharb]

You're right about displayName - you can also Object.defineProperty a name on it just fine, but the point is that it's extra effort most people fail to do.

It's supposed to ensure types are provided. It works fine on propTypes for arrow functions - it's just a failing in the TS parser that it can't properly type arrow functions.

Use propTypes, and you can use arrow functions all you want. The rule is working perfectly.

Separately, I don't use prettier because I don't like some of its style choices, and either way, eslint rules should always have style opinions - prettier users can disable the style rules if they like. Style problems are genuine issues.

[abmantis]

@ljharb but if I have const MyCompo = (props) => { ... } won't MyCompo be inferred for the component and show in the debugger as if it was a traditional function, without the need for displayName?

[ljharb]

@abmantis yes, but function name inference is implicit, doesn't work in every browser engine you might be debugging in (or collecting stack traces from), and I'm saying that for clarity/explicitness, it should never be relied upon.

[TeoTN]

It's supposed to ensure types are provided. It works fine on propTypes for arrow functions - it's just a failing in the TS parser that it can't properly type arrow functions.

The problem is that it doesn't always work, and because of that it enforces debatable code style, instead of fulfilling its main purpose. I think that arrow functions are better, because they save me a few bytes of the name in the bundle, and that's more important than having debug names. I can always add them ad-hoc when I need.
This example is not a genuine style problem but, unfortunately, your personal preference, that is no better than anyone's preference.

[ljharb]

The bundle gets gzipped; you save nothing consequential, and that is very much less important than debuggability anyways.

[TeoTN]

I think you're consistently missing my point, that the purpose of this rule is to check props - it is not react/enforce-function-keyword. It's cool that you want to always write the lengthy way and you like it but its not the only way to ensure you have debug names, and there's no reason to push your solution. But the key point here is that the rule is not serving its purpose well not that it has an unrelated side-effect that is opinionated and probably unneeded.

[ljharb]

I agree that due to a flaw in the typescript eslint parser, when using typescript and when writing components as arrow functions, you may find this rule annoying. If that's the case, feel free to turn it off - but the rule is functioning perfectly, the side effect doesn't affect most people since non-arrow components are a best practice and the majority aren't using TS, and I encourage you to file an issue on the TS eslint parser if you want it addressed.

Since there's nothing actionable for this project here until the TS eslint parser is fixed, I'm going to close this, but will be more than happy to reopen if that changes.

[GuiCavi]

I know this is closed until new changes, just want to drop here an info about something I didn't read in above comments.

interface Props {
   prop1: string;
}

// If you do this alternative, you can't extract children from props.
// This means you have to put children in Props interface always.
const CustomComponent: React.FC<Props> = ({ prop1 }: Props) => (...)

Same for this alternative

interface Props {
   prop1: string;
}

const CustomComponent = ({ prop1 }: Props) => (...)

[ljharb]

You should have to have children in your propTypes / props types anyways, it's a prop like any other.

[GuiCavi]

Oh, I just realize it's missing one piece of explanation, sorry for that. According to React:

type PropsWithChildren<P> = P & { children?: ReactNode };

/// ...

interface FunctionComponent<P = {}> {
    (props: PropsWithChildren<P>, context?: any): ReactElement<any, any> | null;
    propTypes?: WeakValidationMap<P>;
    contextTypes?: ValidationMap<any>;
    defaultProps?: Partial<P>;
    displayName?: string;
}

all FunctionalComponents must have a children prop when using typing like const t: React.FC<T>.

But hey, I'm not disagreeing with you, just here to add info to the conversation.

[shirly-chen-awx]

"overrides": [
        {
            "files": ["**/*.tsx"],
            "rules": {
                "react/prop-types": "off"
            }
        }
    ]

Support or ignore prop-types in mixed JS/TypeScript projects

[icfantv]

~Just a head's up on @shirly-chen-awx's post. Not sure if it's because I'm using @typescript-eslint or having WebStorm live running the linting, but off is not a valid value. I had to use the numeric 0 (as opposed to "0") to turn it off.~

[ljharb]

@icfantv what version of eslint? The two valid values to disable a rule have been "off" and 0 since at least v3.

[icfantv]

@ljharb latest version, 7.16. But now I can't reproduce the error (and don't remember what I did to get it to fire...). 🤷 Could be that it was one last dig by 2020 and now that it's 2021, everything is awesome again. Ignore my original comment...