mirror of
https://github.com/tiennm99/vngeoguessr.git
synced 2026-10-11 03:13:56 +00:00
docs: codify the conventions the code follows, drop the unused tabs dependency
docs/development.md states the rules the code now enforces or keeps: server-only modules and the import-graph test (which now also forbids the daily pick and the region locator from client bundles), route decides best-effort and the library never swallows, typed failure kinds mapped to statuses, claim before write, one module per storage concern through the guarded store, useEffectEvent for imperative callbacks, unprefixed logical keys, three fixed region levels, and the response contract. The parameter rule now says what it always meant: React components destructure props. @radix-ui/react-tabs had no component using it. Stale lines about tabs, pagination, a leaderboard migration and a sub-second suite are corrected.
This commit is contained in:
1 parent
9f663af460
commit
11bfa59377
10 files changed
+129
-94
No files matched your search
@@ -25,7 +25,7 @@ For detailed information about this project, refer to the documentation files in
|
||||
|
||||
**Key Guidelines:**
|
||||
- **JavaScript Only**: No TypeScript files (.ts, .tsx)
|
||||
- **Function Parameters**: Use individual parameters, not object destructuring
|
||||
- **Function Parameters**: Use individual parameters, not object destructuring, in library functions, scripts and route helpers. React components take a single props object and destructure it, as every component here does
|
||||
- **File Modifications**: Only modify source code, `/docs`, and `/plans`
|
||||
- **Generated Data**: `src/data/` is build output; the panorama index lives in
|
||||
Neon Postgres, seeded from gitignored `data-build/panos/` artifacts. Change
|
||||
|
||||
+53
-5
@@ -23,9 +23,55 @@
|
||||
- All components and utilities should be .js or .jsx files
|
||||
|
||||
### Function Parameters
|
||||
- All functions should use **individual parameters** instead of object destructuring
|
||||
- Use `function(param1, param2)` instead of `function({param1, param2})`
|
||||
- This applies to React components, utility functions, and API handlers
|
||||
- Library functions, scripts and route helpers use **individual parameters**
|
||||
instead of object destructuring: `function(param1, param2)`, not
|
||||
`function({param1, param2})`
|
||||
- React components are the exception: they take one props object and
|
||||
destructure it, which is what every component here does and what React
|
||||
expects
|
||||
|
||||
### Conventions the code follows
|
||||
Each of these is enforced by a test or a lint rule where one exists; the rest
|
||||
are the shape the code has and new code should keep.
|
||||
|
||||
- **Server-only modules say so** in their header comment, and the import-graph
|
||||
test in `tests/regions.test.js` keeps `src/lib/regions.js` from reaching
|
||||
`pano-index.js`, `pano-db.js` or `daily.js`. Anything that touches exact
|
||||
panorama coordinates is server-only.
|
||||
- **The route decides what is best-effort; the library never swallows.** A
|
||||
library function throws on failure. A route wraps the writes it can afford
|
||||
to lose (recent-location history, distance records, statistics) in a small
|
||||
`...OrIgnore` / `...OrNone` helper with a comment saying why, and lets the
|
||||
load-bearing write (the session, the score) fail loudly.
|
||||
- **Failure kinds are classes, not message prefixes**: `DryPoolError` and
|
||||
`UpstreamError` in `src/lib/errors.js`. A route maps them to statuses (404
|
||||
for a dry pool, 502 for an upstream failure); nothing string-matches an
|
||||
error message.
|
||||
- **Claim before write.** A session is consumed with an atomic `DEL` before any
|
||||
board is written, and the fan-out after it settles per level rather than
|
||||
all-or-nothing, because nothing after the claim can be retried.
|
||||
- **One module per browser-storage concern**, all reading through
|
||||
`src/lib/storage.js` (never throws, notifies watchers) and rendered through
|
||||
`useStoredValue` in `src/lib/use-stored-value.js`. No component seeds state
|
||||
from storage in an effect; the `react-hooks/set-state-in-effect` rule is an
|
||||
error.
|
||||
- **Callback props reach imperative handlers through `useEffectEvent`**, not
|
||||
refs written during render; the `react-hooks/refs` rule is an error. The one
|
||||
data ref written in render (`CoverageMap.js`) carries an inline disable and
|
||||
its reason.
|
||||
- **Logical Redis keys are unprefixed** and the adapter in `src/lib/upstash.js`
|
||||
applies `KEY_PREFIX`. A new Redis command means a new adapter function and a
|
||||
matching method on `tests/fake-upstash-redis.js`.
|
||||
- **Three region levels, fixed**: country, province, district. Codes are
|
||||
uppercase inside the app and lowercase in URLs (`regionSlug`); players see
|
||||
`regionName()`, the accented form.
|
||||
- **`URLSearchParams.get` is read with `||`, not `??`**: `?region=` yields
|
||||
`''`, which must mean "absent".
|
||||
- **API responses are `{ success, ... }` on success and
|
||||
`{ success: false, error, reason? }` on failure**, with a real HTTP status.
|
||||
`/api/guess` returns `gameResult` alone; the e2e stubs in
|
||||
`tests/e2e/helpers.js` must carry every key a route emits, and
|
||||
`tests/e2e-stub-contract.test.js` checks that they do.
|
||||
|
||||
### File Modification Policy
|
||||
- **Only modify source code files**, documentation (/docs), and plans (/plans)
|
||||
@@ -97,13 +143,15 @@ Restore with
|
||||
|
||||
Tests live in `tests/` and cover the logic in `src/lib/` (scoring and distance,
|
||||
the Upstash key adapter, game sessions, the region tree, the panorama indexes
|
||||
and the leaderboards), the API routes, and the leaderboard migration. Run
|
||||
and the leaderboards), the API routes, the district-assignment and env-loading
|
||||
script helpers, and the e2e stubs' agreement with the routes. Run
|
||||
`npm test` after changing anything under `src/lib/` or `src/data/`.
|
||||
|
||||
The suite runs against two backing stores, from one set of test files:
|
||||
|
||||
- `npm test` uses `tests/fake-upstash-redis.js`, an in-memory stand-in mocked in
|
||||
at the `@upstash/redis` boundary. No service, no Docker, well under a second.
|
||||
at the `@upstash/redis` boundary, and PGlite for Postgres. No service, no
|
||||
Docker; about 25 seconds for the whole suite, most of it PGlite start-up.
|
||||
This is the default.
|
||||
- `npm run test:integration` runs the same files against a real Redis. Two of
|
||||
them skip: one asserts a response shape only an older SDK produces, and one
|
||||
|
||||
+7
-3
@@ -23,7 +23,10 @@
|
||||
from `/images?bbox=` search, which returns HTTP 500 in exactly the dense
|
||||
districts the game wants to play. See the header of `src/lib/mapillary.js`
|
||||
- **Runtime cost is one lookup**: `fetchPanoramaById` resolves a chosen id in
|
||||
~230ms; a couple of alternates are tried in case an image was deleted upstream
|
||||
~230ms; a couple of alternates are tried in case an image was deleted
|
||||
upstream, within an eight-second budget for the whole draw. A region with
|
||||
nothing left to show is a 404 with the coverage message; Mapillary not
|
||||
answering is a 502, never reported as missing coverage
|
||||
- **Country draws pick a province first**, uniformly, so Vietnam rounds are not
|
||||
97% Ha Noi and Ho Chi Minh by panorama count
|
||||
- **No immediate repeats**: the last 50 panoramas a player was shown are
|
||||
@@ -86,8 +89,9 @@
|
||||
- **Single-use sessions**: the session is claimed with an atomic `DEL` before any
|
||||
score is written, so a replayed or concurrent submit scores exactly once.
|
||||
After the claim the score fan-out settles per level: the levels that wrote
|
||||
are returned, `partial: true` marks a level that did not, and only a round
|
||||
where no level wrote is reported as unsaved. The failure carries a `reason` (`session-expired`, `session-consumed`,
|
||||
are returned in `gameResult.levels`, `gameResult.partial` marks a level that
|
||||
did not, and only a round where no level wrote is reported as unsaved. The
|
||||
response is `gameResult` alone. The failure carries a `reason` (`session-expired`, `session-consumed`,
|
||||
`invalid-guess`, `invalid-username`, `invalid-request`) and the result dialog
|
||||
words each one differently, so an expired round is not reported as a failed
|
||||
write
|
||||
|
||||
+1
-1
@@ -139,5 +139,5 @@ points added at every level are the same number.
|
||||
### 10. Continue or Exit
|
||||
- Option to start next round with new session and location in the same region
|
||||
- Option to return to the region picker and choose somewhere else
|
||||
- Option to view full leaderboard with pagination
|
||||
- Option to view the leaderboards (top 200 per board)
|
||||
- Redis session cleanup ensures fresh start for each round
|
||||
@@ -63,7 +63,10 @@ Next.js 16 App Router structure:
|
||||
#### React Components (`src/app/components/`)
|
||||
- `AppBackground.js` - The key art (`public/bg.png`) on one fixed layer under
|
||||
every page, served through `next/image`
|
||||
- `GameClient.js` - Main game client component
|
||||
- `GameClient.js` - The round lifecycle: fetch, epochs, prefetch, submit,
|
||||
daily replay. Layout lives in the components it renders
|
||||
- `GameHeader.js` - The game screen's app bar, presentational
|
||||
- `PlaceName.js` - A Vietnamese place name marked `lang="vi"`
|
||||
- `LeafletMap.js` - Interactive map for guess placement
|
||||
- `PanoramaViewer.js` - 360 degree street view display; owns the Mapillary
|
||||
attribution and a `topBarSlot` for host chrome sharing that row
|
||||
@@ -100,7 +103,6 @@ the shadcn CLI when a screen needs them, rather than keeping unused ones around.
|
||||
- `label.jsx` - Form labels
|
||||
- `select.jsx` - Grouped select (region picker)
|
||||
- `skeleton.jsx` - Loading skeletons
|
||||
- `tabs.jsx` - Tab navigation
|
||||
|
||||
### Generated Data (`src/data/`)
|
||||
Both directories are build output. Do not hand-edit; see *Rebuilding the
|
||||
@@ -150,6 +152,13 @@ Neon Postgres, which is what the app queries at runtime.
|
||||
- `region-locate.js` - **Server-side only.** Which region a map point falls in,
|
||||
from the generated boundaries; feeds the result dialog's region-hit line
|
||||
- `debug-access.js` - The production gate on `/api/debug/*`
|
||||
- `errors.js` - `DryPoolError` and `UpstreamError`, the two ways a draw fails
|
||||
- `storage.js` - The one place localStorage is touched: guarded read, write,
|
||||
and change notification across this tab and others
|
||||
- `use-stored-value.js` - **Client-side only.** Renders a stored value via
|
||||
useSyncExternalStore
|
||||
- `first-round-hint.js` - Whether the how-to-play hint has been seen
|
||||
- `geo-search.js` - Client-safe region and street search for the guess map
|
||||
- `share.js` - Client-safe share text for a round and the share-sheet call
|
||||
- `upstash.js` - Upstash Redis REST client adapter with multi-tenant key prefix
|
||||
- `theme.js`, `use-count-up.js` - Theme persistence and a count-up hook
|
||||
@@ -170,6 +179,10 @@ Each carries a header comment with its flags and its cost.
|
||||
(`npm run leaderboard:export`; run weekly by a GitHub Actions workflow)
|
||||
- `lib/assign-districts.mjs` - District assignment shared by the two pano scripts
|
||||
- `lib/pano-schema.mjs` - Panorama table DDL shared by the seed and the tests
|
||||
- `lib/env.mjs` - The `.env` parser and loader every script shares
|
||||
- `lib/region-config.mjs` - The hand-edited region configuration the boundary
|
||||
builder reads (input data, distinct from the generated tree)
|
||||
- `lib/barrel.mjs`, `lib/paths.mjs` - Barrel writer and output paths
|
||||
|
||||
## Tests (`tests/`)
|
||||
Vitest, mostly one file per `src/lib/` module, plus a route test for
|
||||
|
||||
+2
-2
@@ -58,7 +58,7 @@
|
||||
|
||||
## UI Components & Styling
|
||||
- **shadcn/ui**: Complete component library with "new-york" style
|
||||
- **Radix UI**: Headless component primitives -- dialog, label, slot, tabs, plus
|
||||
- **Radix UI**: Headless component primitives -- dialog, label, slot, plus
|
||||
accordion (province expansion) and select (region picker)
|
||||
- **Lucide React**: Icon library
|
||||
- **class-variance-authority**: Component variant management
|
||||
@@ -66,7 +66,7 @@
|
||||
|
||||
## Testing
|
||||
- **Vitest**: Test runner for the logic in `src/lib/`, the API routes, the
|
||||
generated region data, and the leaderboard migration
|
||||
generated region data, and the pipeline script helpers
|
||||
- **In-memory Upstash fake**: Default Redis backing store, no service required
|
||||
- **PGlite**: In-process Postgres (WASM) mocked in at the
|
||||
`@neondatabase/serverless` boundary, so the panorama queries run against real
|
||||
|
||||
Generated
-79
@@ -15,7 +15,6 @@
|
||||
"@radix-ui/react-label": "^2.1.11",
|
||||
"@radix-ui/react-select": "^2.3.7",
|
||||
"@radix-ui/react-slot": "^1.3.0",
|
||||
"@radix-ui/react-tabs": "^1.1.17",
|
||||
"@turf/turf": "^7.4.0",
|
||||
"@upstash/redis": "^1.38.3",
|
||||
"@vercel/analytics": "^1.5.0",
|
||||
@@ -1935,39 +1934,6 @@
|
||||
}
|
||||
}
|
||||
},
|
||||
"node_modules/@radix-ui/react-roving-focus": {
|
||||
"version": "1.1.19",
|
||||
"resolved": "https://registry.npmjs.org/@radix-ui/react-roving-focus/-/react-roving-focus-1.1.19.tgz",
|
||||
"integrity": "sha512-V9jI6hDjT7l3jsCQD9bLNvDLM3tH/gdbOTp7Tefp3hbbgCGQoK7tUvrWiRlcoBHIZ809ElXwNQwVo0B98LuTXQ==",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"@radix-ui/primitive": "1.1.7",
|
||||
"@radix-ui/react-collection": "1.1.15",
|
||||
"@radix-ui/react-compose-refs": "1.1.5",
|
||||
"@radix-ui/react-context": "1.2.2",
|
||||
"@radix-ui/react-direction": "1.1.4",
|
||||
"@radix-ui/react-id": "1.1.4",
|
||||
"@radix-ui/react-primitive": "2.1.10",
|
||||
"@radix-ui/react-use-callback-ref": "1.1.4",
|
||||
"@radix-ui/react-use-controllable-state": "1.2.6",
|
||||
"@radix-ui/react-use-is-hydrated": "0.1.3",
|
||||
"@radix-ui/react-use-layout-effect": "1.1.4"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@types/react": "*",
|
||||
"@types/react-dom": "*",
|
||||
"react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc",
|
||||
"react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc"
|
||||
},
|
||||
"peerDependenciesMeta": {
|
||||
"@types/react": {
|
||||
"optional": true
|
||||
},
|
||||
"@types/react-dom": {
|
||||
"optional": true
|
||||
}
|
||||
}
|
||||
},
|
||||
"node_modules/@radix-ui/react-select": {
|
||||
"version": "2.3.7",
|
||||
"resolved": "https://registry.npmjs.org/@radix-ui/react-select/-/react-select-2.3.7.tgz",
|
||||
@@ -2030,36 +1996,6 @@
|
||||
}
|
||||
}
|
||||
},
|
||||
"node_modules/@radix-ui/react-tabs": {
|
||||
"version": "1.1.21",
|
||||
"resolved": "https://registry.npmjs.org/@radix-ui/react-tabs/-/react-tabs-1.1.21.tgz",
|
||||
"integrity": "sha512-UKxJlZid7FVtsk/WTxj4i4uSEgj2Au+KBbS7SQyTlzMhhn+86Cz3tISZdTa87bfEfcuvZezf2ZsxD4xuEKtkog==",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"@radix-ui/primitive": "1.1.7",
|
||||
"@radix-ui/react-context": "1.2.2",
|
||||
"@radix-ui/react-direction": "1.1.4",
|
||||
"@radix-ui/react-id": "1.1.4",
|
||||
"@radix-ui/react-presence": "1.1.10",
|
||||
"@radix-ui/react-primitive": "2.1.10",
|
||||
"@radix-ui/react-roving-focus": "1.1.19",
|
||||
"@radix-ui/react-use-controllable-state": "1.2.6"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@types/react": "*",
|
||||
"@types/react-dom": "*",
|
||||
"react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc",
|
||||
"react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc"
|
||||
},
|
||||
"peerDependenciesMeta": {
|
||||
"@types/react": {
|
||||
"optional": true
|
||||
},
|
||||
"@types/react-dom": {
|
||||
"optional": true
|
||||
}
|
||||
}
|
||||
},
|
||||
"node_modules/@radix-ui/react-use-callback-ref": {
|
||||
"version": "1.1.4",
|
||||
"resolved": "https://registry.npmjs.org/@radix-ui/react-use-callback-ref/-/react-use-callback-ref-1.1.4.tgz",
|
||||
@@ -2113,21 +2049,6 @@
|
||||
}
|
||||
}
|
||||
},
|
||||
"node_modules/@radix-ui/react-use-is-hydrated": {
|
||||
"version": "0.1.3",
|
||||
"resolved": "https://registry.npmjs.org/@radix-ui/react-use-is-hydrated/-/react-use-is-hydrated-0.1.3.tgz",
|
||||
"integrity": "sha512-umO/aJ+82CpOnhDZUTbILCQf7kU/g0iv+oGs/Q8jw7IkhWBzaEP4sA268PhFAJTFetbwp3ICc6ktpI4TqtxcIw==",
|
||||
"license": "MIT",
|
||||
"peerDependencies": {
|
||||
"@types/react": "*",
|
||||
"react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc"
|
||||
},
|
||||
"peerDependenciesMeta": {
|
||||
"@types/react": {
|
||||
"optional": true
|
||||
}
|
||||
}
|
||||
},
|
||||
"node_modules/@radix-ui/react-use-layout-effect": {
|
||||
"version": "1.1.4",
|
||||
"resolved": "https://registry.npmjs.org/@radix-ui/react-use-layout-effect/-/react-use-layout-effect-1.1.4.tgz",
|
||||
|
||||
@@ -32,7 +32,6 @@
|
||||
"@radix-ui/react-label": "^2.1.11",
|
||||
"@radix-ui/react-select": "^2.3.7",
|
||||
"@radix-ui/react-slot": "^1.3.0",
|
||||
"@radix-ui/react-tabs": "^1.1.17",
|
||||
"@turf/turf": "^7.4.0",
|
||||
"@upstash/redis": "^1.38.3",
|
||||
"@vercel/analytics": "^1.5.0",
|
||||
|
||||
@@ -108,3 +108,49 @@ modules; `development.md` says tests run "well under a second" (24.5s
|
||||
measured); `game-flow.md` describes leaderboard pagination that does not
|
||||
exist; `tech-stack.md` two stale lines. All listed with line numbers in the
|
||||
architecture report.
|
||||
|
||||
## Resolution (same day)
|
||||
|
||||
Applied on `dev` in five commits, one per area, all gates green throughout
|
||||
(401 tests, lint 0 errors and 0 warnings, production compile).
|
||||
|
||||
- **Bugs 1–8**: `partial` reaches the dialog; new-game answers 404 for a dry
|
||||
pool and 502 for an upstream failure, its error handler tolerates a
|
||||
non-Error, and it no longer logs the answer's district; the theme toggle
|
||||
pair reads one store; every localStorage access goes through a guarded
|
||||
module; the map centre is derived; the e2e stubs carry `hit`, `partial`,
|
||||
`guessedRegion` and a `/api/daily` stub, and `tests/e2e-stub-contract.test.js`
|
||||
asserts each stub is a superset of the real route; the scripts share one
|
||||
`.env` loader with the unquoting.
|
||||
- **Dead surface**: aliases, legacy ranks, duplicated envelope, `submitScore`,
|
||||
unread leaderboard fields, the new-game debug POST, `zScore`, `isDay`,
|
||||
`indexedProvinces`, `@radix-ui/react-tabs` — all gone. `leaderboard.test.js`
|
||||
awards points through `submitRoundScore`.
|
||||
- **Item 2** (envelope): `/api/guess` returns `gameResult` alone;
|
||||
`/api/leaderboard` returns the rows alone.
|
||||
- **Item 3** (error seam): `src/lib/errors.js`; eight string-match sites
|
||||
replaced; an eight-second draw budget and a fifteen-second client timeout.
|
||||
- **Item 4** (storage): `src/lib/storage.js` and `useStoredValue`; 20 lint
|
||||
warnings fixed with useSyncExternalStore, useEffectEvent, render-time
|
||||
adjustment and derived values; the one data ref carries its reason; both
|
||||
react-hooks rules are now errors.
|
||||
- **Item 5** (GameClient): GameHeader extracted (713 → 644 lines); the daily
|
||||
replay derives from the stored record rather than six setState calls. The
|
||||
`use-round` hook is deferred until a multi-round feature needs it.
|
||||
- **Item 6** (scripts): `scripts/lib/env.mjs`, `scripts/lib/region-config.mjs`,
|
||||
`data:refresh`. `drawFromProvinces` not extracted: `pickPanoBySeed` keeps its
|
||||
own walk because its skip-empty-province rule differs from the random draw.
|
||||
- **Item 7**: 28 tests for `assign-districts.mjs`, 14 for the env loader.
|
||||
- **Item 8**: PlaceName with `lang="vi"` at the pure-name sites, the expanded
|
||||
guess map is a dialog, MapSearchBox loads with the map, debug hub is a
|
||||
server component, header buttons use the default touch size. Not done:
|
||||
`cardRowVariants`, `safe-x` utility, the reveal-sequence class — cosmetic
|
||||
and each touches several files for no behaviour change.
|
||||
- **Item 9**: conventions codified in `docs/development.md`; the parameter
|
||||
rule amended in `CLAUDE.md`; doc drift fixed.
|
||||
|
||||
Decisions taken (defaults, override freely): upstream outage is a 502, not a
|
||||
200; the lint warnings are fixed rather than the comment updated;
|
||||
`submitScore` is deleted (no backfill procedure references it); the
|
||||
`use-round` hook waits for a multi-round feature; the home-page username
|
||||
interception stays.
|
||||
@@ -282,6 +282,10 @@ describe('client safety', () => {
|
||||
'pano-db',
|
||||
'pano-history',
|
||||
'data/boundaries',
|
||||
// The daily pick passes the day's answer through; a client bundle that
|
||||
// reached it would hold every player's round.
|
||||
'lib/daily.js',
|
||||
'region-locate',
|
||||
];
|
||||
|
||||
const collectSourceFiles = (dir, out) => {
|
||||
|
||||
Reference in new issue
Block a user