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

[viveleroi]

Is there an existing issue for this?

• I have searched the existing issues and my issue is unique
• My issue appears in the command-line and not only in the text editor

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]

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

[viveleroi]

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]

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]

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]

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.

eslint-plugin-react/lib/rules/prop-types.js

Lines 189 to 201 in 9f4b2b9

	return {
	'Program:exit'() {
	const list = components.list();
	// Report undeclared proptypes for all classes
	values(list)
	.filter((component) => mustBeValidated(component))
	.forEach((component) => {
	reportUndeclaredPropTypes(component);
	});
	},
	};
	}),
	};

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.

eslint-plugin-react/lib/rules/prop-types.js

Lines 78 to 86 in 9f4b2b9

	function mustBeValidated(component) {
	const isSkippedByConfig = skipUndeclared && typeof component.declaredPropTypes === 'undefined';
	return Boolean(
	component
	&& component.usedPropTypes
	&& !component.ignorePropsValidation
	&& !isSkippedByConfig
	);
	}

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.

[rbondoc96]

I ran into this issue on an updated version, but in a different manner. I was able to find a workaround, but I wanted to post my findings here in case it's of any use:

Versions (from package-lock.json)

* @types/react: 18.3.12
* eslint: 9.14.0
* eslint-plugin-react: 7.37.2
* typescript: 5.6.3

I replicated the type structure in the initial post and saw I wasn't getting the error (which is good):

import * as React from 'react';

export const SheetHeader = React.forwardRef<HTMLDivElement, React.HTMLAttributes<HTMLDivElement>>(
    // No lint error on `className`
    ({ children, className, ...props }, ref) => {
        return (
            <div className={cn('flex flex-col gap-2', 'text-center', 'sm:text-left', className)} ref={ref} {...props}>
                {children}
            </div>
        );
    },
);
SheetHeader.displayName = 'SheetHeader';

---

Explanation

I was doing some refactoring and was initially importing types from React by name. With this approach, I wasn't getting the linting error:

import { type ComponentPropsWithoutRef, forwardRef } from 'react';

export const SheetHeader = forwardRef<HTMLDivElement, ComponentPropsWithoutRef<'div'>>(
    // No lint error on `className`
    ({ children, className, ...props }, ref) => {
        return (
            <div className={cn('flex flex-col gap-2 text-center sm:text-left', className)} ref={ref} {...props}>
                {children}
            </div>
        );
    },
);
SheetHeader.displayName = 'SheetHeader';

But once I changed my code to use a namespace import to shorten my import code, I got the linting error:

// Same if I were to use `import React from 'react'`
import * as React from 'react';

export const SheetHeader = React.forwardRef<HTMLDivElement, React.ComponentPropsWithoutRef<'div'>>(
    // Lint error with `className` :(
    ({ children, className, ...props }, ref) => {
        return (
            <div className={cn('flex flex-col gap-2 text-center sm:text-left', className)} ref={ref} {...props}>
                {children}
            </div>
        );
    },
);
SheetHeader.displayName = 'SheetHeader';

---

Workaround

I was able to get around this by instead using React.JSX.IntrinsicElements['div'] instead of React.ComponentPropsWithoutRef<'div'> since it effectively works the same way:

• ComponentPropsWithoutRef<T> uses PropsWithoutRef<ComponentProps<T>>...
  • and ComponentProps<T> uses JSX.IntrinsicElements[T] if T extends keyof JSX.IntrinsicElements
• forwardRef<T, P> uses PropsWithoutRef<P> on the render callback, so the ref prop on P isn't actually used.

import * as React from 'react';

export const SheetHeader = React.forwardRef<HTMLDivElement, React.JSX.IntrinsicElements['div']>(
    ({ children, className, ...props }, ref) => {
        return (
            <div className={cn('flex flex-col gap-2 text-center sm:text-left', className)} ref={ref} {...props}>
                {children}
            </div>
        );
    },
);
SheetHeader.displayName = 'SheetHeader';

Important

However, this only solves forward ref components that use HTML element props.

Forward ref components that use another component's ref and rely on ComponentProps or ComponentPropsWithoutRef to extract that component's prop type don't have a nice workaround. Unless that component has a type alias for their props, like below:

import * as SheetPrimitive from '@radix-ui/react-dialog';
import * as React from 'react';

// Radix thankfully exports the prop type
export const SheetDescription = React.forwardRef<
    React.ElementRef<typeof SheetPrimitive.Description>,
    SheetPrimitive.DialogDescriptionProps
>(({ children, className, ...props }, ref) => {
    return (
        <DialogPrimitive.Description className={cn('text-sm text-muted-foreground', className)} ref={ref} {...props}>
            {children}
        </DialogPrimitive.Description>
    );
});
SheetDescription.displayName = 'SheetDescription';

[ljharb]

import * should be avoided in code regardless, but it would be ideal if using it didn't make the linter rule stop working.

[rbondoc96]

import * should be avoided in code regardless, but it would be ideal if using it didn't make the linter rule stop working.

I usually try to avoid using import * in code, but after adopting ShadCN in my projects, they use it practically everywhere. It makes editing the copy-pasted code a bit tedious 😞 So I've chosen to ignore it for now.

I've been learning a bit more about how tree-shaking works, but can't say I'm too familiar with it yet. From what I've gathered so far, it seems like using the namespace import pattern isn't a huge issue when used on packages that support ESM along with bundlers like Vite?

[ljharb]

It's always an issue; tree-shaking can never do anywhere near as good as a job as only importing what you need in the first place. Simply having barrel files in your application will make your bundle and memory footprint much larger, no matter what optimizations you attempt to employ.

[rbondoc96]

It's always an issue; tree-shaking can never do anywhere near as good as a job as only importing what you need in the first place. Simply having barrel files in your application will make your bundle and memory footprint much larger, no matter what optimizations you attempt to employ.

I see, thank you! I appreciate the explanation 🙇🏽‍♂️ Personally, I think I'll choose to not care about it for now since it's just me working on my projects. And I just want to get something out the door and develop as fast as possible. But I'm glad to know this so I can just refactor it all at once in the future 🥴

I think it is slightly unfortunate though that a lot of people using ShadCN might not know this and will continue to use that pattern, since it's what's shown in docs and what will eventually get copy-pasted.

[ljharb]

Indeed, it'd be ideal if they improved their docs to avoid this bad practice.

This is all a tangent tho, since we should still work with it :-)

[rbondoc96]

I think I might have a possible fix. If you think it could be a viable solution, I could submit a PR with this.

Disclaimer: I'm not at all familiar with how ESLint rule packages work. This is my first time digging into it.

---

Findings

I found that searchDeclarationByName in propTypes.ts wasn't resolving an index from the node's type names:

Possible Solution

After I added ComponentProps and ComponentPropsWithoutRef to allowedGenericTypes and genericTypeParamIndexWherePropsArePresent, I no longer got the linting errors when using the import * pattern:

[ljharb]

As long as it comes with test cases, that sounds great!

[rbondoc96]

As long as it comes with test cases, that sounds great!

Cool 🙂 I submitted #3859.