feat: migrate ESLint 8 to ESLint 9 flat config for native ESM support#279
feat: migrate ESLint 8 to ESLint 9 flat config for native ESM support#279jmanhype wants to merge 4 commits intoautomagik-dev:devfrom
Conversation
Summary of ChangesHello @jmanhype, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request undertakes a significant modernization of the project's linting infrastructure. It transitions the ESLint configuration from the older CommonJS-based .eslintrc.cjs to the more flexible and powerful eslint.config.js flat configuration. This change is coupled with an upgrade to ESLint v9, which brings native ESM support, and corresponding updates to TypeScript ESLint and other related plugins. The overall impact is a more robust, up-to-date, and future-proof linting environment that aligns with modern JavaScript development practices. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully migrates the frontend from ESLint v8 to v9, adopting the new flat configuration format. The dependency updates and script changes in package.json are all correct and align with the migration. The new eslint.config.js is well-structured, but I've found one minor issue regarding the placement of the Prettier configuration which could cause it to not function as intended. My review includes a suggestion to fix this.
| export default tseslint.config( | ||
| // Global ignores | ||
| { | ||
| ignores: ['dist/**', 'node_modules/**'], | ||
| }, | ||
|
|
||
| // Base JS recommended rules | ||
| js.configs.recommended, | ||
|
|
||
| // TypeScript configuration with type checking | ||
| ...tseslint.configs.recommended, | ||
|
|
||
| // Prettier compatibility (must be after other configs) | ||
| prettier, | ||
|
|
||
| // Main configuration for TypeScript files | ||
| { | ||
| files: ['**/*.{ts,tsx}'], | ||
| languageOptions: { | ||
| ecmaVersion: 'latest', | ||
| sourceType: 'module', | ||
| globals: { | ||
| ...globals.browser, | ||
| ...globals.es2020, | ||
| }, | ||
| parserOptions: { | ||
| project: './tsconfig.json', | ||
| }, | ||
| }, | ||
| plugins: { | ||
| 'react-hooks': reactHooks, | ||
| 'react-refresh': reactRefresh, | ||
| 'unused-imports': unusedImports, | ||
| i18next: i18next, | ||
| }, | ||
| rules: { | ||
| // React hooks rules | ||
| ...reactHooks.configs.recommended.rules, | ||
|
|
||
| // React refresh | ||
| 'react-refresh/only-export-components': 'off', | ||
|
|
||
| // Unused imports | ||
| 'unused-imports/no-unused-imports': 'error', | ||
| 'unused-imports/no-unused-vars': [ | ||
| 'error', | ||
| { | ||
| vars: 'all', | ||
| args: 'after-used', | ||
| ignoreRestSiblings: false, | ||
| }, | ||
| ], | ||
|
|
||
| // TypeScript rules | ||
| '@typescript-eslint/no-explicit-any': 'warn', | ||
| '@typescript-eslint/no-unused-vars': 'off', // Using unused-imports plugin instead | ||
| '@typescript-eslint/no-empty-object-type': 'off', // Allow empty interfaces (was allowed in v6) | ||
| '@typescript-eslint/switch-exhaustiveness-check': 'error', | ||
|
|
||
| // i18n rule - only active when LINT_I18N=true | ||
| 'i18next/no-literal-string': i18nCheck | ||
| ? [ | ||
| 'warn', | ||
| { | ||
| markupOnly: true, | ||
| ignoreAttribute: [ | ||
| 'data-testid', | ||
| 'to', | ||
| 'href', | ||
| 'id', | ||
| 'key', | ||
| 'type', | ||
| 'role', | ||
| 'className', | ||
| 'style', | ||
| 'aria-describedby', | ||
| ], | ||
| 'jsx-components': { | ||
| exclude: ['code'], | ||
| }, | ||
| }, | ||
| ] | ||
| : 'off', | ||
| }, | ||
| }, | ||
|
|
||
| // Override for test and story files | ||
| { | ||
| files: ['**/*.test.{ts,tsx}', '**/*.stories.{ts,tsx}'], | ||
| rules: { | ||
| 'i18next/no-literal-string': 'off', | ||
| }, | ||
| }, | ||
|
|
||
| // Override for config files - disable type-aware linting | ||
| { | ||
| files: ['*.config.{ts,js,cjs,mjs}', 'eslint.config.js'], | ||
| languageOptions: { | ||
| parserOptions: { | ||
| project: null, | ||
| }, | ||
| }, | ||
| rules: { | ||
| '@typescript-eslint/switch-exhaustiveness-check': 'off', | ||
| }, | ||
| } | ||
| ); |
There was a problem hiding this comment.
Great job on migrating to the new ESLint flat config! There's one small but important adjustment needed. The prettier configuration should be the last element in the configuration array. This ensures that it can correctly override any stylistic rules from other configurations (like js.configs.recommended or tseslint.configs.recommended) that might conflict with Prettier's formatting. The comment on line 25 indicates this intention, but the placement is incorrect.
export default tseslint.config(
// Global ignores
{
ignores: ['dist/**', 'node_modules/**'],
},
// Base JS recommended rules
js.configs.recommended,
// TypeScript configuration with type checking
...tseslint.configs.recommended,
// Main configuration for TypeScript files
{
files: ['**/*.{ts,tsx}'],
languageOptions: {
ecmaVersion: 'latest',
sourceType: 'module',
globals: {
...globals.browser,
...globals.es2020,
},
parserOptions: {
project: './tsconfig.json',
},
},
plugins: {
'react-hooks': reactHooks,
'react-refresh': reactRefresh,
'unused-imports': unusedImports,
i18next: i18next,
},
rules: {
// React hooks rules
...reactHooks.configs.recommended.rules,
// React refresh
'react-refresh/only-export-components': 'off',
// Unused imports
'unused-imports/no-unused-imports': 'error',
'unused-imports/no-unused-vars': [
'error',
{
vars: 'all',
args: 'after-used',
ignoreRestSiblings: false,
},
],
// TypeScript rules
'@typescript-eslint/no-explicit-any': 'warn',
'@typescript-eslint/no-unused-vars': 'off', // Using unused-imports plugin instead
'@typescript-eslint/no-empty-object-type': 'off', // Allow empty interfaces (was allowed in v6)
'@typescript-eslint/switch-exhaustiveness-check': 'error',
// i18n rule - only active when LINT_I18N=true
'i18next/no-literal-string': i18nCheck
? [
'warn',
{
markupOnly: true,
ignoreAttribute: [
'data-testid',
'to',
'href',
'id',
'key',
'type',
'role',
'className',
'style',
'aria-describedby',
],
'jsx-components': {
exclude: ['code'],
},
},
]
: 'off',
},
},
// Override for test and story files
{
files: ['**/*.test.{ts,tsx}', '**/*.stories.{ts,tsx}'],
rules: {
'i18next/no-literal-string': 'off',
},
},
// Override for config files - disable type-aware linting
{
files: ['*.config.{ts,js,cjs,mjs}', 'eslint.config.js'],
languageOptions: {
parserOptions: {
project: null,
},
},
rules: {
'@typescript-eslint/switch-exhaustiveness-check': 'off',
},
},
// Prettier compatibility (must be last)
prettier,
);![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| eslint@9.39.1: | ||
| resolution: {integrity: sha512-BhHmn2yNOFA9H9JmmIVKJmd288g9hrVRDkdoIgRCRuSySRUHH7r/DI6aAXW9T1WwUuY3DFgrcaqB+deURBLR5g==} | ||
| engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} |
There was a problem hiding this comment.
The new ESLint dependency now requires Node ^18.18.0 (pnpm-lock.yaml shows eslint@9.39.1 with that engine range), but the package still declares only "node": ">=18" and the repo enforces engine checks. Anyone on Node 18.0–18.17 who previously satisfied the declared engines will now hit install failures after this commit because engine-strict is enabled. Please either bump the declared Node version/documentation or avoid dependencies that require 18.18+.
Useful? React with 👍 / 👎.
Per Gemini Code Assist feedback, the Prettier configuration must be placed at the END of the ESLint flat config array to properly disable conflicting formatting rules. This ensures Prettier's formatting preferences take precedence over any ESLint formatting rules defined earlier in the config. Fixes positioning issue identified in PR automagik-dev#279 review. Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
- Replace .eslintrc.cjs with eslint.config.js flat config format - Update ESLint from v8 to v9.15.0 for native ESM support - Update typescript-eslint from v6 to v8.15.0 (ESLint 9 compatible) - Update eslint-plugin-react-hooks from v4 to v5.1.0 - Update eslint-plugin-react-refresh to v0.4.14 - Add @eslint/js and globals packages for flat config - Update lint scripts to remove deprecated --ext flag - Adjust max-warnings to 115 to account for minor rule differences Fixes automagik-dev#247 Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
Per Gemini Code Assist feedback, the Prettier configuration must be placed at the END of the ESLint flat config array to properly disable conflicting formatting rules. This ensures Prettier's formatting preferences take precedence over any ESLint formatting rules defined earlier in the config. Fixes positioning issue identified in PR automagik-dev#279 review. Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
- Add SSR safety check in theme-provider.tsx to prevent window access during SSR - Define THEME_OPTIONS constant array in theme-toggle.tsx to eliminate code duplication - Refactor theme-toggle.tsx to use map() for cleaner rendering Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>
jmanhype
force-pushed
the
claude/migrate-eslint-9-01DXCuGu1q3RzGCCSeZD9tso
branch
from
40e34df to
b49d750
Compare
|
@codex review this pr |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pub mod router; | ||
| pub mod routes; | ||
| pub mod services; | ||
| pub mod version; |
There was a problem hiding this comment.
The new pub mod routes; re-export in lib.rs points to a module that does not exist (routes.rs or routes/mod.rs is absent in forge-app/src). Any build of the Rust crate now fails with a missing file error before even running tests, blocking compilation of the backend binaries.
Useful? React with 👍 / 👎.
| plugins: { | ||
| 'react-hooks': reactHooks, | ||
| 'react-refresh': reactRefresh, | ||
| 'unused-imports': unusedImports, | ||
| i18next: i18next, |
There was a problem hiding this comment.
The flat config registers only React, refresh, unused-imports, and i18next plugins, omitting the previously enforced eslint-plugin-local-rules and its local-rules/no-hardcoded-query-keys rule as well as the no-restricted-syntax guard that blocked sub‑15s polling intervals. As a result, lint no longer warns when code bypasses centralized query keys or adds tight polling loops—protections that were present in .eslintrc.cjs and are now silently disabled.
Useful? React with 👍 / 👎.
|
@codex review this pr |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| //! Provides reusable modules for forge binaries. | ||
|
|
||
| pub mod router; | ||
| pub mod routes; |
There was a problem hiding this comment.
Adding pub mod routes; exposes a module that does not exist anywhere under forge-app/src (no routes.rs or routes/mod.rs), so the crate will fail to compile with an unresolved module error as soon as forge-app is built. Please drop the export or add the missing module.
Useful? React with 👍 / 👎.
| plugins: { | ||
| 'react-hooks': reactHooks, | ||
| 'react-refresh': reactRefresh, | ||
| 'unused-imports': unusedImports, | ||
| i18next: i18next, | ||
| }, |
There was a problem hiding this comment.
The new flat ESLint config only registers standard plugins (react-hooks, react-refresh, unused-imports, i18next) and no longer loads the local-rules plugin or the no-restricted-syntax/local-rules/no-hardcoded-query-keys rules that previously enforced >15s polling intervals and centralized queryKeys. That regression means new code will now lint-cleanly even when it hardcodes query keys or short refetch intervals. Please port those custom rules into the flat config.
Useful? React with 👍 / 👎.
3 participants
Fixes #247