From 6d7cb30e5ecb61b45244d79f4eb437d587480a97 Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Sun, 6 Sep 2026 16:50:15 +0700 Subject: [PATCH] fix(404): give every not-found route a way out, in the right theme There was no app-wide not-found route, so any unmatched path got Next's stock page: unstyled text over the full-bleed background, no footer, and no link back -- its own full-height wrapper pushes the footer off screen. Both 404s now share NotFoundPanel, since they say the same three things and only the wording differs: what is missing, why it probably happened, and the one way out. One action each, deliberately -- a "try again" on a deterministically invalid URL is a button guaranteed to reproduce the same page. The region 404 is a client component for one reason: a thrown notFound() is served from Next's error shell, which carries none of the root layout's pre-paint theme script, so a dark-theme visitor got a white page permanently. Re-applying on mount costs a brief light flash on a rare page and fixes the palette. The app-wide route needs none of this -- an unmatched path prerenders inside the root layout, where the script runs. Both are covered by e2e now, including the theme, since neither failure mode is visible from a status code. Also records the Windows ISR case-collision finding in the debugger's agent memory: a local next start case-folds cache keys on NTFS, so redirect and dynamicParams behaviour cannot be verified from a local production build. The region route no longer redirects, so nothing trips it today, but the verification guidance outlives this change. --- .claude/agent-memory/debugger/MEMORY.md | 1 + .../debugger/windows-isr-case-collision.md | 39 +++++++++++++++++ docs/project-structure.md | 6 ++- src/app/components/NotFoundPanel.js | 37 ++++++++++++++++ src/app/game/[region]/not-found.js | 43 ++++++++++--------- src/app/not-found.js | 14 ++++++ tests/e2e/routing.spec.js | 24 +++++++++++ 7 files changed, 143 insertions(+), 21 deletions(-) create mode 100644 .claude/agent-memory/debugger/MEMORY.md create mode 100644 .claude/agent-memory/debugger/windows-isr-case-collision.md create mode 100644 src/app/components/NotFoundPanel.js create mode 100644 src/app/not-found.js diff --git a/.claude/agent-memory/debugger/MEMORY.md b/.claude/agent-memory/debugger/MEMORY.md new file mode 100644 index 0000000..ab768f2 --- /dev/null +++ b/.claude/agent-memory/debugger/MEMORY.md @@ -0,0 +1 @@ +- [Windows ISR case collision](windows-isr-case-collision.md) — local next start case-folds cache keys; verify redirect/dynamicParams with next dev or a Vercel preview, not a Windows production build. diff --git a/.claude/agent-memory/debugger/windows-isr-case-collision.md b/.claude/agent-memory/debugger/windows-isr-case-collision.md new file mode 100644 index 0000000..4db4797 --- /dev/null +++ b/.claude/agent-memory/debugger/windows-isr-case-collision.md @@ -0,0 +1,39 @@ +--- +name: windows-isr-case-collision +description: next start on this Windows/NTFS box corrupts ISR cache for case-variant dynamic-segment redirects — do not trust local production-build smoke tests for this +metadata: + type: project +--- + +Proven 2026-09-06 while investigating `feat/game-region-path-segment` +(`src/app/game/[region]/page.js`): this machine's NTFS filesystem is +case-insensitive. When a Next 16 App Router page under `generateStaticParams` ++ default `dynamicParams: true` gets an on-demand request whose dynamic +segment differs only in case from an already-prebuilt param (e.g. +`/game/la` vs. the prebuilt `/game/LA`), the ISR disk cache lookup +case-folds and serves the prebuilt file directly — bypassing the page's own +logic entirely (here, a lowercase-to-canonical `redirect()`). Worse, the +background revalidation this triggers then overwrites the *canonical* +page's cache file with a cached `redirect()` response that is missing its +`Location` header, permanently breaking the canonical URL for that server +process (confirmed: `200` on first case-variant hit, then `307` with no +`Location` header on both casings afterward, `ETag` identical). + +**Why:** filesystem-level case folding is Windows/NTFS-specific; Vercel +(Linux, its own cache-key store) should not have this collision, so it did +not block shipping — but any redirect/dynamicParams acceptance criterion +tested via `npm run build:check && next start` **on this machine** is +unreliable and order-dependent, not a real signal. + +**Resolved in the code:** the region route no longer redirects at all — slugs +are lowercase and any casing serves directly — so there is no cached redirect +on a prerendered route to corrupt. The verification guidance below still +applies to any future route that does redirect. + +**How to apply:** when a task asks to verify redirect or dynamicParams +behavior for a Next.js App Router dynamic segment locally, either use `next +dev` (no ISR cache, always re-executes) or verify against an actual Vercel +preview / Linux CI, not a local Windows `next start`. If a "smoke test looks +broken after a build" surprise recurs for a route with case-variant dynamic +segments, check this first before assuming a code regression. Full writeup: +`plans/reports/debugger-260906-1523-region-path-runtime-edges.md`. diff --git a/docs/project-structure.md b/docs/project-structure.md index b7db2fa..cb4dd7a 100644 --- a/docs/project-structure.md +++ b/docs/project-structure.md @@ -28,10 +28,14 @@ Next.js 16 App Router structure: - `favicon.ico` - Site favicon #### Game Pages +- `not-found.js` - The app-wide 404 for any unmatched path +- `components/NotFoundPanel.js` - Shared body of both 404s - `game/[region]/page.js` - The game screen for one region (`/game/tphcm`). Server Component: validates the slug, prerenders one page per region, 404s an unknown one -- `game/[region]/not-found.js` - The 404 for an unknown region code +- `game/[region]/not-found.js` - The 404 for an unknown region code. A client + component only so it can re-apply the theme: a thrown `notFound()` is served + from Next's error shell, which carries no pre-paint theme script - `game/page.js` - Redirects the legacy `?region=` / `?location=` links to `/game/{slug}`; a region-less `/game` goes to the country round - `credits/page.js` - Data sources, licenses, and open-source credits diff --git a/src/app/components/NotFoundPanel.js b/src/app/components/NotFoundPanel.js new file mode 100644 index 0000000..e9652d6 --- /dev/null +++ b/src/app/components/NotFoundPanel.js @@ -0,0 +1,37 @@ +import Link from 'next/link'; +import { Button } from '@/components/ui/button'; + +/** + * The shared body of every 404 in the app. + * + * Both not-found routes say the same three things -- what is missing, why it + * probably happened, and the one way out -- so the layout, spacing and the + * heading level live here and only the wording differs. + * + * A real

, not a styled

: a not-found route owns the whole document, + * unlike GameClient's in-place error panel, which sits inside a page that + * already has its own heading. + * @param {Object} props + * @param {string} props.title Short statement of what is missing. + * @param {React.ReactNode} props.children The explanation beneath it. + * @param {string} props.actionLabel Text for the single action. + * @param {string} props.actionHref Where that action goes. + */ +export default function NotFoundPanel({ title, children, actionLabel, actionHref }) { + return ( +

+
+

{title}

+ {/* text-muted-foreground on this size sits near the AA floor; the + explanation is supporting text and the heading and action both + clear it comfortably. */} +

{children}

+ {/* One action, deliberately. A "try again" on a deterministically + invalid URL is a button guaranteed to reproduce the same page. */} + +
+
+ ); +} diff --git a/src/app/game/[region]/not-found.js b/src/app/game/[region]/not-found.js index 569d8f2..7c632d4 100644 --- a/src/app/game/[region]/not-found.js +++ b/src/app/game/[region]/not-found.js @@ -1,29 +1,32 @@ -import Link from 'next/link'; -import { Button } from '@/components/ui/button'; +"use client"; + +import { useEffect } from 'react'; +import NotFoundPanel from '../../components/NotFoundPanel'; +import { applyTheme, getStoredTheme } from '../../../lib/theme'; // Reached when the [region] page rejects a code that is not in the tree -- -// a typo'd or truncated shared link, mostly. Next's stock 404 is a dead end, -// and the one thing this visitor needs is the way to a region that does -// exist, so the page is mostly that link. +// a typo'd or truncated shared link, mostly. // // Deliberately says nothing about which codes are valid: there are 85 of them // and the picker is a better answer than a list. export default function RegionNotFound() { + // A thrown notFound() is served from Next's own error shell + // (), which does not carry the pre-paint theme + // script the root layout puts in -- so a dark-theme visitor would get + // this page in the light palette, permanently. Re-apply once mounted. + // + // This is why the file is a client component; nothing else here needs to be. + // The cost is a light flash before hydration on a page that is already rare. + // The app-wide src/app/not-found.js needs none of this: an unmatched path + // prerenders inside the root layout, where the script does run. + useEffect(() => { + applyTheme(getStoredTheme()); + }, []); + return ( -
-
- {/* A real heading, not a styled

: a not-found page owns the whole - document, unlike GameClient's in-place error panel, which sits - inside a page that already has its own h1. */} -

No such region

-

- That link points at a region that doesn't exist. It may have been - mistyped or cut short. -

- -
-
+ + That link points at a region that doesn't exist. It may have been + mistyped or cut short. + ); } diff --git a/src/app/not-found.js b/src/app/not-found.js new file mode 100644 index 0000000..668d313 --- /dev/null +++ b/src/app/not-found.js @@ -0,0 +1,14 @@ +import NotFoundPanel from './components/NotFoundPanel'; + +// The app-wide 404, for any path no route claims. Without it Next serves its +// stock page, which paints unstyled text over the full-bleed background with +// no footer and no way out -- its own full-height wrapper pushes the footer +// off screen. +export default function NotFound() { + return ( + + There's nothing at this address. It may have moved, or the link may + have been mistyped. + + ); +} diff --git a/tests/e2e/routing.spec.js b/tests/e2e/routing.spec.js index 53b000d..8f227ec 100644 --- a/tests/e2e/routing.spec.js +++ b/tests/e2e/routing.spec.js @@ -122,3 +122,27 @@ test('serves a real region with no imagery instead of 404ing it', async ({ page expect(response.status()).toBe(200); expect(landedAt(page)).toBe(`/game/${regionSlug(code)}`); }); + +test('the region 404 offers a way out, in the visitor\'s theme', async ({ page }) => { + // A thrown notFound() is served from Next's error shell, which carries none + // of the root layout's pre-paint theme script -- so without the re-apply in + // not-found.js this page renders light for a dark-theme visitor. + await page.emulateMedia({ colorScheme: 'dark' }); + await page.addInitScript(() => window.localStorage.setItem('vngeoguessr_theme', 'dark')); + + const response = await page.goto('/game/notaregion'); + expect(response.status()).toBe(404); + await expect(page.getByRole('heading', { name: 'No such region' })).toBeVisible(); + await expect(page.getByRole('link', { name: 'Pick a region' })).toBeVisible(); + await expect(page.locator('html')).toHaveClass(/dark/); +}); + +test('an unmatched path gets the app-wide 404, not a bare Next page', async ({ page }) => { + const response = await page.goto('/nosuchpath'); + expect(response.status()).toBe(404); + await expect(page.getByRole('heading', { name: 'Page not found' })).toBeVisible(); + await expect(page.getByRole('link', { name: 'Go to the start' })).toBeVisible(); + // The footer proves it rendered inside the root layout: Next's stock page + // pushes it off screen with its own full-height wrapper. + await expect(page.getByText('Made by')).toBeVisible(); +});