eslint-plugin-react: False positives of react/prop-types in forwardRefs with 7.27.0

prop type checking seems to broken inside forwardRef with version 7.27.0, 7.26.1 still worked fine.

The error only happens for type intersection hierarchies (I didn’t test interface extensions).

When you have types like this:

type ControlProps = {
  className?: string | undefined;
};

type NamedProps = {
  name: string;
};

type ControlPropsWithChildren = ControlProps & {
  children?: ReactNode;
};

type BaseButtonProps = ControlPropsWithChildren &
  NamedProps & {
    onClick?: (() => void) | undefined;
    onMouseDown?: (() => void) | undefined;
    onMouseUp?: (() => void) | undefined;
    disabled?: boolean | undefined;
    width?: number;
    type?: 'submit' | 'reset' | 'button' | undefined;
  };

The plugin will now falsely flag all properties in nested type intersections as missing in props validation, in this case name, className and children (see screenshot wiggly lines).

The problem does not happen when the component is not wrapped in forwardRef. It also didn’t happen with version 7.26.1

D:\...\BaseButton.tsx
  23:7  error  'name' is missing in props validation       react/prop-types
  24:7  error  'className' is missing in props validation  react/prop-types
  28:7  error  'children' is missing in props validation   react/prop-types

Its likely that this commit: https://github.com/yannickcr/eslint-plugin-react/commit/4bc34995dbab597492c5081493d77755fd3501e8 triggered the problem

About this issue

Original URL
State: closed
Created 3 years ago
Reactions: 23
Comments: 24 (13 by maintainers)

Commits related to this issue

chore: temporarily disable react/prop-types recent release of eslint-plugin-react introduced a bug related to type checking props inside functions: - https://github.com/yannickcr/eslint-plugin-react/... — committed to wwnorton/design-system by sh0ji 3 years ago
[Fix] `propTypes`: handle imported types/interface in forwardRef generic Fixes #3140 — committed to vedadeepta/eslint-plugin-react by vedadeepta 2 years ago

Most upvoted comments

hi, another intermediate fix is to specify the argument types explicitly, like so

import React, { ForwardedRef, forwardRef } from "react";

export interface HelloWorldProps {
  subject: string;
  className?: string;
}

const HelloWorld = forwardRef(
  function HelloWorldBase (
    props: HelloWorldProps,
    ref: ForwardRef<HtmlDivElement>,
  ) {
    const { subject, className } = props;

    return (
      <div 
        className={className}
        ref={ref}
      >
        Hello {subject}
      </div>
    );
  },
);

export default HelloWorld;

+12

aakash-lohono on Jan 29, 2022

Interesting.

On the other hand, the validation still fails when you destructure right away in the function parameters:

const BaseButton = forwardRef<HTMLButtonElement, BaseButtonProps>(
  (
    {
      name,
      className,
      onClick,
      onMouseDown,
      onMouseUp,
      children,
      disabled,
      width,
      type,
    }: BaseButtonProps,
    ref,
  ): JSX.Element => (

gunters63 on Nov 22, 2021

Can also confirm exporting the type in the same file breaks only for forward refs

"@types/react": "^17.0.37",
"@typescript-eslint/eslint-plugin": "^5.6.0",
"@typescript-eslint/parser": "^5.6.0",

The following is with the export:

In saying this, I have no idea why exporting the type would break eslint from getting the proptype. 😕

seems similar to the discussion happening at https://github.com/yannickcr/eslint-plugin-react/issues/3015

JameelKhan9 on Dec 8, 2021

I’ve put together a reasonably minimal example that triggers this:

import React, { forwardRef } from 'react'

interface D {
  x: string
  y: string
  z: string
}
interface C extends 
  Omit<D, 'z'> {}
const A = forwardRef<HTMLDivElement, C>(({ x, y }, ref) => {
  return (
    <div ref={ref}>
      {x} {y}
    </div>
  )
})

Output:

/workspaces/redwood-type-lint-issue/index.tsx
  9:44  error  'x' is missing in props validation  react/prop-types
  9:47  error  'y' is missing in props validation  react/prop-types

✖ 2 problems (2 errors, 0 warnings)

Remove the omission and the props are validated correctly:

interface C extends 
  D {}

Issue seems to be that type is missing at https://github.com/yannickcr/eslint-plugin-react/blob/94826da35804b7ea1ae5adcd6ba4b8e049418cf1/lib/rules/prop-types.js#L107

nzdjb on Nov 23, 2021

@ljharb I can take it if you and @vedadeepta don’t mind 😃

mobily on Mar 12, 2022

Or at least, it should only warn when the entire type is defined in the same file?

ljharb on Dec 21, 2021

That’d be great!

ljharb on Mar 12, 2022

In that case yes, we should remove it.

ljharb on Dec 21, 2021

@ljharb we’re not getting the imported types in the AST. I checked the AST by creating a sample repo and importing types from other files.

Also its not just that the imported types don’t work with forwardRef<> but they also don’t work in general. we just don’t throw an error for other cases.

I don’t think this case can be handled without using the TS compiler API which can get the types from other files.

vedadeepta on Dec 21, 2021

@vedadeepta the TS parser should provide all those types; we shouldn’t have to figure them out ourselves.

ljharb on Nov 29, 2021

@vedadeepta:

I have many other components using those external types which do not use forwardRef. Those work fine.

gunters63 on Nov 29, 2021