Skip to content

Provide types for all react refs#3328

Merged
robertbrignull merged 3 commits intomainfrom
robertbrignull/ref_types
Feb 8, 2024
Merged

Provide types for all react refs#3328
robertbrignull merged 3 commits intomainfrom
robertbrignull/ref_types

Conversation

@robertbrignull
Copy link
Copy Markdown
Contributor

Everywhere we create or consume a React ref, give it a proper type instead of using Ref<any>.

To make this work I switched to passing null as the initial value instead of leaving it blank. I believe this doesn't change the behaviour of the components in any way.

Leaving the initial value empty was leading to a computed type of Thing | undefined and that was causing problems when trying to pass it to ref={ref} on an element. The ref type already expects null to be a possible value, so using that instead of undefined makes everything happy.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@robertbrignull robertbrignull requested a review from a team February 7, 2024 17:53
@robertbrignull robertbrignull requested a review from a team as a code owner February 7, 2024 17:53
Copy link
Copy Markdown
Member

@koesie10 koesie10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM!

Leaving the initial value empty was leading to a computed type of Thing | undefined and that was causing problems when trying to pass it to ref={ref} on an element. The ref type already expects null to be a possible value, so using that instead of undefined makes everything happy.

It seems like the behavior of passing in null is slightly different than specifying the type as T | null since the first doesn't allow you to directly modify the ref. However, we don't need that, so this seems like it works for better for us.

@robertbrignull robertbrignull merged commit dd74372 into main Feb 8, 2024
@robertbrignull robertbrignull deleted the robertbrignull/ref_types branch February 8, 2024 10:02
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

Successfully merging this pull request may close these issues.

2 participants