Conversation
1330Petrel
commented
Dec 20, 2025
1330Petrel
commented
- bump deps to latest
- add eslint and prettier
There was a problem hiding this comment.
Pull request overview
This PR updates the client dependencies to their latest versions and introduces code quality tooling with ESLint and Prettier. The major changes include upgrading React from v18 to v19, Vite from v5 to v7, React Router from v6 to v7, and adding comprehensive linting and formatting configurations.
- Bumps all major dependencies to latest versions (React 19, Vite 7, React Router v7, etc.)
- Adds ESLint 9 with flat config format and TypeScript/React plugins
- Adds Prettier with project-specific formatting rules
Reviewed changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| client/package.json | Updates all dependencies to latest versions and adds new scripts for linting and formatting |
| client/tsconfig.json | Changes moduleResolution from "node" to "bundler" for better Vite compatibility |
| client/eslint.config.js | Adds comprehensive ESLint flat config with TypeScript and React support |
| client/.prettierrc | Adds Prettier configuration with code style preferences |
| .gitignore | Excludes JavaScript files but allows eslint.config.js |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "moduleResolution": "node", | ||
| "moduleResolution": "bundler", | ||
| "resolveJsonModule": true, | ||
| "isolatedModules": true, |
There was a problem hiding this comment.
For a Vite project, it's recommended to add "noEmit": true to the compilerOptions since Vite handles the transpilation and bundling. The 'tsc' command in the build script (line 8 of package.json) is being used for type-checking only, so TypeScript should not emit JavaScript files. Adding this option prevents accidental file generation and clarifies the intent.
| "isolatedModules": true, | |
| "isolatedModules": true, | |
| "noEmit": true, |
client/tsconfig.json
Outdated
| "moduleResolution": "bundler", | ||
| "resolveJsonModule": true, | ||
| "isolatedModules": true, | ||
| "jsx": "react" |
There was a problem hiding this comment.
The tsconfig.json sets "jsx": "react" which is the classic JSX transform requiring React to be in scope. With React 19 and the modern JSX transform, this should be "jsx": "react-jsx" to match the ESLint configuration (line 87 of eslint.config.js disables 'react/react-in-jsx-scope'). This will eliminate the need to import React in every JSX file and align with the current ESLint rules.
| "jsx": "react" | |
| "jsx": "react-jsx" |
| "noFallthroughCasesInSwitch": true, | ||
| "module": "esnext", | ||
| "moduleResolution": "node", | ||
| "moduleResolution": "bundler", |
There was a problem hiding this comment.
The "moduleResolution" change from "node" to "bundler" is appropriate for Vite projects. However, this may affect how TypeScript resolves module imports. Ensure that all imports in the codebase are compatible with the "bundler" resolution strategy, particularly for packages that don't have proper ESM exports. The "bundler" mode is more strict and may require package.json "exports" fields to be present.
| languageOptions: { | ||
| parser: tseslint.parser, | ||
| parserOptions: { | ||
| ecmaFeatures: { jsx: true }, |
There was a problem hiding this comment.
Using the "typescript-eslint" package version 8.50.0 requires TypeScript 5.4 or higher. The package.json specifies TypeScript 5.9.3, which is compatible. However, ensure that the typescript-eslint parser configuration is complete - you should also set the "project" option in parserOptions to enable type-aware linting rules for better type checking. Without this, many useful TypeScript-specific ESLint rules won't work.
| ecmaFeatures: { jsx: true }, | |
| ecmaFeatures: { jsx: true }, | |
| project: ['./tsconfig.json'], |
| }, | ||
| settings: { | ||
| 'import/resolver': { | ||
| typescript: true, |
There was a problem hiding this comment.
The eslint-import-resolver-typescript package is included in devDependencies (line 28) but the resolver configuration in eslint.config.js (line 45) only sets "typescript: true". For the resolver to work properly with the flat config format, you may need to explicitly configure it. Consider adding the resolver configuration as: 'import/resolver': { typescript: { project: './tsconfig.json' } } to ensure proper module resolution.
| typescript: true, | |
| typescript: { | |
| project: './tsconfig.json', | |
| }, |
client/package.json
Outdated
| "lint": "eslint \"src/**/*.{ts,tsx,js,jsx}\"", | ||
| "lint:fix": "eslint \"src/**/*.{ts,tsx,js,jsx}\" --fix", | ||
| "lint:ci": "eslint \"src/**/*.{ts,tsx,js,jsx}\" --max-warnings=0", | ||
| "prettier": "prettier --write \"src/**/*.{ts,tsx,js,jsx,json,css,md}\"", |
There was a problem hiding this comment.
The prettier script (line 12) only formats files in the "src/" directory. Consider also formatting the root-level configuration files (eslint.config.js, vite.config.js) by adding them to the glob pattern. This ensures consistent formatting across the entire codebase. For example: "prettier --write "src//.{ts,tsx,js,jsx,json,css,md}" ".{js,json}""
| "prettier": "prettier --write \"src/**/*.{ts,tsx,js,jsx,json,css,md}\"", | |
| "prettier": "prettier --write \"src/**/*.{ts,tsx,js,jsx,json,css,md}\" \"*.{js,json}\"", |
| globals: { | ||
| // Browser | ||
| window: 'readonly', | ||
| document: 'readonly', | ||
| navigator: 'readonly', | ||
| location: 'readonly', | ||
|
|
||
| // Web APIs | ||
| fetch: 'readonly', | ||
| Request: 'readonly', | ||
| Response: 'readonly', | ||
| Headers: 'readonly', | ||
| URL: 'readonly', | ||
|
|
||
| // Runtime | ||
| console: 'readonly', | ||
| setTimeout: 'readonly', | ||
| clearTimeout: 'readonly', | ||
| setInterval: 'readonly', | ||
| clearInterval: 'readonly', | ||
| }, |
There was a problem hiding this comment.
The ESLint configuration manually defines browser globals (lines 18-38). Instead of manually listing globals like 'window', 'document', 'fetch', etc., you should use the 'globals' package which provides predefined sets. Install 'globals' and import it, then use 'globals.browser' in the languageOptions.globals field. This is the recommended approach for ESLint flat config and reduces the risk of missing globals or typos.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 13 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import StatsChart from './components/StatsChart'; | ||
| import { startGameLoop, subscribeToGame, sendHumanAction, getInitialState, resetGame } from './services/mockGameEngine'; | ||
|
|
||
| import type { GameState } from '../src/types'; |
There was a problem hiding this comment.
The import path ../src/types is incorrect since this file is already in the src directory. It should be ./types instead. This works accidentally because the parent directory contains a src folder, but it's semantically wrong and could break if the project structure changes.
| import type { GameState } from '../src/types'; | |
| import type { GameState } from './types'; |
| "prettier": "^3.7.4", | ||
| "typescript-eslint": "^8.50.0", | ||
| "typescript": "^5.9.3", | ||
| "vite": "^7.3.0" |
There was a problem hiding this comment.
Vite has been upgraded from v5.4.1 to v7.3.0, which is a major version jump (skipping v6). This is a significant breaking change. Ensure all Vite configuration and plugins are compatible with v7, as this version may have breaking changes in API, plugin interfaces, or default behaviors. Refer to Vite's migration guide for v6 and v7.
| "react-router-dom": "^6.15.0", | ||
| "recharts": "^2.8.0" | ||
| "lucide-react": "^0.562.0", | ||
| "react": "^19.2.3", |
There was a problem hiding this comment.
React has been upgraded from v18.3.1 to v19.2.3, which is a major version change. React 19 introduces breaking changes including the removal of legacy APIs and changes to Suspense behavior. Verify that all React usage in the codebase is compatible with React 19, particularly around concurrent features, StrictMode, and any deprecated APIs that may have been removed.
| "lucide-react": "^0.562.0", | ||
| "react": "^19.2.3", | ||
| "react-dom": "^19.2.3", | ||
| "react-router-dom": "^7.11.0", |
There was a problem hiding this comment.
react-router-dom has been upgraded from v6.15.0 to v7.11.0, which is a major version change. React Router v7 has significant breaking changes including changes to the routing API, data loading patterns, and component interfaces. Ensure all router usage (HashRouter, Routes, Route, useNavigate, useLocation) is compatible with v7 and that any deprecated patterns have been updated.
| "react": "^19.2.3", | ||
| "react-dom": "^19.2.3", | ||
| "react-router-dom": "^7.11.0", | ||
| "recharts": "^3.6.0" |
There was a problem hiding this comment.
recharts has been upgraded from v2.8.0 to v3.6.0, which is a major version change. Recharts v3 likely has breaking changes in its API or behavior. Verify that the BarChart component usage in StatsChart.tsx is compatible with v3, particularly around prop interfaces and chart rendering behavior.
1 participant
