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.

Sign up for GitHub

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

Jump to bottom

useControlledValue: let TypeScript infer the return type #46164

Merged
merged 2 commits into from
Nov 30, 2022
Merged

useControlledValue: let TypeScript infer the return type #46164

merged 2 commits into from
Nov 30, 2022

Conversation

ciampo
Copy link
Contributor

@ciampo ciampo commented Nov 29, 2022 •
edited
Loading

What?

Refine useControlledValue's return type

Why?

The new type makes the hook behave more similarly to a native react hook, avoiding the need for type casting like discussed in #45771 (comment)

How?

  • in the return statement, we typecast setValue to be "whatever is the type of the setState function
  • we add as const, which basically tells TypeScript to interpret the type of the return statement as literally as possible (see const assertions)

Testing Instructions

  • code makes sense
  • no TypeScript errors

@ciampo ciampo added [Type] Enhancement A suggestion for improvement. [Package] Components /packages/components labels Nov 29, 2022
@ciampo ciampo requested a review from ajitbohra as a code owner November 29, 2022 15:27
@ciampo ciampo self-assigned this Nov 29, 2022
@ciampo ciampo mentioned this pull request Nov 29, 2022
Open
sarayourfriend
sarayourfriend reviewed Nov 29, 2022
View reviewed changes
Copy link
Contributor

@sarayourfriend sarayourfriend left a comment

Choose a reason for hiding this comment

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

Makes sense to me 🙂 It is a nice simplification.

@ciampo ciampo enabled auto-merge (squash) November 29, 2022 22:48
tyxla
tyxla approved these changes Nov 30, 2022
View reviewed changes
Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

Looks good 🚀

@ciampo ciampo merged commit 8633ae8 into trunk Nov 30, 2022
@ciampo ciampo deleted the refactor/use-controlled-value-refine-type branch November 30, 2022 09:46
@github-actions github-actions bot added this to the Gutenberg 14.7 milestone Nov 30, 2022
mpkelly pushed a commit to mpkelly/gutenberg that referenced this pull request Dec 7, 2022
@ciampo @mpkelly
useControlledValue: let TypeScript infer the return type (WordPress#4…
020c67b 
…6164)

* useControlledValue: let TypeScript infer the return type

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

@sarayourfriend sarayourfriend sarayourfriend left review comments

@tyxla tyxla tyxla approved these changes

@mirka mirka Awaiting requested review from mirka

@chad1008 chad1008 Awaiting requested review from chad1008

@brookewp brookewp Awaiting requested review from brookewp

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

Assignees

@ciampo ciampo

Labels
[Package] Components /packages/components [Type] Enhancement A suggestion for improvement.
Projects
None yet
Milestone
Gutenberg 14.7
Development

Successfully merging this pull request may close these issues.
3 participants
@ciampo @tyxla @sarayourfriend