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

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

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Reactions: 152
  • Comments: 83 (31 by maintainers)

Commits related to this issue

Most upvoted comments

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

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

Adding : PersonProps (somewhat redundantly) seems to work…

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

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

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>

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.

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.

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.

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.

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

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

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.

@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

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

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.

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

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:

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 = () => {}

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

@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.

I just stopped tracking this issue : by adding "rules": { "react/prop-types": "off" } in .eslintrc.js

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

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

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.

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

Can’t you do? :

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

This rule is just doubling up on the job TS is already doing. Don’t recommend.

@ITenthusiasm i’d prefer a new issue be filed that clearly outlines the problem and the needed solution - this thread is quite long and full of distractions.

Yes, there ideally should be an eslint rule forbidding functional components to be written as arrow functions; i’m not sure why we don’t have one yet.

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) => (...)

@AdrianDiazG you’re not supposed to do that tho - components should be named functions.

Returning to this conversation again after quite some time to make 2 comments. The first one is just to get people thinking. You don’t have to care about it. 🙂

First, although I mentioned earlier that I typically use regular functions, I have found React.FC appealing in the past because it created a simple way to make components in React with TypeScript. The place I’m working at now uses React.FC frequently. However, after trying to make some re-usable and non-simple components, I realized something and wanted to give a warning:

I think we’re used to React.FC because it’s what we’re given out of the box. But actually, React.FC makes your components less specific than they should be. Here are 2 basic examples. There are likely more.

1) Children

From gearFifth’s comment (mohsinulhaq’s comment had something similar):

with this you lose the default props that come with the functional component like the children prop.

At first this made me sad. But I’ve learned this is actually a really good thing. Although in reality all React components can take a children prop, not all components are intended to take a children prop. Similarly, any regular function you define can take more than the specified number of arguments. However, that’s not the intent of the function and it won’t accomplish anything. Supplying extraneous arguments will only cause confusion. This is part of the point of TS to begin with.

“But I actually intend to use the children prop for such and such component”. That’s a valid argument. But in that case, it’s a much better developer experience to explicitly supply children in your ComponentProps type/interface for two reasons:

  1. It tells the developer that this component explicitly requires or intends to use the children prop.
  2. It tells the developer what is allowed for the children prop, preventing any confusion.

On that second note, let’s say that you have a Card component that’s intended to take text as its children. And it only expects text. The default children prop from React.FC is a ReactNode. This means TS will not care if your developer passes some jank JSX into the component and breaks it because you didn’t tell them what was expected by your types. Thus, it’s friendlier to explicitly type your children anyway.

Unfortunately, React.FC also has the potential to get in the way of your explicitly defined children prop. For instance, when I try to say “Hey, use a string child”, React.FC says, “Oh. You wanted string & ReactNode for children”. In the end, TS seems to restrict it properly. But that can still potentially be a source of confusion.

2) Return Types

This is the thing that really bothers me, to be honest. I absolutely hate specifying return types on components. And yet… I’ve come to see they’re useful while making some re-usable or non-simple components. The default return type from React.FC is nice, but it’s also not always valid. React.FC leaves the door open for a component to return null, but maybe you have components that are never intended to return null. Again, stricter, more explicit type guards protect you from this issue. They also prevent TypeScript from complaining about how a component could be null when you know you’ve written your code so that it never would be null.


Regarding this

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;
}

defaultProps and propTypes are taken care of by TS. contextTypes and the context argument are legacy from what I understand and are taken care of by useContext. And an explicit children prop and return type provide a better developer experience with clearer type specificity (…long term, that is… I get the pain of not liking to write it out).

Of course, you may not always care about this level of specificity. But oftentimes you do, and that’s worth nothing. (And if you don’t care, it might be worth asking why you’re interested in TS.) Defaulting to regular functions instead of React.FCs increasingly seems like more of a good idea to me as I learn more (except for the fact that I have to keep specifying return types 😔).

The problem with the default FC type provided by React is that it tries to be JavaScript and TypeScript at the same time. That is, it acknowledges the fact that the JS functionality will allow you to do whatever you want (like adding extraneous args). And it seems to try to keep the type “backwards compatible” for anyone working with legacy code. So it creates a TS type to cover all of the use cases. But again, this defeats the point of the safety TS aims to provide to begin with. Instead, they should’ve provided the bare minimum types for a functional component.

Stated differently (and perhaps far too strongly), React.FC as it is now – in some sense – is the React version of using any in TS… but for functional components.

Consider these things when you write your own components…and in the significance of this overall discussion.

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

Because React.FC doesn’t make sense if React isn’t in scope.

Adding : PersonProps (somewhat redundantly) seems to work…

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

with this you lose the default props that come with the functional component like the children prop.

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.

@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?

I fixed the error

'name' is missing in props validationeslint[react/prop-types](https://github.com/jsx-eslint/eslint-plugin-react/tree/master/docs/rules/prop-types.md)
import { useEffect } from 'react';

interface Props {
  name: string;
}

const App: React.FC<Props> = ({ name }) => {
  useEffect(() => {}, []);
  return <div>{name}</div>;
};

export default App;

error disappears after importing React, although I still don’t understand why it worked

import React, { useEffect } from 'react';

interface Props {
  name: string;
}

const App: React.FC<Props> = ({ name }) => {
  useEffect(() => {}, []);
  return <div>{name}</div>;
};

export default App;

I would like to mention another case where I couldn’t resolve the linting errors after testing all the proposed solutions in this thread (turning off this lint rule should not be the solution but rather a temporary workaround):

import React from 'react';

type MySvgIconProps = React.SVGProps<SVGSVGElement>;

export function MySvgIcon(props: MySvgIconProps) {
  const withDefaults = {
    ...props,
    width: props.width ?? 100,
    height: props.height ?? 100,
    color: props.color ?? '#fff',
  };

  return (
    <svg
      xmlns="http://www.w3.org/2000/svg"
      enableBackground="new 0 0 24 24"
      viewBox="0 0 24 24"
      {...withDefaults}>
      <rect fill="none" height="24" width="24" />
      <path
        d="M1,1C20,20,16,-8"
        fill={withDefaults.color}
      />
    </svg>
  );
}

Error:

   9:18  error  'width' is missing in props validation   react/prop-types
  10:19  error  'height' is missing in props validation  react/prop-types
  11:18  error  'color' is missing in props validation   react/prop-types

Modules:

"@typescript-eslint/eslint-plugin": "3.10.1",
"@typescript-eslint/parser": "3.10.1",
"eslint": "7.26.0",
"@types/react": "16.14.6",
"react": "16.14.0",

While typescript understands it fully and doesn’t complain, eslint probably doesn’t understand React.SVGProps<SVGSVGElement> fully (which defines the used properties)

For React, named functions are absolutely enough. If you can find a popular tool that doesn’t respect a component’s name when available, call it out and i’ll send them a PR fixing it.

Dan himself says that components shouldn’t be arrow functions, because of the name in particular. It is a perfect candidate for being a recommended rule - components should be regular functions.

My two cents:

  1. While you can make a rule for enforcing arrow functions, realize they are necessary for React.memo, React.forwardRef, or any other type of composition function. Also while they are better for automatically setting display name, any project that uses babel will still have displayName on arrow functions by simply using the preset for react. You can also handle setting displayName through a babel plugin macro. IMO this should never be part of the recommended ruleset, it’s too dependent on the toolchain to decide whether it’s a good policy or not. Considering React.memo forces you to use variable assignment (blowing away the main reason here for using function) there’s even an argument for always using fat arrows for consistency.
  2. React.FC was removed from the create-react-app typescript template because of how it handles children. While a convenient shorthand, you can no longer make children required or disallowed, or restrict it further than ReactNode which is pretty broad. If you’re fine with the looser typings on children it’s still pretty convenient. I don’t think it’s relevant to this discussion but I think the community is moving away from it.
  3. You can always turn off this rule and just use explicit module boundary types from typescript-eslint

Second, despite everything that I said in that first comment of mine above, I think there is still an issue that needs to be addressed.

We have an ESLint rule that can’t be disabled by default because it provides good common standards to follow. (There are other React developers that don’t use TS. 😏) At the same time, the linting does seem to be incorrect here for TS. In a theoretical world where React.FC was improved to have the bear minimum types, I’m assuming that we’d still run into this issue. So if possible, this problem warrants fixing.

The comments about turning this rule off locally are becoming redundant, and they aren’t providing a true solution to the problem. At the same time, comments about only using non-arrow functions still leave the problem at hand. If it’s truly important, restricting components to regular functions would need to be its own ESLint rule…though idk how it would be written. But that shouldn’t bleed into this discussion.

Although I haven’t been able to do anything with it myself yet, the discussion at typescript-eslint/typescript-eslint#3075 provides guidance towards a solution to this problem if I understood it correctly. Since this issue is important, if it is actionable, then @ljharb could you reopen this issue? If it’s nothing that’s being immediately prioritized, maybe something like a help wanted tag would be fine. But an open issue would more clearly specify intent here.

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) => (...)

with this you lose the default props that come with the functional component like the children prop.

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.

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

@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.

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

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.