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

prop-types: children and React.VFC #2913

Open
MOZGIII opened this issue Feb 1, 2021 · 14 comments
Open

prop-types: children and React.VFC #2913

MOZGIII opened this issue Feb 1, 2021 · 14 comments

Comments

@MOZGIII
Copy link

MOZGIII commented Feb 1, 2021

There is now a way to properly communicate the absence of the children prop at the TypeScript level: React.VFC (vs React.FC).
React.VFC aka React.VoidFunctionComponent does not allow children prop, the prop-types rule should derive the presence/absence of children from the underlying TypeScript types (when used with TypeScript).

Related: #7.

@ljharb
Copy link
Member

ljharb commented Feb 1, 2021

That seems very wrong and strange to me; “void” refers to the return type generally, but “takes children” has no bearing on “renders children”. Can you confirm?

@MOZGIII
Copy link
Author

MOZGIII commented Feb 1, 2021

Yes, the naming is poor, but the semantics is truly about taking children, and not about the "void".

@ljharb
Copy link
Member

ljharb commented Feb 1, 2021

Who decided this type name? Can you provide a link?

@MOZGIII
Copy link
Author

MOZGIII commented Feb 2, 2021

Here is the type: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/f4f453b018d0d512cde50efb6db35fd3f8a001ab/types/react/index.d.ts#L551
Here's the PR: DefinitelyTyped/DefinitelyTyped#46643

UPD: just wanted to add that, in my opinion, the name is not perfect, however, from the code it looks like it does exactly this - eliminates the children from the props. So, I propose that we don't spend time on the discussion of the type name (it is a confusing one), and focus on thinking about how we could make use of it - because, it seems like there is finally a way to make it right. To address the name, we can create an issue/PR to the https://github.com/DefinitelyTyped/DefinitelyTyped. I'm sure it'll be easy to patch up another name if it's renamed at DefinitelyTyped after the implementation is there.

@ljharb
Copy link
Member

ljharb commented Feb 2, 2021

Thanks.

While it is quite a terrible name, it makes sense that prop-types should be able to respect it.

@sladyn98
Copy link

@ljharb I could give this a shot. From what I understand is that the plugin needs to communicate the absence of children prop at the TypeScript level: React.VFC (vs React.FC). Should the prop type rules be changed to accomodate this

@ljharb
Copy link
Member

ljharb commented Feb 13, 2021

I'm not sure where the best place to change it is - we have shared propType detection code, and it might be useful to detect this there so other propType rules can check it. However, it may instead be better to only check it in the prop-types rule.

@sladyn98
Copy link

@ljharb I guess the React.VFC would be deprecated in the next release so adding it to the prop-type rules makes sense. DefinitelyTyped/DefinitelyTyped#46643 We could get rid of the rule later ?

@ljharb
Copy link
Member

ljharb commented Feb 14, 2021

What indicates it would be deprecated?

@sladyn98
Copy link

@ljharb Yeah your right there is no idication I guess I just read it somewehre

@tlmader
Copy link

tlmader commented Jun 28, 2021

@ljharb @sladyn98 ran into it here: https://basarat.gitbook.io/typescript/tsx/react#void-function-components

As of @types/react PR #46643, you can use a new React.VoidFunctionComponent or React.VFC type if you wish to declare that a component does not take children. This is an interim solution until the next major version of the type defs (where VoidFunctionComponent will be deprecated and FunctionComponent will by default accept no children).

@ljharb
Copy link
Member

ljharb commented Jun 28, 2021

In that case we’d need to know which version of the types we were looking at, which makes this a bit more complex :-/

@reberthkss
Copy link

Which class replaces VFC?

@flying-sheep
Copy link

flying-sheep commented Dec 17, 2021

I like that they’ll delete FC and rename VFCFC in the next major release. That’s how FC should have been defined from the start.

If that’s the intention, VFC is truly a horrible name though. it should be named E(xplicit)FC then: “VFC” implies the upgrade path is:

  • use FC for components taking children, specify child type in props interface only if it’s not ReactNode
  • use VFC for components not taking children
  • sort out the mess when upgrading to the next major typing version

I much prefer this upgrade path:

  • use import { VFC as FC } from 'react' everywhere, explicitly state child types (or lack thereof) everywhere

  • (optional) add this to your .eslintrc.yaml:

    no-restricted-imports:
    - error
    - paths:
      - name: react
        importNames: [FC]
        message: 'Use VFC instead and directly type children (e.g. `children?: ReactNode`)'
  • delete VFC as (and eslint rule) in next major typing version

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

No branches or pull requests

6 participants