Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using React.FC does not properly extract prop types #1989

Open
dbasilio opened this issue Mar 24, 2022 · 4 comments
Open

Using React.FC does not properly extract prop types #1989

dbasilio opened this issue Mar 24, 2022 · 4 comments

Comments

@dbasilio
Copy link

dbasilio commented Mar 24, 2022

Current behavior

We're migrating parts of our app to typescript slowly, and leaning pretty heavily on React.FC to correctly set prop and return types on our component. When you cast a component using only React.FC, styleguidist does not display the Props and Methods field.

I tested this on both 11.2.0 and on 12.0.0-alpha9.9 and the issue is present on both.

To reproduce

This component will fail to produce a Props & Methods field

import React from 'react'

type Props = {
    someProp: boolean
}

const MyComponent: React.FC<Props> = ({ someProp }) => {
    if (someProp) return null
    return <div>Not Empty<div>
}

This component will produce a Props & Methods field, but the type arg for each prop will be empty. Only default field will have values.

import React from 'react'

type Props = {
    someProp?: boolean
    someOtherProp?: boolean
}

const MyComponent: React.FC<Props> = ({ someProp, someOtherProp = false }) => {
    if (someProp && someOtherProp) return null
    return <div>Not Empty<div>
}

MyComponent.defaultProps = {
    someProp: false,
}

Weirdly, casting the props directly will produce the correct results regardless of how defaultProps are provided. So there's likely some logic to extract the correct types from the args that are not being correctly applied to React.FC.

import React from 'react'

type Props = {
    someProp?: boolean
    someOtherProp?: boolean
}

const MyComponent: React.FC<Props> = ({ someProp, someOtherProp = false }: Props) => {
    if (someProp && someOtherProp) return null
    return <div>Not Empty<div>
}

MyComponent.defaultProps = {
    someProp: false,
}

Expected behavior

Usage of React.FC extracts the correct prop types to display in the "Props & Methods" field.

@poteirard
Copy link
Contributor

Maybe it's not the solution you are looking for as I agree it should work.

We are refactoring our code base in our UI library to stop using React.FC after reading this article. I used this guide to have proper typing and found this part that explains why React.FC is discuraged.

@noahnu
Copy link

noahnu commented Jul 12, 2022

The article is a bit outdated.

  1. You type a function, not its arguments

In React, a function component is not just a function which returns JSX children. It also has some static properties such as displayName and propTypes (though prop types are discouraged in favour of parameter defaults). If you don't type the function, you lose typings on the static properties.

  1. FC<> always imply children

Perhaps when the article is written, however React exposes React.VFC which is a void function component that omits children.

  1. Easier to move to Preact

I suppose whether this is a pro/con is up to the maintainers of this library. At my work we're only interested in React support.

  1. React.FC<> breaks defaultProps

defaultProps can be used, however React is recommending using function parameters. See tweet by Dan Abramov and RFC.

@poteirard
Copy link
Contributor

In our case, we don't need static props. Regarding preact, we don't use it either.

What I see after migrating to my approach is:

  1. One less 3rd party interface dependency to take care of. For example, in case React changes again the interface.
  2. Better compatibility with tools such as react-styleguidist (which is the OP is trying to fix)
  3. Less code
  4. Code more readable. No need to know what VFC is.

@adbutterfield
Copy link

React.FC works fine for me. I think I had to propsParser though.

In my styleguide.config.js:

const reactDocgenTypescript = require('react-docgen-typescript');

// You need this or else you will get a ton of props if your component also includes something like React.ComponentPropsWithoutRef<'div'>
const options = {
  propFilter: (prop) => {
    if (prop.parent) {
      return !prop.parent.fileName.includes('node_modules');
    }
    return true;
  },
};

module.exports = {
  // other config...
  propsParser: reactDocgenTypescript.withCustomConfig(`${process.cwd()}/tsconfig.json`, options).parse,
};

Defaults do not work unless you have defaultProps. I also add them as function parameters though, because ts usually will complain if optional parameters don't have a default assignment.

And yeah, I'm also considering dropping React.FC from the next major version of the component library I work on...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants