refactor(hooks): burn down the 35 compiler-era react-hooks findings, rules bind as errors - #459
Merged
Merged
Conversation
…rules bind as errors PR #451 downgraded five react-hooks rules to "warn" with a note to delete the block once the call sites were fixed. This is that burn-down: all 35 findings restructured, the downgrade block deleted, so set-state-in-effect / refs / purity / immutability / preserve-manual-memoization now bind at error severity (verified by mutation probe: a violating file fails lint with exit 1). By class: set-state-in-effect (25) — three recipes, chosen per call site: - Prop/URL→state mirror effects became guarded render-time adjustments (React's "adjusting state when props change" pattern): ProjectAutopilotToggle, ZellijLivePanel, LiveUrlField, ProjectWorkspaceHeader (x2), ProjectCard (capacity re-arm + agent-running transition), ControlPanel (selection reconciliation), CommandPalette (open-reset), LokiWorkspace (?q= prefill, selection seed, transcript switch), use-auto-continue, use-poll, use-project-card-actions. Same commit semantics, one paint earlier. - Async loaders called from mount effects were split into pure fetch cores whose every setState lives in a promise callback (.then/.catch/.finally), with thin event-context wrappers that prime spinners for buttons/timers: ActivityTimeline, PeekTabDrawer (fetchRemotePeek now returns content instead of applying it), PeopleBookPanel, LokiWorkspace reloads, use-control-data (fetchControl core + refresh wrapper). - Values readable at first render moved into useState lazy initializers: CommandPalette recents (sessionStorage), BillingSettings ?billing= notice. MermaidDiagram's matchMedia mirror became a useSyncExternalStore subscription. LogConversationButton clears results in the input's onChange (the actual event) instead of the debounce effect. refs (6): - ProjectOperationsView's frozen row order moved from a ref mutated inside useMemo to state adjusted during render behind the same setKey guard. - queue-list/queue-item-row: editRef existed only to focus the edit textarea after open; the textarea mounts with autoFocus instead and the ref plumbing is deleted. - TerminalView's fontOverrideRef is now mirrored via an effect (declared before the mount effect so declaration-order seeding still precedes the first read), matching the file's existing onLive/onGeometry pattern. purity (1): - ProjectCard no longer calls Date.now() in render; it receives nowS from ControlPanel's per-render clock via buildCardProps, so the card's staleness math now uses the same clock as the snapshots it renders. immutability (1): - BillingSettings navigates to Stripe Checkout via window.location.assign() (method call) instead of assigning location.href. preserve-manual-memoization (2): - TerminalSurface.switchAgent and AskLokiButton.ask captured whole context objects while declaring narrowed deps; the needed field (tabDir / workspaceKey) is now a local, making the manual deps match what the compiler infers. Two targeted eslint-disable-next-line react-hooks/set-state-in-effect remain, each with an inline justification: ProjectCard's auto-reroute automation (once-per-capacity-episode loop guard around a live agent-switching path) and ControlPanel's ?focus= deep-link handler (App Router param changes only surface as re-renders; splitting it would resolve the target twice against possibly-different data). Two unused directives (ready-banner, ShellWorkspace) were removed. Verified: npm run lint (0 errors; 3 pre-existing @next/next location warnings untouched), npx tsc --noEmit, npm run test:unit 124/124. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqKqMnHQHSmkGFfc5t7Rxn
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.
Burn-down for the five compiler-era react-hooks rules that PR #451 downgraded to "warn". All 35 findings are restructured, the warn-downgrade block in
eslint.config.mjsis deleted (as its own comment instructed), and the rules now bind at error severity — verified by mutation probe (a violating scratch component fails lint with exit 1). Thesettings.react.versionpin stays.Per-rule counts
Notable restructures
set-state-in-effect — three recipes, chosen per call site:
.then/.catch/.finally); thin wrappers prime spinners for buttons/timers. ActivityTimeline, PeekTabDrawer (fetchRemotePeekreturns the content instead of applying it), PeopleBookPanel, LokiWorkspace reloads, use-control-data (fetchControlcore +refresh(manual)wrapper).useSyncExternalStore; LogConversationButton clears results in the input's onChange.refs: ProjectOperationsView's frozen row order moved from a ref-inside-useMemo to state adjusted during render behind the same setKey guard (row-freeze semantics preserved). queue-list's
editRefplumbing is deleted — the edit textarea mounts withautoFocus. TerminalView'sfontOverrideRefis mirrored via an effect, matching the file's existing onLive/onGeometry pattern.purity: ProjectCard now receives
nowSfrom ControlPanel's per-render clock viabuildCardPropsinstead of callingDate.now()in render — the card's staleness math uses the same clock as the snapshots it renders.immutability: BillingSettings navigates to Stripe Checkout via
window.location.assign()instead of assigninglocation.href.preserve-manual-memoization: TerminalSurface.switchAgent and AskLokiButton.ask captured whole context objects while declaring narrowed deps; the needed field (
tabDir/workspaceKey) is now a local, so the manual deps match what the compiler infers.Justified per-line disables (2)
ProjectCard.tsxauto-reroute effect — episodic automation reacting to polled session state with ref-tracked loop guards (once per capacity episode); deriving that state during render would rewire the loop-safety of a live agent-switching path.ControlPanel.tsx?focus=deep-link effect — App Router param changes only surface as re-renders, so this effect is the event handler; it resolves the target once, atomically, then selects + highlights + scrolls + clears the params. Splitting it would resolve the target twice against possibly-different data mid-refresh.Two unused directives (ready-banner, ShellWorkspace) were removed.
Verification
npm run lint— 0 errors (3 pre-existing@next/next/no-location-assign-relative-destinationwarnings untouched); mutation probe confirms the five rules fail the lint at error severitynpx tsc --noEmit— cleannpm run test:unit— 124/124npm run format:check— clean🤖 Generated with Claude Code
https://claude.ai/code/session_01WqKqMnHQHSmkGFfc5t7Rxn