-
Notifications
You must be signed in to change notification settings - Fork 1
Feat/refacter #39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/refacter #39
Changes from all commits
aa1c8b5
2051f19
04e7f9e
eeb2911
8d84cf3
47e3e5b
3e97d5a
d5ad3a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| --- | ||
| name: splatoon-random-weapon | ||
| description: Use this skill when working in the splatoon-random-weapon repository. It covers the Preact + Vite frontend, the Hono API running on Cloudflare Pages Functions, and the D1/KV data flow used for random weapon selection and result history. | ||
| --- | ||
|
|
||
| # Splatoon Random Weapon | ||
|
|
||
| Use this skill for feature work, bug fixes, refactors, or reviews in this repository. | ||
|
|
||
| ## Repo map | ||
|
|
||
| - `src/main.tsx`: frontend entry point. | ||
| - `src/app.tsx`: main UI flow. This is the first place to inspect for product behavior. | ||
| - `src/components/`: presentational UI components. | ||
| - `functions/api/[[route]].ts`: API entry point for Cloudflare Pages Functions. | ||
| - `functions/api/routes/weapon.ts`: D1-backed weapon endpoints. | ||
| - `functions/api/routes/result.ts`: KV-backed result endpoint. | ||
| - `wrangler.toml`: Cloudflare bindings for D1 (`DB`) and KV (`RANDOM_WEAPONS`). | ||
|
|
||
| ## Architecture | ||
|
|
||
| - Frontend uses `preact`, `vite`, `tailwindcss`, and `swr`. | ||
| - API uses `hono` on Cloudflare Pages Functions under `/api`. | ||
| - The frontend builds a typed client with `hc<AppType>('/')` and calls: | ||
| - `GET /api/weapons/random?count=<n>` | ||
| - `GET /api/results` | ||
| - Local Vite dev proxies `/api` to `http://localhost:3000`. | ||
|
|
||
| ## Working rules for this repo | ||
|
|
||
| - Treat `src/app.tsx` as the current source of truth for user-visible behavior. | ||
| - Keep frontend and API changes aligned. If you change an API response shape, update the typed client usage in `src/app.tsx` in the same task. | ||
| - Be careful with the boundary between real data and placeholders: | ||
| - `useSWR('results', ...)` fetches live result data from KV. | ||
| - `cards` in `src/app.tsx` are currently hardcoded sample history entries. | ||
| - Do not assume the rendered history UI is already wired to backend data. | ||
| - `src/constants/weapon.ts` appears to be legacy or unused. Confirm usage before editing it. | ||
| - `tsconfig.json` includes only `src`, even though the frontend imports types from `functions/`. If type changes behave strangely, inspect this include boundary first. | ||
| - `package.json` is configured for `husky` and `lint-staged`. Avoid assuming hooks are consistently installed. | ||
|
|
||
| ## Safe change workflow | ||
|
|
||
| 1. Read `src/app.tsx` and the relevant route files before editing. | ||
| 2. Prefer the existing API shape and component patterns unless the task clearly requires a change. | ||
| 3. For backend changes, verify the corresponding Cloudflare binding exists in `wrangler.toml`. | ||
| 4. For frontend changes, check whether data is live or placeholder before wiring UI logic. | ||
| 5. Run the narrowest useful verification available after edits. | ||
|
|
||
| ## Verification | ||
|
|
||
| - Install dependencies: `npm install` | ||
| - Frontend dev server: `npm run dev` | ||
| - Lint scripts: `npm run lint:script` | ||
| - Lint styles: `npm run lint:style` | ||
|
|
||
| If a task touches Cloudflare runtime behavior, note that local verification may also require a Pages/Workers dev setup that is not fully captured by current package scripts. | ||
|
|
||
| ## Known pitfalls | ||
|
|
||
| - `functions/api/routes/weapon.ts` builds the SQL `LIMIT` clause from the query string directly. Be cautious when changing request handling there. | ||
| - `index.html` still has the default Vite title, so product polish tasks may need to update app metadata. | ||
| - `wrangler.toml` contains concrete binding IDs. Do not rotate or replace them casually. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,9 +5,46 @@ type Bindings = { | |||||||||||||||||||||||||||||
| RANDOM_WEAPONS: KVNamespace | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const result = new Hono<{ Bindings: Bindings }>().get('/', async (c) => { | ||||||||||||||||||||||||||||||
| let results = await c.env.RANDOM_WEAPONS.get('results', 'json') | ||||||||||||||||||||||||||||||
| return c.json(results) | ||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||
| export type ResultHistoryItem = { | ||||||||||||||||||||||||||||||
| id: string | ||||||||||||||||||||||||||||||
| title: string | ||||||||||||||||||||||||||||||
| weaponList: string[] | ||||||||||||||||||||||||||||||
| createdAt: string | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| type CreateResultPayload = { | ||||||||||||||||||||||||||||||
| weaponList?: string[] | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const result = new Hono<{ Bindings: Bindings }>() | ||||||||||||||||||||||||||||||
| .get('/', async (c) => { | ||||||||||||||||||||||||||||||
| const results = | ||||||||||||||||||||||||||||||
| (await c.env.RANDOM_WEAPONS.get<ResultHistoryItem[]>('results', 'json')) ?? | ||||||||||||||||||||||||||||||
| [] | ||||||||||||||||||||||||||||||
| return c.json(results) | ||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||
| .post('/', async (c) => { | ||||||||||||||||||||||||||||||
| const body = (await c.req.json()) as CreateResultPayload | ||||||||||||||||||||||||||||||
| const weaponList = body.weaponList?.filter(Boolean) ?? [] | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
Comment on lines
+27
to
+29
|
||||||||||||||||||||||||||||||
| const body = (await c.req.json()) as CreateResultPayload | |
| const weaponList = body.weaponList?.filter(Boolean) ?? [] | |
| let body: CreateResultPayload | |
| try { | |
| body = (await c.req.json()) as CreateResultPayload | |
| } catch { | |
| return c.json({ message: 'Invalid JSON payload' }, 400) | |
| } | |
| if (!body || !Array.isArray(body.weaponList)) { | |
| return c.json({ message: 'weaponList must be an array' }, 400) | |
| } | |
| const weaponList = body.weaponList.filter(Boolean) |
Copilot
AI
Mar 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
weaponList is currently accepted with no upper bound and without validating element types. Since this is persisted into KV, a malicious/accidental large payload could exceed KV limits or bloat storage; non-string items could also break frontend assumptions. Consider enforcing a max length (e.g., 4) and ensuring each entry is a non-empty string before storing.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,8 +25,14 @@ const weapon = new Hono<{ Bindings: Bindings }>() | |
| }) | ||
| .get('/random', async (c) => { | ||
| const { count } = c.req.query() | ||
| const parsedCount = Number.parseInt(count ?? '', 10) | ||
|
|
||
| if (!Number.isInteger(parsedCount) || parsedCount < 1 || parsedCount > 4) { | ||
| return c.json({ message: 'count must be an integer between 1 and 4' }, 400) | ||
| } | ||
|
|
||
| let { results }: Weapons = await c.env.DB.prepare( | ||
| `SELECT * FROM Weapons ORDER BY RANDOM() LIMIT ${count};` | ||
| `SELECT * FROM Weapons ORDER BY RANDOM() LIMIT ${parsedCount};` | ||
| ).all() | ||
|
Comment on lines
34
to
36
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SQLインジェクションのリスクを完全に排除するため、D1のプリペアドステートメントでは、クエリ文字列に直接変数を埋め込むのではなく、 let { results }: Weapons = await c.env.DB.prepare(
`SELECT * FROM Weapons ORDER BY RANDOM() LIMIT ?`
)
.bind(parsedCount)
.all() |
||
| return c.json(results) | ||
| }) | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -11,90 +11,149 @@ import { CardList } from '@/components/CardList' | |||||
| import { SelectBox } from '@/components/SelectBox' | ||||||
|
|
||||||
| import { AppType } from '../functions/api/[[route]]' | ||||||
| import { ResultHistoryItem } from '../functions/api/routes/result' | ||||||
| import { Weapon } from '../functions/api/routes/weapon' | ||||||
|
||||||
| import { Weapon } from '../functions/api/routes/weapon' | |
| import type { Weapon } from '../functions/api/routes/weapon' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
リクエストボディを
as CreateResultPayloadで型アサーションしていますが、これでは型安全性が保証されず、予期しない形式のデータが送られてきた場合にランタイムエラーが発生する可能性があります。Honoには
zodValidatorなどのバリデーションミドルウェアが用意されています。これらを利用してリクエストボディの検証と型付けを安全に行うことをお勧めします。これにより、コードの堅牢性が向上し、意図しないデータに対するエラーハンドリングも容易になります。これは、スタイルガイドの「
anyの回避」(19行目)の原則にも合致する改善です。References
anyの使用は極力避け、具体的な型を指定します。 (link)