jsx-eslint / eslint-plugin-react

React-specific linting rules for ESLint
MIT License
8.88k stars 2.76k forks source link

[Bug]: False positive on `react/no-unused-prop-types` with memo + forwardRef #3524

Open luixo opened 1 year ago

luixo commented 1 year ago

Is there an existing issue for this?

Description Overview

When combining React.memo with React.forwardRef, react/no-unused-prop-types gives false positive.

import { Image } from "react-native";

type Props = {
  foo: string;
};

const LazyImage = React.memo<Props>(
  React.forwardRef<Image, Props>(
    ({ foo }, ref) => {
      console.log("foo", foo);
      return <Image ref={ref} />;
    }
  )
);

Expected Behavior

No error

eslint-plugin-react version

v7.32.0

eslint version

v8.31.0

node version

v16.17.0

Yohandah commented 1 year ago

I have the same issue.

Downgrading to 7.25.1 fixes the issue so I guess the regression was introduced in 7.25.2

vedadeepta commented 1 year ago

@Yohandah can you provide a example of the regression you're facing if possible? will check

Yohandah commented 1 year ago

@Yohandah can you provide a example of the regression you're facing if possible? will check

Of course.

Here it is :

image

(triggers for each props)

triggering in 7.25.2, not triggering in 7.25.1

vedadeepta commented 1 year ago

@Yohandah that seems like weird react syntax or something that i'm not aware of, can you confirm if this is the correct simplification of what you wrote above? this seems to be passing correctly in the latest version

        import React from 'react'
        import { Image } from "react-native";

        type Props = {
          foo: string;
        };

        const LazyImage = React.memo(
          ({ foo }: Props) => {
              return <Image ref={ref} />;
          }
        );

I can replicate the issue with OP's code sample but not yours.

Yohandah commented 1 year ago

Sorry I use Facebook's flow typing. Here's a simpler syntax that triggers the error as well

// @flow

import * as React from 'react'

type SideSheetProps = {|
  isOpened: boolean,
  template: string,
  children: React.Node,
  footerActions?: React.Node,
  close: () => void | Promise<void>,
  step?: 'enter' | 'targeting',
|}

export const SideSheet: React.ComponentType<SideSheetProps> = React.memo(
  ({ isOpened, children, step, template, close, footerActions }: SideSheetProps): React.Node => {
})
vedadeepta commented 1 year ago

yeah i cannot replicate it with this. seems to works fine, can you try the latest version of this lib 7.32.2

Yohandah commented 1 year ago

Hum, interesting because I was on 7.32.0 and had the issue. I will try with 7.32.2, and get back to you

afzaalahmad commented 1 year ago

I am facing the same issue with forwardRef in a higher order component that I've written.

My hoc is like this:

import type React from 'react';
import type { ObjectSchema } from 'yup';
import type { UseFormProps, UseFormReturn, FieldValues, DefaultValues } from 'react-hook-form';
import { useForm } from 'react-hook-form';
import { yupResolver } from '@hookform/resolvers/yup';
import { forwardRef, memo, useImperativeHandle } from 'react';
import { useTranslation, UseTranslationResponse } from 'react-i18next';

export type Context = 'CREATE' | 'UPDATE';

export interface Props<T extends FieldValues> {
  // it is complaining on all of these fields
  context?: Context;
  readonly?: boolean;
  initialValues?: DefaultValues<T>;
}

export type FormRefAttributes<T extends FieldValues, C = unknown> = Pick<
  UseFormReturn<T, C>,
  'handleSubmit' | 'watch' | 'setError' | 'formState'
>;

export const withFormValidation = <T extends FieldValues, P = unknown, C = unknown>(
  Form: React.FC<P & Props<T> & UseFormReturn<T, C>>,
  getFormSchema: (context: Context, t: UseTranslationResponse<any>) => ObjectSchema<T>,
  useFormProps?: Omit<UseFormProps<T, C>, 'resolver'>,
): React.MemoExoticComponent<
  React.ForwardRefExoticComponent<React.PropsWithoutRef<P & Props<T>> & React.RefAttributes<FormRefAttributes<T, C>>>
> =>
  memo(
    forwardRef<FormRefAttributes<T, C>, P & Props<T>>((props, ref) => {
      const { context = 'CREATE', initialValues } = props;
      const translation = useTranslation();
      const form = useForm<T, C>({
        mode: 'onSubmit',
        reValidateMode: 'onChange',
        defaultValues: initialValues,
        resolver: yupResolver<ObjectSchema<T | any>>(getFormSchema(context, translation)),
        ...useFormProps,
      });

      useImperativeHandle(ref, () => form, [form]);
      return <Form context={'CREATE'} {...props} {...form} />;
    }),
  );

My usage of hoc is

export const ContactDetailsForm = withFormValidation<ContactFields>((props) => <Form {...props} />)

and then I am using this form as

<ContactDetailsForm initialValues={initialData} readonly={true} />

Now I am using these props, I just joined them using & with generics of hoc.

Let me know if you have any questions.

artola commented 10 months ago

Similar to @luixo example above, this is a test added to tests/lib/rules/prop-types.js (using master at 7.33.2, https://github.com/jsx-eslint/eslint-plugin-react/commit/ecadb92609998520be80d64c0bd6bc5e05934aa9):

    {
      code: `
        import React, { forwardRef, memo } from 'react'
        interface IfooProps { e: string; }
        const Foo= memo(forwardRef<HTMLDivElement, IfooProps>(function Foo({ e }, ref) {
          console.log(e);
          return  <div ref={ref}>hello</div>;
        }));
      `,
      features: ['ts', 'no-babel'],
    },

with the following result:

 AssertionError [ERR_ASSERTION]: Should have no errors but had 1: [
  {
    ruleId: 'prop-types',
    severity: 1,
    message: "'e' is missing in props validation",
    line: 4,
    column: 78,
    nodeType: 'Property',
    messageId: 'missingPropType',
    endLine: 4,
    endColumn: 79
  }
]

When I destructure the props, the test passes.:

    {
      code: `
        import React, { forwardRef, memo } from 'react'
        interface IfooProps { e: string; }
        const Foo= memo(forwardRef<HTMLDivElement, IfooProps>(function Foo({ ...rest }, ref) {
          console.log(rest);
          return  <div ref={ref}>hello</div>;
        }));
      `,
      features: ['ts', 'no-babel'],
    },
ilijavuk commented 7 months ago

I am having a similar problem using eslint-plugin-react version 7.33.2. This is my code:

type CardItem = {
  id: number;
  text: string;
};

type CardProps = {
  cardItems: CardItem[];
};

And then later on in the code:

{cardItems.map(({ id, text }: CardItem) => {
  return (
    <div key={id} className='card__item'>
      {text}
    </div>
  );
})}

This gives me the following errors:

ESLint: 'id' PropType is defined but prop is never used(react/no-unused-prop-types)
ESLint: 'text' PropType is defined but prop is never used(react/no-unused-prop-types)
MikolajWBD commented 2 months ago

Issue still occurs in 7.34.1 version, any update on it?