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

Audit components for type-safety around forwardRef #2450

Closed
mattcosta7 opened this issue Oct 18, 2022 · 2 comments
Closed

Audit components for type-safety around forwardRef #2450

mattcosta7 opened this issue Oct 18, 2022 · 2 comments

Comments

@mattcosta7
Copy link
Collaborator

Related to: #2445

In a discussion in slack, we discovered that types for Button were incorrect for consumers, effectively making them React.FunctionComponent<any> instead of maintaining rich types.

After a quick investigation, it appeared like types were fine, going into the forwardRef that returned ButtonComponent but that they lost fidelity on that assignment.

At first glance, this appears as though forwardRef was swallowing a generic, leading to type broadening that allowed unexpected types.

That PR introduced the PolymorphicForwardRefComponent typing as a way to properly constrain the output types, which had previously been overly broadened.

We should probably do a larger bit of discovery around this

  • Add type tests for all components - specifically those related to a forwardRef or a memo call, where typescript might swallow those generics.
  • Update calls to forwardRef (and maybe memo) to use the polymorphic type where necessary

It's not immediately clear where/if this is necessary more than just on button, but we should also try to enforce type tests for new components. Maybe via a PR template update asking about them - as a first step?

@lesliecdubs
Copy link
Member

Also seems related: #1956

@github-actions
Copy link
Contributor

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

@github-actions github-actions bot added the Stale label Apr 19, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants