jsx-eslint / eslint-plugin-react

React-specific linting rules for ESLint
MIT License
8.97k stars 2.77k forks source link

[Bug]: forwardRef error: 'className' is missing in props #3684

Open viveleroi opened 8 months ago

viveleroi commented 8 months ago

Is there an existing issue for this?

Description Overview

The following code from shadcn/ui triggers a lint error: 'className' is missing in props validationeslint[react/prop-types), except this it definitely defined in the prop types.

eslint-config-next 14.1.0 └── eslint-plugin-react 7.33.2

const FormItem = React.forwardRef<HTMLDivElement, React.HTMLAttributes<HTMLDivElement>>(
  ({ className, ...props }, ref) => {
    const id = React.useId()

    return (
      <FormItemContext.Provider value={{ id }}>
        <div className={cn('space-y-2', className)} ref={ref} {...props} />
      </FormItemContext.Provider>
    )
  }
)
FormItem.displayName = 'FormItem'

Expected Behavior

eslint-plugin-react version

7.33.2

eslint version

8.56.0

node version

v21.5.0

ljharb commented 8 months ago

I think this may be fixed by #3679 once it lands, and is a duplicate of #3521?

viveleroi commented 8 months ago

Possibly, but it seems like those involve nested forwardRef calls and mine doesn't. The only existing issue I had found was closed as fixed a long time ago but I'm on the latest version.

developer-bandi commented 8 months ago

in prop-types test code, below code is passed already

{
  code: `
      import React, { forwardRef } from "react";

      export interface IProps {
        children: React.ReactNode;
        type: "submit" | "button"
      }

      export const FancyButton = forwardRef<HTMLButtonElement, IProps>((props, ref) => (
        <button ref={ref} className="MyClassName" type={props.type}>
          {props.children}
        </button>
      ));
    `,
  features: ['ts', 'no-babel'],
}

so In prop-types, the forwardRef single use(not with memo) is not a problem

I think React.HTMLAttributes type inference error is cause of issue's error, so this issue is same to #3325

viveleroi commented 8 months ago

Except in another project we have HTMLAttributes and we get no errors. I'm not sure what the difference is (it being function vs arrow function didn't change anything). In both cases VSCode/TS properly says the type of className is string | undefined

export const BaseButton = forwardRef<HTMLButtonElement, PropsWithChildren<ButtonHTMLAttributes<HTMLButtonElement>>>(
  function baseButton({ className, pressed, uiVariant, ...props }, ref) {
    const classNames = clsx(className, styles.button, uiVariant && styles[uiVariant], {
      [styles.pressed]: pressed
    })

    return <button className={classNames} ref={ref} {...props} />
  }
)
developer-bandi commented 8 months ago

i think the diffrence is PropsWithChildren. To make it more general, the difference arises from importing types from different files.

Let me give you an example. If we expand the code you posted by including the import path, it will look like this:

import React, { forwardRef, PropsWithChildren } from 'react'

export const BaseButton = forwardRef<HTMLButtonElement, PropsWithChildren<ButtonHTMLAttributes<HTMLButtonElement>>>(
  function baseButton({ className, pressed, uiVariant, ...props }, ref) {
    const classNames = clsx(className, styles.button, uiVariant && styles[uiVariant], {
      [styles.pressed]: pressed
    })

    return <button className={classNames} ref={ref} {...props} />
  }
)

and in the code, props-type check is not performed and same thing happens with similar code below

// type.ts
interface PropsTypes{
  falsyTest:string
}

// test.js
import React from "react"
import {PropsTypes} from "./types"

export const App = (props: PropsTypes) => {
    console.log(props.test) // ?? is Ok
     return <div></div>;
}

We can roughly understand why this phenomenon occurs by looking at the rule code of props-type.

The components given as examples above are components.list(); It is returned to the component through , and at this time, if a false value is received from the mustBeValidated function, the subsequent rules are passed.

https://github.com/jsx-eslint/eslint-plugin-react/blob/9f4b2b96d92bf61ae61e8fc88c413331efe6f0da/lib/rules/prop-types.js#L189-L201

The value to look at in the mustBeValidated function below is ignorePropsValidation, which is defined as true and returns false when the type declaration is imported and loaded as in the example above.

https://github.com/jsx-eslint/eslint-plugin-react/blob/9f4b2b96d92bf61ae61e8fc88c413331efe6f0da/lib/rules/prop-types.js#L78-L86

For this reason, it appears that no error occurs in the example code. I hope it will be of help. If you see anything wrong or have any questions, please leave a comment.