-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Add ComponentProps reference as first React Types example #626
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for leading this! Definitely important work.
I think we currently mention somewhere that you should prefer *WithRef or *WithoutRef but I don't know why. Could you double check we have a consistent messaging between this cheatsheet and @types/react
Great feedback @eps1lon! I made some updates now. Added a note similar to the note from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love the direction this is going. Finally getting API docs for React types. There's one naming confusion we need to resolve.
61467b4
to
b41e6fb
Compare
|
||
:::note | ||
|
||
Prefer `ComponentPropsWithRef<ElementType>` if ref is forwarded and `ComponentPropsWithoutRef<ElementType>` when ref is not forwarded. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally always found that confusing. The whole point of ComponentProps
is to get the props but at the same time I should know that it has a ref
? Or does that mean ComponentPropsWithoutRef
is just a shorthand for Omit<ComponentPropsWithRef<T>, 'ref'>
?
Either way, this confusion already exist in the types so I'd hope we can do a follow-up with either clearer instructions or a plan to only ComponentProps
and deprecate/remove the ones related to ref
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, I tend to use ComponentProps
despite the recommendation to prefer "being explicit about the ref", and I have never had problems with it.
I made a small experiment and found that ComponentProps
probably is a better default when passing a component type, because it correctly infers whether or not ref is forwarded.
Is there a case for being explicit when passing an element string literal? Is ComponentPropsWithRef<"div">
even something you'd want to do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer having a single ComponentProps
if the only justification for ComponentPropsWithoutRef
is a shorthand for ComponentProps
+ Omit
.
Long-term we won't have forwardRef
anyway and then ref
will be in the props interface for function components.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any insights into how you'll define ref
s in the future? Just a regular prop like this?
interface Props {
ref: Ref
}
function Component({ ref }: Props) {
return <div ref={ref} />
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to investigate if we can deprecate ComponentPropsWithRef
and ComponentPropsWithoutRef
in @types/react. Any tips on how to go about it? Make a PR with these thoughts? Someone in particular to get in touch with before taking that step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd try to find out when they got introduced and why. Then deprecate and provide a counter argument to the initial rationale. By default we don't want to add stuff unless we have to. And if it is convenience there needs to be frequent usage.
This is a visual example of what I'm envisioning in #625.
Having dedicated reference pages for each important React type with explanations and usage examples could probably be very useful.
I haven't been documenting types this way before so I'm not sure what a good way is. Exploring one way here. Happy to receive feedback!
What do you think about the overall idea here @eps1lon?