Files
explorer/docs/todo.md
D. Rimron-Soutter e803274af4 react-bootstrap migration, review & parser fixes
UI / react-bootstrap:
Migrate client components to react-bootstrap (Card, Table, Form,
Alert, Badge, Nav, Button, Spinner, Row, Col): the ZXDB explorers
and detail pages (Labels, Genres, Languages, MachineTypes, Releases,
Entries), TapeIdentifier, home page, Navbar and ThemeDropdown.
Server components (home, zxdb hub, magazines, issues) keep raw
HTML+className — react-bootstrap barrel imports resolve to undefined
under Turbopack in server components. Replace bi bi-* CSS icons with
react-bootstrap-icons. Add aria-labels to search inputs and
visually-hidden captions to data tables.

Code-review remediation (docs/todo.md):
- FileViewer: replace useState-as-effect with a proper useEffect.
- register.service: restore request-level caching of parsed registers.
- middleware: convert .js to .ts, dev-only request logging.
- Extract shared types to src/types/zxdb.ts; add src/server/repo
  barrel for incremental per-domain splitting.
- Extract helpers: parseIdList (params.ts), serialize (serialize.ts),
  buildRegisterSummary/isInfoLine (register_helpers.ts).
- Add loading.tsx skeletons for dynamic ZXDB detail routes.
- generateMetadata + notFound() on entry/release/label detail pages.
- opengraph-image: stable keys; ThemeDropdown: drop hardcoded cookie
  domain; remove unused page.module.css.

Register parser & data:
- Update data/nextreg.txt from upstream tbblue (SpectrumNext FPGA):
  0x04/0x0A/0x0F/0x80/0x81 bit changes, new Issue 5 board id, 0x43
  renamed "Palette Control", 0xF0/0xF8/0xF9/0xFA now "Issues 4 and 5
  Only".
- Add reg_44 custom parser for 0x44 (Palette Value 9-bit): the two
  consecutive writes render as separate "1st write" / "2nd write"
  modes.
- Skip commented-out register headers so the disabled 0xA3 block no
  longer leaks a phantom register.
- Add detailHasContent guard so body-less registers (0xC7/0xCB/0xCF/
  0xFF) and 0xF0's leading blank no longer emit empty tab strips.
- Capture 0xF0's leading "Issues 4 and 5 Only" line as register text.
- Add isIssueRestricted (case-sensitive) to detect the issue badge
  across rewording without flagging per-bit "(issue 5 only)" notes;
  update badge label to "Issues 4 & 5 Only".

claude-opus-4-8@lucy
2026-06-08 22:47:37 +01:00

13 KiB

📋 Next Explorer — Code Review & TODO

Full codebase review performed 2026-03-04. Findings grouped by priority and area.


🔴 Critical

🐛 Bug: FileViewer uses useState as useEffect

File: src/components/FileViewer.tsx:23

useState(() => { ... }) is being abused as a side-effect initializer — it calls fetch() inside useState's initializer function. This works by accident on first render but violates React rules:

  • The initializer can run multiple times in Strict Mode (double-invocation).
  • It never re-runs if url or title props change.
  • Side effects in useState initializers are explicitly discouraged by React.

Fix: Replace with useEffect with proper dependency array on [url, isText].

🐛 Bug: Register service caching is commented out

File: src/services/register.service.ts:14-18

The if (registers.length === 0) guard is commented out, so the file is re-read and re-parsed on every call to getRegisters(). This means every register page load (including the OG image generator and generateMetadata) re-parses the entire nextreg.txt file. The [hex]/page.tsx calls getRegisters() twice (once in generateMetadata, once in the page function), reading the file from disk twice per request.

Fix: Uncomment the caching guard, or better yet, wrap with React cache() for request-level deduplication.

🔒 Security: middleware.js is untyped JavaScript

File: src/middleware.js

The only .js file in the project. It logs every request path to stdout, including potentially sensitive paths. In production this creates noise and potential log injection vectors.

Fix: Convert to TypeScript (.ts). Consider restricting logging to development only, or removing it entirely since Next.js has built-in request logging.


🟠 High Priority

🧱 Architecture: Duplicated type definitions across files

Paged<T>, Item, SearchScope, EntryFacets, and similar types are independently re-declared in:

  • src/hooks/useSearchFetch.ts
  • src/app/zxdb/entries/EntriesExplorer.tsx
  • src/app/zxdb/releases/ReleasesExplorer.tsx
  • src/app/zxdb/labels/LabelsSearch.tsx
  • src/app/zxdb/genres/GenresSearch.tsx

And the EntryDetailData type in EntryDetail.tsx is a near-duplicate of EntryDetail in src/server/repo/zxdb.ts.

Fix: Extract shared types to src/types/zxdb.ts and import everywhere.

🧱 Architecture: Duplicated parseMachineIds / parseIdList helpers

The same ID-parsing logic appears in:

  • src/app/zxdb/entries/page.tsx
  • src/app/zxdb/entries/EntriesExplorer.tsx
  • src/app/zxdb/releases/page.tsx
  • src/app/zxdb/releases/ReleasesExplorer.tsx
  • src/app/api/zxdb/search/route.ts

Fix: Extract to a shared utility (e.g., src/utils/params.ts).

🧱 Architecture: Duplicated buildRegisterSummary logic

[hex]/page.tsx and [hex]/opengraph-image.tsx each have their own version of register-summary-building logic (one returns a string, one returns lines). The isInfoLine filter is duplicated.

Fix: Extract to a shared utility in src/utils/register_helpers.ts.

Performance: Repository file is 800+ lines with no code splitting

File: src/server/repo/zxdb.ts (31,000+ tokens)

This monolithic file contains all DB queries. It's hard to navigate and cannot benefit from tree-shaking at the module level.

Fix: Split into per-domain files: repo/entries.ts, repo/labels.ts, repo/releases.ts, repo/magazines.ts, repo/lookups.ts, etc.

Performance: releases/page.tsx uses JSON.parse(JSON.stringify(...)) for serialization

File: src/app/zxdb/releases/page.tsx:51-63

Uses JSON.parse(JSON.stringify(initial)) to strip non-serializable values. This is a known workaround for Drizzle decimal types. However, it's applied 7 times in the same function.

Fix: Create a serialize() helper, or configure Drizzle's decimal columns to return strings/numbers natively.

🎨 UI Consistency: Mixed raw HTML and react-bootstrap components

Per CLAUDE.md, the project should always use react-bootstrap components. Several pages use raw HTML instead:

File Issue
LabelsSearch.tsx Raw <table>, <input>, <button>, <form> instead of Table, Form.*, Button
LabelDetail.tsx Raw <table>, <input>, <button>, nav tabs using raw <ul>/<li>/<button>
GenresSearch.tsx Raw <table>, <input>, <button>
MachineTypesSearch.tsx Likely same pattern
LanguagesSearch.tsx Likely same pattern
ReleaseDetail.tsx Raw <table> throughout instead of <Table>
zxdb/page.tsx Raw <input>, <select>, <button> in the search form
magazines/page.tsx Raw <table>, local Pagination component instead of shared one
magazines/[id]/page.tsx Raw <table>
issues/[id]/page.tsx Raw <table>
page.tsx (home) Raw <div className="card"> instead of <Card>
TapeIdentifier.tsx Raw <table> for hash display

Fix: Systematically replace with react-bootstrap equivalents to match EntriesExplorer.tsx and RegisterBrowser.tsx patterns.

🎨 UI: Magazines page has its own inline Pagination component

File: src/app/zxdb/magazines/page.tsx:89-117

Defines a local Pagination function instead of using the shared src/components/explorer/Pagination.tsx.

Fix: Use the shared Pagination component.


🟡 Medium Priority

⚛️ React: useSearchFetch onExtra callback may cause infinite loops

File: src/hooks/useSearchFetch.ts:75

The fetch_ callback depends on [endpoint, onExtra]. If the caller doesn't memoize onExtra, this dependency changes every render, creating a new fetch_ reference, which could cascade into effect re-runs.

The EntriesExplorer.tsx correctly useCallback-wraps handleExtra, but this is a fragile contract.

Fix: Store onExtra in a ref instead of including it in the dependency array, or document the requirement clearly.

⚛️ React: Missing notFound() call in entry detail page

File: src/app/zxdb/entries/[id]/page.tsx:13-15

When getEntryById returns null, the page still renders with status 200 — the client component shows an "alert-warning" div. This means:

  • Search engines index a 200 page with "Not found" content.
  • No proper 404 HTTP status.

Fix: Call notFound() in the server component when data is null, like magazines/[id]/page.tsx does.

⚛️ React: Same issue for release detail page

File: src/app/zxdb/releases/[entryId]/[releaseSeq]/page.tsx

Same pattern — no notFound() when data is null.

⚛️ React: RegisterBrowser disables exhaustive-deps without justification

File: src/app/registers/RegisterBrowser.tsx:90-91

The eslint-disable-next-line react-hooks/exhaustive-deps on the searchParams sync effect excludes searchTerm from deps, which could lead to stale closures if searchParams changes while searchTerm is mid-update.

📦 Metadata: Entry/release/label detail pages lack dynamic metadata

Files:

  • src/app/zxdb/entries/[id]/page.tsx — static metadata = { title: "ZXDB Entry" }
  • src/app/zxdb/releases/[entryId]/[releaseSeq]/page.tsx — static metadata = { title: "ZXDB Release" }
  • src/app/zxdb/labels/[id]/page.tsx — static metadata = { title: "ZXDB Label" }

These should use generateMetadata to include the entry/release/label title for SEO and social sharing, similar to how registers/[hex]/page.tsx does it.

File: src/components/ThemeDropdown.tsx:16

document.cookie = `${name}=...; Domain=specnext.dev`;

This hardcoded domain means the theme cookie won't work on localhost or any non-specnext.dev domain during development.

Fix: Remove the Domain= attribute (let it default to current host), or conditionally set it based on environment.

Performance: No loading.tsx or Suspense boundaries

None of the route segments define loading.tsx files. For force-dynamic pages (entries, releases, labels, genres, languages, machinetypes), users see a blank page or frozen UI while the server fetches from MySQL.

Fix: Add loading.tsx with skeleton/spinner states for ZXDB routes.

Performance: opengraph-image.tsx calls getRegisters() for every OG image

File: src/app/registers/[hex]/opengraph-image.tsx:132-136

Loads ALL registers from disk just to find one. With the caching fix above this becomes a non-issue, but currently it's reading and parsing the full file for each image request.

🔒 Security: Download API path traversal protection should normalize before joining

File: src/app/api/zxdb/download/route.ts:27-28

The current protection path.normalize(path.join(baseDir, filePath)) is correct, but the check should also reject paths containing .. before join for defense-in-depth.

📝 DX: parseNextReg() is async but does no async work

File: src/utils/register_parser.ts:49

parseNextReg() is declared async and returns Promise<Register[]>, but the function body is entirely synchronous. This forces callers to await unnecessarily.

Fix: Remove async and return Register[] directly.


🟢 Low Priority

File: src/components/Navbar.tsx:14-16

For consistency with react-bootstrap patterns, use <Nav.Link as={Link} href="..."> instead of raw <Link className="nav-link">.

🎨 UI: Home page uses bi bi-* CSS classes instead of react-bootstrap-icons

File: src/app/page.tsx:12,29

Uses <span className="bi bi-collection"> instead of the react-bootstrap-icons package that's used elsewhere.

🎨 UI: TapeIdentifier uses bi bi-* CSS classes

File: src/app/zxdb/TapeIdentifier.tsx

Same issue — uses Bootstrap icon CSS classes instead of react-bootstrap-icons components.

🎨 UI: Inconsistent "not found" patterns

Some pages use notFound() (magazines, issues), others render inline alerts (entries, releases, labels). This creates inconsistent UX.

⚛️ React: buildRegisterSummaryLines in OG image could use better key strategy

File: src/app/registers/[hex]/opengraph-image.tsx:174,188

Uses key={line} which will produce duplicate keys if two lines have identical text.

📝 DX: Commented-out code in register parsers

Files:

  • src/utils/register_parsers/reg_default.ts:13,106,122-126 — commented footnoteTarget variable and valueMatch block
  • src/services/register.service.ts:14,18 — commented caching guard

Fix: Remove dead code or convert to tracked TODOs.

📝 DX: app/page.module.css exists but is never imported

Check if this file has any content; if empty/unused, remove it.

🧪 Testing: Zero test coverage

No test files exist (*.test.ts, *.spec.ts, __tests__/). Key areas that would benefit:

  1. Register parser (parseNextReg, parseDescriptionDefault, parseDescriptionF0) — complex parsing logic with edge cases.
  2. API route input validation (Zod schemas).
  3. useSearchFetch hook behavior (cancellation, race conditions).
  4. computeMd5 correctness.
  5. Component rendering for entry detail, release detail.

Accessibility: Search inputs lack proper labeling

Several search inputs use placeholder as the only label (no associated <label> or aria-label):

  • zxdb/page.tsx search form
  • Various filter sidebars

Accessibility: Tables lack <caption> elements

Data tables throughout the ZXDB explorer have no <caption>, making it harder for screen readers to understand table purpose.

Performance: EntriesExplorer and ReleasesExplorer have very similar structures

Both follow the same pattern: sidebar with filters, table results, pagination. They share about 60% structural similarity. Consider extracting a shared SearchExplorer wrapper that accepts column definitions and filter config.

📦 Bundle: import * as Icon from 'react-bootstrap-icons'

File: src/app/registers/RegisterDetail.tsx:8, src/components/ThemeDropdown.tsx:4

Importing the entire icon library. While tree-shaking should handle this, named imports are safer and make dependencies explicit.

Fix: import { Wikipedia, Link45deg, CodeSlash } from 'react-bootstrap-icons'

📝 DX: CLAUDE.md structure tree is outdated

The project tree in CLAUDE.md doesn't reflect the current file structure (missing hooks/, components/explorer/, ZXDB pages, API routes, server/, etc.).


🗺️ Feature Ideas (non-bugs)

  • Search debouncingRegisterBrowser updates the URL on every keystroke. Consider debouncing the URL update (keep instant local filtering).
  • Entry detail OG images — Register pages have OG images; ZXDB entry pages do not.
  • Keyboard navigation — Add keyboard shortcuts for pagination (left/right arrows).
  • Back-to-top button — Long entry detail pages would benefit.
  • Error boundaries — No error.tsx files exist for graceful error recovery in route segments.
  • Rate limiting — API routes have no rate limiting for the search endpoints.