Components: refactor BorderControl to pass exhaustive-deps#41259
Components: refactor BorderControl to pass exhaustive-deps#41259
BorderControl to pass exhaustive-deps#41259Conversation
BorderControl to pass exhaustive-deps
|
|
||
| onChange( newBorder ); | ||
| }, | ||
| [ onChange, shouldSanitizeBorder, sanitizeBorder ] |
There was a problem hiding this comment.
@aaronrobertshaw , do you remember if there's any particular reason why you added sanitizeBorder to the list of dependencies here?
There was a problem hiding this comment.
🤔 Honestly can't recall and looks like I added it before tidying up local experimentation and drafting the initial PR.
I probably just had it in my head that changing sanitizeBorder would mean a changed callback. It makes sense that coming from an outer scope it isn't a valid dependency and should be removed.
aaronrobertshaw
left a comment
There was a problem hiding this comment.
Thanks for fixing this @chad1008 👍
✅ No linting errors
✅ BorderControl and BorderBoxControl continue to function in Storybook
✅ Border controls within the editor work correctly
LGTM! 🚢
|
|
||
| onChange( newBorder ); | ||
| }, | ||
| [ onChange, shouldSanitizeBorder, sanitizeBorder ] |
There was a problem hiding this comment.
🤔 Honestly can't recall and looks like I added it before tidying up local experimentation and drafting the initial PR.
I probably just had it in my head that changing sanitizeBorder would mean a changed callback. It makes sense that coming from an outer scope it isn't a valid dependency and should be removed.
|
Just chiming in to say it looks like the testing instructions for these PRs could be simplified with command line overrides. I tested: |
What?
Updates the
BorderControlcomponent to satisfy theexhaustive-depseslint ruleWhy?
Part of the effort in #41166 to apply
exhuastive-depsto the Components packageHow?
sanitizeBorderfrom dependency array.sanitizeBorderisn't a prop, and mutating it doesn't rerender the component so it's not a valid dependencycolorSelectionandstyleSelectionto dependency arrayTesting Instructions
exhaustive-depseslint rule #41166, OR manually set'react-hooks/exhaustive-deps': 'warn'in your local eslint filenpx eslint packages/components/src/border-control