mirror of
https://github.com/tiennm99/vngeoguessr.git
synced 2026-10-11 03:13:56 +00:00
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.
This commit is contained in:
1 parent
ae0cc3d798
commit
6d7cb30e5e
7 files changed
+143
-21
No files matched your search
@@ -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.
|
||||
@@ -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`.
|
||||
@@ -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
|
||||
|
||||
@@ -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 <h1>, not a styled <p>: 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 (
|
||||
<div className="flex-1 flex items-center justify-center vn-surface p-6">
|
||||
<div className="text-center space-y-4 max-w-sm animate-fade-in-up">
|
||||
<h1 className="text-2xl font-bold text-foreground">{title}</h1>
|
||||
{/* 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. */}
|
||||
<p className="text-muted-foreground text-sm">{children}</p>
|
||||
{/* One action, deliberately. A "try again" on a deterministically
|
||||
invalid URL is a button guaranteed to reproduce the same page. */}
|
||||
<Button asChild>
|
||||
<Link href={actionHref}>{actionLabel}</Link>
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -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
|
||||
// (<html id="__next_error__">), which does not carry the pre-paint theme
|
||||
// script the root layout puts in <head> -- 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 (
|
||||
<div className="flex-1 flex items-center justify-center vn-surface p-6">
|
||||
<div className="text-center space-y-4 max-w-sm animate-fade-in-up">
|
||||
{/* A real heading, not a styled <p>: 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. */}
|
||||
<h1 className="text-2xl font-bold text-foreground">No such region</h1>
|
||||
<p className="text-muted-foreground text-sm">
|
||||
That link points at a region that doesn't exist. It may have been
|
||||
mistyped or cut short.
|
||||
</p>
|
||||
<Button asChild>
|
||||
<Link href="/">Pick a region</Link>
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
<NotFoundPanel title="No such region" actionLabel="Pick a region" actionHref="/">
|
||||
That link points at a region that doesn't exist. It may have been
|
||||
mistyped or cut short.
|
||||
</NotFoundPanel>
|
||||
);
|
||||
}
|
||||
@@ -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 (
|
||||
<NotFoundPanel title="Page not found" actionLabel="Go to the start" actionHref="/">
|
||||
There's nothing at this address. It may have moved, or the link may
|
||||
have been mistyped.
|
||||
</NotFoundPanel>
|
||||
);
|
||||
}
|
||||
@@ -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();
|
||||
});
|
||||
Reference in new issue
Block a user