fix: React #300 hook-order crash in LinksPanel + ESLint gate#19
Merged
Conversation
Root cause: LinksPanel called useAppStore for linksFilterActive and
toggleLinksFilter AFTER an early return at line 81. On the "happy" path
6 hooks ran; when the last tab closed (selectedFile → null) the early
return fired and only 4 hooks ran. React 19 throws minified error #300
("Rendered fewer hooks than expected") in production; dev only warns,
which is why the bug was only seen in the packaged DMG.
Fix: move both hook calls above the early return so the same 6 hooks
run on every render regardless of state.
Guardrail: add ESLint 9 with eslint-plugin-react-hooks. The
react-hooks/rules-of-hooks rule catches this class of bug statically —
running `npm run lint` on the pre-fix LinksPanel flags both offending
lines as errors. Wire it into a new CI workflow that runs lint + tests
on every PR and push to main, so a regression fails before merge.
Also:
- styleCheck.ts: autofixed `let working` → `const working` (lint
autofix, unrelated to the hook bug)
- eslint.config.mjs: flat config with TypeScript + react-hooks;
exhaustive-deps kept as warning to avoid false-positive churn on
existing code.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Addresses the five findings from Gestalt review (codex SHIP + TDD-critic
RED) in a single follow-up pass. All green: 240 tests pass, 0 lint
errors/warnings, build clean.
LinksPanel regression test
- tests/unit/LinksPanel.test.tsx asserts hook-order stability across
selectedFile/linkGraph null ↔ populated transitions. Verified to
hard-fail on the pre-fix code with React's "Rendered more hooks than
during the previous render" invariant. ESLint is the static defence;
this is the runtime safety net.
E2E regression — the real reason the close-all tests missed this bug
- tests/e2e/close-all-tabs.test.ts:517 looked for
`.segmented-btn:text-is("Links")`. That class only matches the
Settings segmented controls; the Links button uses `.outline-segment`.
The selector returned zero matches, the `count > 0` guard silently
skipped the click, and LinksPanel was never mounted during close-all.
Fixed selector + removed the silent-skip guard so a future class
rename fails the test.
- Console listeners now also collect React-hook-specific warnings
(not just "error" type messages), closing the gap flagged by the
TDD critic.
CI hardening
- .github/workflows/ci.yml now runs `npm run build` (typecheck +
bundle) in addition to lint + unit tests.
Stale-closure bugs flagged by newly-enabled `react-hooks/exhaustive-deps`
- CollapsiblePreview.tsx:~181 — wrap `links` fallback in useMemo so
its reference stabilises across renders (would otherwise thrash the
preamble-HTML useMemo on every render when annotatedTokens.links is
missing).
- CollapsiblePreview.tsx:~294 — add `searchExpanded` to `toggle` deps
(real stale-closure: the callback read a stale value and allowed
fold-state mutation during search-expanded mode).
- FileTree.tsx:~226 — tighten deps on `handleDrop` (add
`dropTargetPath`, drop redundant `node.type`).
- MarkdownPreview.tsx:~791,825 — drop unused `sectionModel` from
export-handler deps.
- Sidebar.tsx:~288 — drop unused `selectFile` from deps and remove
the unused destructure; the handler reads selectFile via
`useAppStore.getState()`.
- eslint.config.mjs: `react-hooks/exhaustive-deps` promoted from
`warn` to `error` now that the backlog is clear.
Housekeeping (dead imports/vars flagged by no-unused-vars)
- src/main/main.ts: drop unused `buildLinkIndex` import
- FileTree.tsx: drop unused `memo` import
- LinksPanel.tsx: drop unused `selectFile` subscription
- MarkdownPreview.tsx: drop unused `toggleStyleCheck` subscription
- Settings.tsx: drop unused `useCallback`/`useState`/`FontId` imports
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The existing "collapsible + links panel + mixed fold states across files" test (now at close-all-tabs.test.ts:571) already covers the React #300 class of bug after the selector fix — but it's 80 lines of mixed setup, so the specific code path being guarded is not obvious at a glance. This adds a dedicated 45-line test that names the scenario unambiguously: open three tabs with inter-file links, mount the Links panel, close all tabs, assert no console errors or React hook warnings. Future readers can delete or move this test as a unit, rather than having to untangle it from the broader fold-state coverage. Full e2e suite: 55 passed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.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
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.
Summary
LinksPanelcalled 4 hooks, early-returned when!linkGraph || !selectedFile, then called 2 moreuseAppStoresubscriptions. When the last tab closed, hook count dropped 6 → 4 and React 19 threw minified error #300 in production (dev only warns).eslint-plugin-react-hooksflat config;rules-of-hooksrule catches this class of bug statically.exhaustive-depspromoted fromwarntoerrorafter clearing the backlog..segmented-btnvs.outline-segmentclass mismatch).ci.ymlworkflow runs lint + typecheck/build + unit tests on every PR and push to main.Why the e2e suite missed this
tests/e2e/close-all-tabs.test.tsalready had a test intended to cover this scenario, but line 517 looked for `.segmented-btn:text-is("Links")` — that class belongs to the Settings segmented controls; the Links button uses `.outline-segment`. The `if (count > 0)` guard silently skipped the click, so LinksPanel was never mounted during close-all. The React #300 bug went undetected for the same reason minimum-coverage gates aren't worth much without a second opinion — a green bar was lying about coverage.Review
Four-round Gestalt multi-AI review (thread 5Ze7NBwV1lGV): codex SHIP (both rounds), gemini SHIP (round 2 after model-routing fix), TDD Critic RED → GREEN, AHA Moment Detector surfaced two reusable lessons about silent selector failures and React dev-mode vs prod divergence.
Files changed
Test plan
🤖 Generated with Claude Code