frontend: NamespacesAutocomplete: Fix react-hooks/exhaustive-deps — add missing deps to useDefaultNamespaceFallback#5696
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rajeshsingh241 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR updates NamespacesAutocomplete to satisfy react-hooks/exhaustive-deps for the default namespace fallback effect.
Changes:
- Adds missing dependencies to
useDefaultNamespaceFallback’suseEffect. - Removes the prior exhaustive-deps suppression comment.
Review context: CI/check status and PR commit history were not available; please confirm CI results and linear history.
illume
left a comment
There was a problem hiding this comment.
Thanks for this PR.
A few of the commits don't quite follow the project guidelines. We use Linux kernel style for git commits — have a look at the contributing guide and previous commits with git log.
Commits that need attention
frontend: NamespacesAutocomplete: fix prettier formatting— Description must start with a capital letter — e.g.frontend: HomeButton: Fix the buttonnotfrontend: HomeButton: fix the button.
Commit guidelines
- Use atomic commits focused on a single change.
- Use the title format
<area>: <Description of changes>— description must start with a capital letter. - Keep the title under 72 characters (soft requirement).
- Explain the intention and why the change is needed.
- Make commit titles meaningful and describe what changed.
- Do not add code that a later commit rewrites; squash or reorder commits instead.
- Do not include
Fixes #NNin commit messages.
Good examples:
frontend: HomeButton: Fix so it navigates to homebackend: config: Add enable-dynamic-clusters flag
630a6db to
e69d98a
Compare
|
Thanks for the review @illume! I've squashed the commits into one and fixed the dependency array-- selectedNamespaces and dispatch are now intentionally omitted with comments explaining why. Please take another look. |
1704c78 to
f81478b
Compare
Remove eslint suppression in useDefaultNamespaceFallback. Add allClustersConfigs to deps; omit selectedNamespaces (would cause infinite re-renders when user clears filter) and dispatch (stable across renders per Redux guarantee). Part of kubernetes-sigs#5183 Signed-off-by: desibrownsugar <brownsugar34125@gmail.com>
f81478b to
ae25c13
Compare
illume
left a comment
There was a problem hiding this comment.
Thanks for these changes.
Can you please address the open review comments? Once you've resolved each one, please mark it as resolved.
|
@illume I've resolved both Copilot review comments. Please take another look when you get a chance. |
Add
allClustersConfigsas a genuine missing dependency to theuseEffect in
useDefaultNamespaceFallbackhook.selectedNamespacesanddispatchare intentionally kept out ofthe dependency array — adding
selectedNamespacescauses infinitere-renders when the user clears the namespace filter, and
dispatchis stable across renders (Redux guarantee). The eslint suppression
is kept with an inline comment explaining this.
Part of #5183