fix(frontend): resolve set-state-in-effect in MoreDrawer (#1063)#1072
Merged
ericsocrat merged 1 commit intomainfrom May 1, 2026
Merged
fix(frontend): resolve set-state-in-effect in MoreDrawer (#1063)#1072ericsocrat merged 1 commit intomainfrom
ericsocrat merged 1 commit intomainfrom
Conversation
Splits the open-tracking effect: resetting animating to false on close is now done during render via the adjusting-state-during-render pattern, while enabling animation on open stays in an effect (the setAnimating(true) call lives inside requestAnimationFrame, so it is asynchronous and does not trip the rule).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📸 PR ScreenshotsCaptured 2 screenshots (1 mobile, 1 desktop) for 1 changed page(s):
📥 Download screenshots from workflow artifacts. Captured by PR Screenshots workflow • 2026-05-01 |
Bundle Size Report
✅ Bundle size is within acceptable limits. |
There was a problem hiding this comment.
Pull request overview
Updates MoreDrawer to eliminate a react-hooks/set-state-in-effect lint violation while preserving the drawer’s animation timing (one-frame delay on open, immediate reset on close).
Changes:
- Moves the “reset
animatingon close” logic into a render-phase “adjust state during render” pattern gated byprevOpen. - Keeps “enable animation on open” in an effect, but defers
setAnimating(true)viarequestAnimationFrameand addscancelAnimationFramecleanup.
Comment on lines
+99
to
114
| // Reset animation immediately when the drawer closes — done during render | ||
| // (https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes) | ||
| // rather than in an effect to satisfy react-hooks/set-state-in-effect. | ||
| if (open !== prevOpen) { | ||
| setPrevOpen(open); | ||
| if (!open) setAnimating(false); | ||
| } | ||
|
|
||
| // Defer enabling the animation class to the next frame so CSS transitions | ||
| // run from the closed state. setState inside requestAnimationFrame is | ||
| // asynchronous, so it does not violate set-state-in-effect. | ||
| useEffect(() => { | ||
| if (open) { | ||
| requestAnimationFrame(() => setAnimating(true)); | ||
| } else { | ||
| setAnimating(false); | ||
| } | ||
| if (!open) return; | ||
| const id = requestAnimationFrame(() => setAnimating(true)); | ||
| return () => cancelAnimationFrame(id); | ||
| }, [open]); |
There was a problem hiding this comment.
The new open/close animation sequencing logic (render-phase reset via prevOpen and deferred setAnimating(true) via requestAnimationFrame) isn’t covered by tests. Consider adding a regression test that toggles open (using rerender) and asserts the drawer/backdrop classes start in the non-animating state and only flip after the rAF callback runs (and that a close transition resets the animation state before the next open).
This was referenced May 1, 2026
ericsocrat
added a commit
that referenced
this pull request
May 1, 2026
…) (#1077) Phase 2 of #1063 -- resolves 2 of 19 violations (running total: 12/19 across 9 PRs). Two react-hooks/set-state-in-effect warnings in SearchAutocomplete.tsx resolved by adjusting state during render: 1. Refresh recent-searches list on dropdown open: replaced the show-dep useEffect with a prevShow render-phase tracker. Added a lazy initial-state initializer so the list is hydrated on mount when show is already true (preserves existing test contract). 2. Reset activeIndex when suggestions/query change: replaced the multi-dep useEffect with a composite resetKey + prevResetKey render-phase tracker. Pattern: https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes Behaviour unchanged. 32/32 vitest pass, eslint and tsc clean. Cross-references: #1067, #1069, #1070, #1071, #1072, #1073, #1074, #1075, #1076. Co-authored-by: ericsocrat <ericsocrat@users.noreply.github.com>
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.
Phase 2 of #1063 — fourth of 19
react-hooks/set-state-in-effectviolations.Change
Splits the
open-tracking effect inMoreDrawerinto two parts:animatingtofalseon close is moved out of the effect into the canonical adjust-state-during-render pattern, gated by aprevOpenstate.setAnimating(true)call is insiderequestAnimationFrame, so it's asynchronous and does not violate the rule. Added acancelAnimationFramecleanup for safety.Behaviour is unchanged: the drawer still gets a one-frame delay before the open class is applied (so CSS transitions run from the closed state), and closes immediately reset the animation flag.
Verification
eslint src/components/layout/MoreDrawer.tsx— cleanvitest run src/components/layout/MoreDrawer.test.tsx— 17/17 passtsc --noEmit— cleanProgress