[react/require-default-props] False positive with React.forwardRef

[nwalters512]

The react/require-default-props rule configured with ignoreFunctionalComponents: true produces false positives for functional components that use React.forwardRef, like the following:

import React from "react"
import PropTypes from "prop-types"

const Demo = React.forwardRef((props, ref) => {
  const { text } = props
  return <div ref={ref}>{text}</div>
})

Demo.propTypes = {
  // Error: propType "text" is not required, but has no corresponding defaultProps declaration
  text: PropTypes.string,
}

export default Demo

Here are two CodeSandbox links demonstrating the problem with both .jsx and .tsx files:

• https://codesandbox.io/s/gatsby-starter-default-forked-hfn6k?file=/src/components/demo.jsx
• https://codesandbox.io/s/gatsby-starter-default-forked-hfn6k?file=/src/components/demo.tsx

Run yarn lint to reproduce.

I would expect components built with React.forwardRef to be considered functional components and be ignored when ignoreFunctionalComponents is true.

[ljharb]

Indeed, forwardRef does not take a component - in that case, that's just a normal function.

However, in this case, the warning is 100% correct. You provided a non-required propType, without a defaultProp. The point of the rule is to error on that.

Note that functional components accept propTypes, defaultProps, contextTypes, etc - almost every static property that class components support, functional components do.

[nwalters512]

But Demo itself (the return value of React.forwardRef) is a React component, correct? And given that it's certainly not a class component, I would expect to not get an error if I tell the rule to ignore functional components. Does a React.forwardRef component exist in a limbo between true functional components and class components such that ignoreFunctionalComponents does not apply here?

[ljharb]

Demo itself is not, it's a forwardRef component, which is a distinct and unique thing - and EVERY component, class or function, needs both propTypes and defaultProps as appropriate. With ignoreFunctionalComponents, I would expect only unwrapped functional components to be ignored.

[nwalters512]

Interesting! In that case, would you accept a PR adding something like ignoreForwardRefComponents to complement ignoreFunctionalComponents?

[ljharb]

Since there's forwardRef components, memo components, lazy components, I don't think we'd want to add one option for each kind.

I'm curious about the exact use case here - why don't you want defaultProps on a non-required component here, but you do want that error on class components?

[nwalters512]

For the same reason we don't want to require defaultProps on "standard" functional components - we want to allow developers to use destructuring with defaults if they prefer. I know the debate over that has been had many times before in this repo, so I don't particularly want to rehash it, but suffice it to say that we've made the call to allow it and have configured this lint rule as such. forwardRef components might not be strictly the same as functional components, but they still use a render function, can use hooks, and can be written easily with destructuring defaults.

As to "why enable it for class components", we'd like to keep the rule active for our legacy class components since one can't use destructuring as easily, i.e. you'd need to include the same defaults every time you access this.props in a class method. We write virtually no class components now, but as long as they exist in our codebase, we'd like to apply this lint rule to them.

[ljharb]

The only reasonably justification that's been offered for omitting defaultProps on functional components is if you're using a type system that can infer the same information.

In this case, what does TypeScript or Flow infer for the type of text inside the non-component render function that's passed to forwardRef?

[nwalters512]

That's actually exactly my use case - we're using TypeScript!

Here's a slightly more real-world example:

import React from "react"
import PropTypes from "prop-types"

interface DemoProps {
  text?: string
}

const Demo = React.forwardRef<any, DemoProps>((props, ref) => {
  const { text } = props
  return <div ref={ref}>{text}</div>
})

Demo.propTypes = {
  text: PropTypes.string,
}

export default Demo

Now that we've explicitly typed the prop text as string | undefined (via text?: string), the variable text is typed as such in the body of the forwardRef render function. So, as written, adding something like console.log(text.substring(1)) in the body of the render function would fail with a type error: Object is possibly 'undefined'.ts(2532).

Now consider the case where we provide a default when destructuring:

const Demo = React.forwardRef<any, DemoProps>((props, ref) => {
  const { text = 'default text' } = props
  return <div ref={ref}>{text}</div>
})

Even though the prop text is typed as string | undefined, the destructured variable text has the type string, since it will assume the default value if the prop isn't defined.

[ljharb]

The issue is that react components (like functions) actually have two type signatures: the external one, that consumers/callers see, and the internal one, that the component/function body sees. In your case, DemoProps is the external one, where text is optional. It's an implementation detail of the body that you handle the case where it's not provided.

Without a type system, this would be modeled with an optional propType and a defaultProp.

I do agree that require-default-props should allow, at least, the destructuring of props in the forwardRef function signature, and this seems reasonable as default behavior.

[therealgilles]

Here is what I ended up doing to workaround the issue:

import React from 'react'
import type { ForwardedRef } from 'react'

type CompProps = {
  propRequired: string
  propOptional?: string
}

const defaultProps = {
  propOptional: 'default string', // or undefined
}

// Specify underlying component and attributes types
// Different components will have different attributes getters.
type UnderlyingComp = HTMLDivElement
type UnderlyingCompAttributes = React.HTMLAttributes<HTMLDivElement>

// Exclude component props in case there is an overlap
type OtherProps = Omit<
  React.PropsWithoutRef<UnderlyingCompAttributes>,
  'propRequired' | 'propOptional'
>

type CompPropsOpt = CompProps & OtherProps
type CompPropsReq = Required<CompProps> & OtherProps

const Comp = (props: CompPropsReq, ref: ForwardedRef<UnderlyingComp>) => {
  const { propRequired, propOptional, children, ...otherProps } = props
  
  // ... do something with propRequired & propOptional ...
  
  return (
     <div ref={ref} {...otherProps}>{children}</div>
  )
}

// Call forwardRef with required props type but cast with optional props type
const CompWithForwardedRef = React.forwardRef<UnderlyingComp, CompPropsReq>(
  Comp
) as React.ForwardRefExoticComponent<CompPropsOpt>

// Add default props and display name
CompWithForwardedRef.defaultProps = defaultProps
CompWithForwardedRef.displayName = 'Component'

export default CompWithForwardedRef

[bryanltobing]

is there a workaround for this?

[cristian-atehortua]

Hi, I have a similar case.

I'm using "react/require-default-props": ["error", { functions: "defaultArguments" }] and I have a component similar to this

import React, {  forwardRef, memo } from 'react';
import PropTypes from 'prop-types';

const Button = forwardRef(({
  children,
  onClick = () => { console.log('Button clicked') },
}, ref) => (
  <button
    type="button"
    onClick={onClick}
    ref={ref}
  >
    {content}
  </button>
));

Button.propTypes = {
  children: PropTypes.node.isRequired,
  onClick: PropTypes.func,
};

export default memo(Button);

What I expected here is to not have Eslint errors since we are specifying default props in the function arguments as is configured. However, react eslint plugin is not detecting this as a function component because of the forwardRef and enforces me to define Button.defaultProps.

Can this be solved?

[ljharb]

The rule is called "require default props", and .defaultProps is superior to default arguments for a number of reasons.

In this case, your default onClick callback is created anew every render, without useCallback, where with defaultProps it would not be, so using default arguments here is a potential performance hit unless you remember to wrap it in useCallback (altho in this case, you're passing it to an HTML element, so it's fine, but it's still better to memoize all objects passed as props as good hygiene)

Either way, https://github.com/jsx-eslint/eslint-plugin-react/issues/2856#issuecomment-747742498 and the "help wanted" label still apply, so if you want it solved, submit a PR.

[cristian-atehortua]

@ljharb Take a look on this:

• https://github.com/reactjs/rfcs/blob/createlement-rfc/text/0000-create-element-changes.md#deprecate-defaultprops-on-function-components
• https://github.com/facebook/react/pull/16210

React is deprecating defaultProps on function components.

[ljharb]

I'm aware, but they're wrong to do.

Separately, that RFC is from 2019, and despite two major versions being released since then, they haven't taken steps to go further.

[SalahAdDin]

I am having the same problem but without using any forwardRef.

[woodreamz]

So forwardRef and "functions": "defaultArguments" are just incompatible? There is no workaround?

[vicasas]

Same problem here 🤚

[chandra-logituit]

@cristian-atehortua did you find any solution/workaround for this?

[cristian-atehortua]

Hi @chandra-logituit. You have two options:

• define defaultProps as the component was a Class component (i.e. using MyComponent.defaultProps):

import React, {  forwardRef, memo } from 'react';
import PropTypes from 'prop-types';

const Button = forwardRef(({
  children,
  onClick,
}, ref) => (
  <button
    type="button"
    onClick={onClick}
    ref={ref}
  >
    {content}
  </button>
));

Button.defaultProps = {
  onClick: () => { console.log('Button clicked') },
};

Button.propTypes = {
  children: PropTypes.node.isRequired,
  onClick: PropTypes.func,
};

export default memo(Button);

or,

• define the component as a function and wrap it in forwardRef in a different clause (ugly)

import React, {  forwardRef, memo } from 'react';
import PropTypes from 'prop-types';

const Button =({
  children,
  onClick = () => { console.log('Button clicked') },
}, ref) => (
  <button
    type="button"
    onClick={onClick}
    ref={ref}
  >
    {content}
  </button>
);

Button.propTypes = {
  children: PropTypes.node.isRequired,
  onClick: PropTypes.func,
};

export default memo(forwardRef(Button));

[chandra-logituit]

@cristian-atehortua Thank you so much for taking your time to explain it. I am familiar with both approaches, but my question was related to the false positive with forwardRef. Anyways we have decided to go with defaultProps which is working perfectly fine.

[cristian-atehortua]

HI @chandra-logituit. Yes, they are the only workarounds I know to face the issue with the false positives when using forwardRef.