refactor(hooks): adopt eslint-plugin-react-hooks v6 rules (Next 16) - #115
Closed
catomean wants to merge 1 commit into
Closed
refactor(hooks): adopt eslint-plugin-react-hooks v6 rules (Next 16)#115catomean wants to merge 1 commit into
catomean wants to merge 1 commit into
Conversation
Re-enables the three React-Compiler-era rules deferred in #114 (set-state-in-effect, error-boundaries, immutability) — all now ON — and resolves every violation with a real fix or a justified, documented exception. error-boundaries (12 → 0, real fix): - app/blog/[slug]/page.tsx wrapped its whole render in try/catch that only re-threw. Removed it: notFound()/render errors now propagate to Next's not-found.tsx / error.tsx as intended (the catch also mislogged notFound()). immutability (5 → 0, real fix): - The auth pages assigned window.location.href during render. Moved each redirect into a useEffect (above the early returns for rules-of-hooks), preserving the full-page navigation while keeping render pure. signin, signup, forgot-password, verify-email (×2). set-state-in-effect (13 → 0, justified inline disables): - Every case is a legitimate effect use — external-system sync (Supabase auth, API loads), browser-only hydration (localStorage, client-mount), or seeding editable state from async-loaded data. Forcing them off-effect would add complexity or risk behaviour changes to auth/data code, so each carries an eslint-disable-next-line with a specific reason. The rule stays ON to catch genuinely-avoidable derived-state-in-effect in future code. Verified: eslint . (0 problems), prettier, tsc --noEmit, next build, jest 227 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
Author
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
1 participant
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.
Follow-up to #114. Re-enables the three React-Compiler-era rules that #114 deferred —
set-state-in-effect,error-boundaries,immutability— all now ON — and resolves every one of the 30 violations with a real fix or a justified, documented exception. Stacks on #114 (basedeps/next-16); merge order: #111 → #114 → this.Real fixes (17 violations → genuine code improvements)
error-boundaries(12, one file) —app/blog/[slug]/page.tsxwrapped its entire render in atry/catchthat only re-threw. Removed it:notFound()and render errors now propagate to Next'snot-found.tsx/error.tsxas intended. (The catch also spuriously loggednotFound()as an error.)immutability(5, auth pages) — the auth pages assignedwindow.location.hrefduring render. Moved each redirect into auseEffect(placed above the early returns to satisfy rules-of-hooks), which keeps render pure while preserving the full-page-navigation behavior. Files: signin, signup, forgot-password, verify-email (×2).Justified exceptions (13
set-state-in-effect)Every one is a legitimate effect use — the rule is a heuristic that false-positives on them:
lib/auth), conversation/dashboard API loads (useDocumentChat,useDashboardStats,ConversationList), recovery-session validation (reset-password)localStorage(useLocalStorage), client-mount detection (Comments), client-only image-URL resolution that readswindow.location(MDXComponents)Forcing these off-effect would add complexity or risk behavior changes to auth/data code, so each carries an
eslint-disable-next-linewith a specific reason. The rule stays ON so genuinely-avoidable derived-state-in-effect fails CI in future code — strictly better than leaving it off.Verification
eslint .— 0 problems (all three rules enabled)prettier --checktsc --noEmitnext build(Turbopack, all pages)jest— 227 passed🤖 Generated with Claude Code