fix(frontend): resolve set-state-in-effect in app search page (#1063)#1078
fix(frontend): resolve set-state-in-effect in app search page (#1063)#1078ericsocrat merged 1 commit intomainfrom
Conversation
Resolves 3 react-hooks/set-state-in-effect violations in src/app/app/search/page.tsx (Phase 2 of #1063): - Mount-load effect: replaced with lazy useState initializers for recentSearches, showAvoided, viewMode (helpers already SSR-safe via globalThis.localStorage guards) - debouncedQuery sync effect: replaced with Pattern A render-phase tracker (composite submitKey of debouncedQuery + filters) - page reset effect: replaced with Pattern A render-phase tracker (composite pageResetKey of submittedQuery + filters) Validated: 54/54 vitest pass, eslint clean, tsc clean. Phase 2 cumulative: 15/19 (final non-deferred multi-violation file). Cross-refs: #1067 (Phase 1), #1069-#1077 (Phase 2 micro-PRs).
|
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c59b6c221
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const [filters, setFilters] = useState<SearchFilters>({}); | ||
| const [page, setPage] = useState(1); | ||
| const [showAvoided, setShowAvoided] = useState(false); | ||
| const [showAvoided, setShowAvoided] = useState(getShowAvoided); |
There was a problem hiding this comment.
Avoid localStorage-backed initial state during first render
Initializing state from getShowAvoided/getViewMode/getRecentSearches in render causes server/client divergence on this Next.js page: server prerender sees no localStorage and renders defaults, while the browser's first render may immediately use persisted values, producing different HTML and hydration warnings or forced client re-render. This regression is introduced by replacing the mount useEffect with lazy initializers, and it affects any user with saved search preferences/history.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR updates the frontend search page to eliminate react-hooks/set-state-in-effect lint warnings by replacing mount/effect-driven state synchronization with lazy useState initialization and render-phase “adjust state during render” trackers (per the React guidance used elsewhere in the codebase).
Changes:
- Load
recentSearches,showAvoided, andviewModevia lazyuseStateinitializers instead of a mountuseEffect. - Replace the
debouncedQuery→submittedQuerysync effect with a render-phase tracker keyed offdebouncedQuery+filters. - Replace the “reset page on query/filter change” effect with a render-phase tracker keyed off
submittedQuery+filters.
| const [showAvoided, setShowAvoided] = useState(getShowAvoided); | ||
| const [showAutocomplete, setShowAutocomplete] = useState(false); | ||
| const [showFilters, setShowFilters] = useState(false); | ||
| const [showSaveDialog, setShowSaveDialog] = useState(false); | ||
| const [autocompleteActiveId, setAutocompleteActiveId] = useState< | ||
| string | undefined | ||
| >(undefined); | ||
| const [recentSearches, setRecentSearches] = useState<string[]>([]); | ||
| const [viewMode, setViewMode] = useState<ViewMode>("grid"); | ||
| const [recentSearches, setRecentSearches] = | ||
| useState<string[]>(getRecentSearches); | ||
| const [viewMode, setViewMode] = useState<ViewMode>(getViewMode); |
There was a problem hiding this comment.
getShowAvoided/getViewMode are now invoked during render via lazy useState initializers. Unlike getRecentSearches (which wraps localStorage access in try/catch), these helpers call globalThis.localStorage.getItem directly; in some browser/privacy contexts localStorage access can throw and would now crash initial render. Consider wrapping the get/set helpers in try/catch (and returning defaults on failure) so render stays resilient.
| const submitKey = `${debouncedQuery}|${JSON.stringify(filters)}`; | ||
| const [prevSubmitKey, setPrevSubmitKey] = useState(submitKey); | ||
| if (submitKey !== prevSubmitKey) { | ||
| setPrevSubmitKey(submitKey); | ||
| const trimmed = debouncedQuery.trim(); |
There was a problem hiding this comment.
submitKey is built via string concatenation with | plus JSON.stringify(filters). Since debouncedQuery is user-entered, it can contain |, which can create key collisions (different query+filters pairs producing the same key) and incorrectly skip the tracker update. Prefer a structured/unambiguous key (e.g., stringify a tuple/object) and also avoid duplicating JSON.stringify(filters) work for both trackers (compute once/memoize).
Resolves 3
react-hooks/set-state-in-effectviolations insrc/app/app/search/page.tsx(Phase 2 of #1063).Changes
useStateinitializers forrecentSearches,showAvoided,viewMode. The helpers are already SSR-safe viaglobalThis.localStorageguards (return[]/false/'grid'when unavailable).debouncedQuerysync effect → Pattern A render-phase tracker with compositesubmitKeyofdebouncedQuery+filters.pageResetKeyofsubmittedQuery+filters.Validation
npx vitest run src/app/app/search/page.test.tsx→ 54/54 passnpx eslint src/app/app/search/page.tsx→ clean (noset-state-in-effect)npx tsc --noEmit→ cleanPhase 2 Progress
Final non-deferred multi-violation file. Phase 2 cumulative after this PR merges: 15/19.
Remaining 4 violations are deferred / require separate authorization (complex async/SSR cases — see CURRENT_STATE.md follow-ups).
Cross-references