Repository navigation
refactor: clear the react-hooks/exhaustive-deps lint warnings - #3583
Merged
Merged
Conversation
Contributor
Coverage Report for ./packages/components/
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
🚀 Preview DeploymentPreview environments are ready:
Images:
|
Lisa18289
marked this pull request as ready for review
October 28, 2026 07:44
Lisa18289
enabled auto-merge (squash)
November 3, 2026 06:34
ins0
reviewed
Nov 3, 2026
ins0
left a comment
Member
There was a problem hiding this comment.
Nit: The commit messages contradict each other. fa204ae says LightBoxGallery.tsx keeps its warning, and 139217e says the useWatchPathname and NotificationRenderController warnings stay until the code is fixed. The commits after them silence all three. A squash merge keeps those messages out of main's history.
Deps use an equivalent value where one exists; deliberate deps get an eslint-disable-next-line with the reason. The unused useAutorunEffect is gone. PropsContextProvider keeps its warning: next rewrote that memo, so touching it here would conflict on the next main → next merge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Lisa18289
force-pushed
the
claude/brave-rubin-ab701f
branch
from
November 3, 2026 07:19
980df57 to
f2924f1
Compare
Member
Author
|
Squashed into one commit (f2924f1), so the contradicting messages are gone. |
ins0
approved these changes
Nov 3, 2026
ins0
left a comment
Member
There was a problem hiding this comment.
Nit: The PR body says "clears all but one of them", but the lint job reports two warnings, both on PropsContextProvider.tsx:24: the missing providedProps and the spread element in the dependency array.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
pnpm lintonmainpassed with 69 warnings, allreact-hooks/exhaustive-deps. This clears all but one of them without changing behaviour.regularValueinstead ofvalueFromProps,tunnelinstead ofproviderId, andrenderPromiseinstead ofsrc.OverlayControllerdepends on acontrolleralias instead ofthis. The react-hook-form Field stories get[form];useFormreturns a stable object.useAutorunEffectis gone.eslint-disable-next-linewith the reason. This covers hooks that take deps from their callers, aria-live announcements that must not repeat, values kept per id or persrc, and refs whose.currentre-runs an effect once the element attaches.LightBoxGallery.tsxgets the same disable line as #3513 onnext.PropsContextProvider.tsxkeeps its warning:nextrewrote that memo, so a disable line here would conflict on the nextmain→nextmerge. Merging this PR intonextis clean (git merge-tree).🤖 Generated with Claude Code