Skip to content

Fix production build: RailProps.cardDocumentsByIdentifier needs | undefined - #90

Merged
WilfordGrimley merged 1 commit into
masterfrom
claude/proposal-h-urgent-build-fix-04bam2
Jul 18, 2026
Merged

Fix production build: RailProps.cardDocumentsByIdentifier needs | undefined#90
WilfordGrimley merged 1 commit into
masterfrom
claude/proposal-h-urgent-build-fix-04bam2

Conversation

@WilfordGrimley

Copy link
Copy Markdown

Description

Fixes the npm run build failure blocking deploy-frontend.yml (failed at run #106, before the flag even flipped on, and again at #107 with NEXT_PUBLIC_UNIFIED_DISPLAY_ENABLED=true).

Root cause: DisplayPage.tsx's RailProps.cardDocumentsByIdentifier was typed as { [identifier: string]: CardDocument }. An unrelated same-day PR (task #135, fixing a real crash in BleedOverrideSettings) correctly widened useCardDocumentsByIdentifier()'s own return type to { [identifier: string]: CardDocument | undefined } — a project member's CardDocument genuinely may not have been fetched yet, and the old unwidened type was hiding that possibility from tsc entirely (see cardDocumentsSlice.ts's own comment on the fix). Once both PRs were on master together, passing the (now correctly-typed) hook result into the (still narrowly-typed) RailProps became a real type error — a cross-PR interaction neither branch's own pre-merge CI could have caught in isolation, since each was individually type-correct against its own base.

Fix: widen RailProps.cardDocumentsByIdentifier to match. Every actual field access in DisplayPage.tsx already used ?. defensively (confirmed by reading every call site), so this is a type-annotation-only fix — zero runtime behavior change.

Deviations from spec, with reasoning

None — this is a minimal, targeted fix scoped to exactly the reported failure.

How this was diagnosed (relevant since the first repro attempt gave a false negative)

  1. Pulled the actual failing job logs from deploy-frontend.yml runs Proposal I: DOCS-AS-SITE-SOURCE spec (SPEC + HOLD, no build) #106 and Fix Discord sign-in: remove nested-anchor Nav.Link wrapper around AuthWidget #107 via the GitHub API rather than trusting a local repro's absence of an error — both showed the identical tsc error at DisplayPage.tsx:555:13, and critically Proposal I: DOCS-AS-SITE-SOURCE spec (SPEC + HOLD, no build) #106 failed too, before the flag was ever turned on, which contradicted the initial "the flag flip is the delta" framing enough to warrant checking the actual logs instead of guessing further.
  2. First local repro attempt (a git worktree with node_modules copied over from another checkout, to skip a slow reinstall) built clean — a false negative, because copying node_modules skips npm install's postinstall step, which generates frontend/src/common/generated/keyruneCodepoints.json (gitignored, not committed). Regenerating that file (node scripts/generate-keyrune-assets.js) and confirming the worktree was actually on the failing commit (93059f1b, not a stale earlier one) reproduced the real error.

Test plan

  • NEXT_PUBLIC_UNIFIED_DISPLAY_ENABLED=true npx next build — now succeeds (previously failed with the exact reported error).
  • npx next build (flag off, default) — still succeeds, unaffected.
  • tsc --noEmit — clean.
  • Full Jest suite — 345/345 passing.
  • tests/DisplayPage.spec.ts (7 Playwright tests against next dev) — all still passing, confirming the type-only fix didn't change runtime behavior.
  • eslint on the changed file — clean (no new warnings).

Merge-time checklist

  • Once this merges, deploy-frontend.yml should re-fire automatically and go green (per the owner's note — workflow + variable are already in place).
  • wiki: no page needs updating (internal build-pipeline fix, nothing user-facing changed).

Also adds a standing verification-bar lesson to docs/lessons.md: any flag-gated page needs a real production build with its flag ON (FLAG=true npx next build) before its PR ships — a dev-server Playwright suite never exercises the static-export prerender/type-check path a production build does. Adding this to my own pre-push checks for the remaining Step 2 PRs.


Generated by Claude Code

…low undefined

deploy-frontend.yml #107 (and #106, before the flag even flipped)
failed npm run build with a real tsc error: DisplayPage.tsx's RailProps
declared cardDocumentsByIdentifier as { [id]: CardDocument }, but
useCardDocumentsByIdentifier()'s own return type was correctly widened
to { [id]: CardDocument | undefined } by an unrelated same-day PR
(task #135, fixing a real crash) - a cross-PR interaction neither
branch's own pre-merge CI could have caught alone. Every actual field
access already used `?.`, so this is a type-only fix, no behavior
change.

Verified against the real failure mode, not just a local repro:
confirmed the exact commit (93059f1) and job logs matched before
concluding the diagnosis; regenerated the gitignored keyrune assets
(frontend/src/common/generated/) that a naive worktree-node_modules-copy
repro had silently skipped, which produced a first, unrelated false
build failure. NEXT_PUBLIC_UNIFIED_DISPLAY_ENABLED=true npx next build
now succeeds, and so does the flag-off build (unaffected). Full Jest
suite (345 tests) and tests/DisplayPage.spec.ts (7 tests) still pass.

Adds a standing verification-bar lesson to docs/lessons.md: any
flag-gated page needs a real production build with its flag ON before
shipping, since a dev-server Playwright suite never exercises the
static-export prerender/type-check path a production build does.
@WilfordGrimley
WilfordGrimley merged commit af88d9b into master Jul 18, 2026
8 checks passed
@WilfordGrimley
WilfordGrimley deleted the claude/proposal-h-urgent-build-fix-04bam2 branch July 18, 2026 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants