-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Components: refactor FontSizePicker
to pass exhaustive-deps
#41600
Conversation
Size Change: +5 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
dbd2662
to
7ab1e60
Compare
refactorDraggable
to pass exhaustive-deps
FontSizePicker
to pass exhaustive-deps
7ab1e60
to
a2df833
Compare
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.
Thanks for the PR @chad1008 !
I'm guessing the goal with selectedOption?.slug was to cover both .name and .size with a single dependency
I don't remember exactly, but that was my first thought as well, or something missed(?).. 😄 Either way this PR looks good.
Awesome, thanks @ntsekouras! |
What?
Updates the
FontSizePicker
component to satisfy theexhaustive-deps
eslint ruleWhy?
Part of the effort in #41166 to apply
exhuastive-deps
to the Components packageHow?
add
selectedOption?.name
,selectedOption?.size
, andshouldUseSelectControl
to theheaderHint
useMemo
dep array. RemoveselectedOption?.slug
which isn't a necessary dep of this callback.I'm guessing the goal with
selectedOption?.slug
was to cover both.name
and.size
with a single dependency, but cc'ing @ntsekouras in case you had something different in mind when this was first introducedTesting Instructions
npx eslint --rule 'react-hooks/exhaustive-deps: warn' packages/components/src/font-size-picker