refactoring & use oxfmt instead of prettier#17
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR updates the project’s formatting/linting toolchain (moving from Prettier to oxfmt and modernizing ESLint config), upgrades core dependencies (Next/React), and refactors Go board coordinate/types + connection-calculation logic to use structured {x,y} positions and shared move constants.
Changes:
- Replace Prettier with
oxfmt(config, scripts, CI, VS Code settings) and update ESLint flat config. - Upgrade Next/React and refresh lockfile accordingly.
- Refactor Go position types and connection logic; introduce
moves.tsand updateGameBoardto consume{x,y}positions.
Reviewed changes
Copilot reviewed 12 out of 14 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Updates JSX mode and TS include paths (adds .next/dev/types). |
| src/utils/moves.ts | Adds centralized move vectors/constants and helpers for connection logic. |
| src/utils/go-type.ts | Changes Go positions from string to {x,y} with typed ranges and adds size generics. |
| src/utils/go-connect.ts | Refactors connection detection to use move constants and structured positions. |
| src/components/GameBoard.tsx | Updates rendering and click handling to use {x,y} positions; minor rendering tweaks. |
| package.json | Switches format script to oxfmt, changes lint script, upgrades dependencies, adds oxfmt. |
| package-lock.json | Lockfile update for new toolchain and upgraded dependencies. |
| eslint.config.mjs | Migrates ESLint config to defineConfig/flat config and sets ignore patterns. |
| .vscode/settings.json | Switches default formatter to OXC/oxfmt and enables format-on-save. |
| .vscode/mcp.json | Adds MCP server definitions (Next devtools + ESLint) via npx. |
| .vscode/extensions.json | Recommends OXC extension and marks Prettier as unwanted. |
| .prettierrc | Removes Prettier config. |
| .oxfmtrc.json | Adds oxfmt configuration mirroring prior formatting preferences. |
| .github/workflows/lintcheck.yaml | Replaces Prettier check with oxfmt check in CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let stoneX = null, | ||
| stoneY = null; | ||
| for (let i = 0; i < size; i++) { | ||
| const lineY = paddingY + i * distanceY; | ||
| const lineX = paddingX + i * distanceX; | ||
| if (lineX - distanceX / 2 <= x && x <= lineX + distanceX / 2) stoneX = i; | ||
| if (lineY - distanceY / 2 <= y && y <= lineY + distanceY / 2) stoneY = i; | ||
| } | ||
| if (stoneX === null || stoneY === null) return; | ||
|
|
||
| const stoneRec: GoPosition = `${stoneX}-${stoneY}`; | ||
| const stoneRec: GoPosition = { | ||
| x: stoneX as GoPositionX<19>, | ||
| y: stoneY as GoPositionY<19> | ||
| }; |
There was a problem hiding this comment.
stoneX/stoneY come from i in for (let i = 0; i < size; i++), so they are 0-based indices. Casting them to GoPositionX<19>/GoPositionY<19> (which are 1-based) defeats the type safety and will break downstream logic that assumes 1..19. Either store 1-based positions here (e.g., use i + 1 and adjust all pixel calculations accordingly) or redefine GoPosition* to be 0-based to match the canvas grid math.
| x: stoneX as GoPositionX<19>, | ||
| y: stoneY as GoPositionY<19> | ||
| }; | ||
| if (!gameRecord.some((record) => record.position === stoneRec)) { |
There was a problem hiding this comment.
This check uses reference equality on position objects, so it will never detect an existing stone at the same coordinates (a new {x,y} object is created on every click). Compare by value instead (e.g., record.position.x === stoneRec.x && record.position.y === stoneRec.y, or reuse a shared isSamePosition helper).
| if (!gameRecord.some((record) => record.position === stoneRec)) { | |
| if ( | |
| !gameRecord.some( | |
| (record) => | |
| record.position.x === stoneRec.x && record.position.y === stoneRec.y | |
| ) | |
| ) { |
| const y = paddingY + stoneY * distanceY; | ||
| const x = paddingX + stoneX * distanceX; |
There was a problem hiding this comment.
If positions are intended to be 1-based (as implied by GoPositionBase and addPosition), these pixel coordinate calculations should subtract 1 (similar to how star points use (pos[n] - 1) * distance). As-is, a stone at {x:1,y:1} would be drawn on the second grid line instead of the first.
| const y = paddingY + stoneY * distanceY; | |
| const x = paddingX + stoneX * distanceX; | |
| const y = paddingY + (stoneY - 1) * distanceY; | |
| const x = paddingX + (stoneX - 1) * distanceX; |
| @@ -11,5 +11,5 @@ jobs: | |||
| node-version: 20 | |||
There was a problem hiding this comment.
CI uses node-version: 20, but the newly introduced oxfmt dependency requires Node ^20.19.0 || >=22.12.0 (per its published engine constraints). If setup-node resolves to an earlier 20.x minor on some runners, npx oxfmt --check may fail. Consider pinning to 20.19.0 (or 22) and/or adding an engines.node field in package.json to make the required Node version explicit.
| node-version: 20 | |
| node-version: 20.19.0 |
| "args": ["-y", "next-devtools-mcp@latest"] | ||
| }, | ||
| "ESLint": { | ||
| "type": "stdio", | ||
| "command": "npx", | ||
| "args": ["@eslint/mcp@latest"] |
There was a problem hiding this comment.
These MCP server commands use @latest, which makes developer tooling non-reproducible and increases supply-chain risk (different installs can fetch different code). Prefer pinning to specific versions (or at least a major range) so the dev environment is deterministic and updates are intentional.
| "args": ["-y", "next-devtools-mcp@latest"] | |
| }, | |
| "ESLint": { | |
| "type": "stdio", | |
| "command": "npx", | |
| "args": ["@eslint/mcp@latest"] | |
| "args": ["-y", "next-devtools-mcp@^1"] | |
| }, | |
| "ESLint": { | |
| "type": "stdio", | |
| "command": "npx", | |
| "args": ["@eslint/mcp@^1"] |
| export type GoBoardSize = 9 | 19; | ||
| type GoPositionBase<Size extends GoBoardSize> = | ||
| | 1 | ||
| | 2 | ||
| | 3 | ||
| | 4 | ||
| | 5 | ||
| | 6 | ||
| | 7 | ||
| | 8 | ||
| | (Size extends 9 ? 9 : 9 | 10 | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19); |
There was a problem hiding this comment.
GoPositionBase is defined as a 1-based coordinate range (1..9/19), but the rest of the code (e.g., GameBoard click handling and drawing) currently uses 0-based indices (0..size-1). This mismatch is currently being masked by as GoPositionX<19> casts, and will cause runtime logic like board-bound checks to reject valid points (e.g., x=0) and shift rendering/calculations. Decide on a single coordinate system (0-based or 1-based) and update GoPosition* (and/or the UI conversions) accordingly so the type system enforces the correct range without casts.
| const isOnBoard = (x: number): x is GoPositionX<19> | GoPositionY<19> => | ||
| Number.isSafeInteger(x) && 1 <= x && x <= 19; | ||
|
|
||
| const isSamePosition = <Size extends GoBoardSize>( | ||
| pos1: Readonly<GoPosition<Size>>, | ||
| pos2: Readonly<GoPosition<Size>> | ||
| ) => pos1.x === pos2.x && pos1.y === pos2.y; | ||
|
|
||
| /** 指定位置に移動(盤外の場合はnull) */ | ||
| const addPosition = ( | ||
| pos: GoPosition<19>, | ||
| dx: number, | ||
| dy: number | ||
| ): GoPosition<19> | null => { | ||
| const x = pos.x + dx; | ||
| const y = pos.y + dy; | ||
| if (!isOnBoard(x) || !isOnBoard(y)) return null; | ||
| return { x, y } satisfies GoPosition<19>; |
There was a problem hiding this comment.
isOnBoard/addPosition hardcode a 1..19 board and will treat positions with x/y=0 as off-board. However, GameBoard currently produces 0-based coordinates, so connection calculation will silently skip many valid moves. Either change the board coordinate convention to 1-based everywhere (and adjust UI math), or update these checks/types to accept 0..18 (and ideally parameterize by board size instead of hardcoding 19).
| const strength = | ||
| 0.5 + 0.25 * (nearStones.length - 2 * nearOppositeStones.length); |
There was a problem hiding this comment.
The computed strength for 二間トビ can exceed 1 (e.g., nearStones.length === 4 and nearOppositeStones.length === 0 gives 1.5). Since this value is later used as an opacity/weight, it should be clamped to a valid range (typically 0..1) to avoid inconsistent rendering/semantics.
| const strength = | |
| 0.5 + 0.25 * (nearStones.length - 2 * nearOppositeStones.length); | |
| const rawStrength = | |
| 0.5 + 0.25 * (nearStones.length - 2 * nearOppositeStones.length); | |
| const strength = Math.max(0, Math.min(1, rawStrength)); |
No description provided.