ci: phase-1 eslint hardening with flat config#92
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's GuideMigrates ESLint to the new flat config with stricter linting (React hooks, Jest, import ordering, unused imports), updates lint/test tooling wiring, and makes small code tweaks to satisfy new rules, including stabilizing the electron API reference and normalizing import ordering. Flow diagram for ESLint flat config layers and targetsflowchart TD
base[js_configs_recommended]
ignores["Global ignores (node_modules, dist, build, coverage, scripts, bundles)"]
js_ts_common["JS/TS common rules (src, tests)\n- globals: browser, node\n- unused-imports/no-unused-imports"]
imports_order["Import ordering (src JS/TS)\n- import/order"]
react_cfg["React and hooks (all JSX/TSX)\n- react recommended\n- react_hooks rules"]
ts_cfg["TypeScript rules (all TS/TSX)\n- @typescript-eslint recommended\n- no_unused_vars, no_explicit_any overrides"]
jest_cfg["Jest rules (tests, __tests__, *.test, *.spec)\n- jest recommended\n- noisy rules relaxed"]
files_src["src/**/*.{js,jsx,ts,tsx}"]
files_tests["tests/**/*.{js,jsx,ts,tsx}"]
files_jsx_tsx["**/*.{jsx,tsx}"]
files_ts_tsx["**/*.{ts,tsx}"]
files_jest["tests/**, __tests__/**, *.test.*, *.spec.*"]
base --> js_ts_common
js_ts_common --> files_src
js_ts_common --> files_tests
files_src --> imports_order
imports_order --> imports_src["src import ordering enforced"]
files_jsx_tsx --> react_cfg
files_ts_tsx --> ts_cfg
files_jest --> jest_cfg
ignores -. excludes .-> files_src
ignores -. excludes .-> files_tests
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Review Summary by QodoMigrate ESLint to flat config with phase-1 rule hardening
WalkthroughsDescription• Migrate ESLint to flat config format with phase-1 rule hardening • Enable React hooks, Jest plugin, import ordering, and unused-import rules • Reorganize imports across codebase to follow new import ordering rules • Remove legacy ESLint environment variable and deprecated Prettier option • Add new ESLint plugin dependencies for enhanced linting coverage Diagramflowchart LR
A["Legacy ESLint Config"] -->|"Migrate to Flat Config"| B["eslint.config.js"]
B -->|"Enable Plugins"| C["React Hooks<br/>Jest<br/>Import Order<br/>Unused Imports"]
D["Source Files"] -->|"Reorganize Imports"| E["Grouped & Ordered Imports"]
F["package.json"] -->|"Add Dependencies"| G["@eslint/js<br/>eslint-plugin-import<br/>eslint-plugin-jest<br/>globals"]
H["Prettier Config"] -->|"Remove Deprecated"| I["jsxBracketSameLine Removed"]
File Changes1. eslint.config.js
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a new flat ESLint configuration and updates lint scripts/devDependencies; removes one Prettier option; and makes non-functional import reordering/formatting edits across many source files, plus small local refactors in the renderer App to centralize Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @Mehdi-Bl, 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 significantly upgrades the project's code quality tooling by migrating ESLint to its modern flat configuration. This change introduces a more robust and maintainable linting setup, enabling stricter rules for React hooks, Jest tests, and import statements. The update aims to improve code consistency, catch potential issues earlier, and streamline the development workflow without introducing excessive noise, focusing on a gradual rollout of new rules. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
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
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new flat config uses
@eslint/js9.x features whileeslintin package.json is still pinned to 8.x; consider bumpingeslintto a compatible 9.x version so that the flat config andjs.configs.recommendedare officially supported and behave consistently across environments. - In
App.tsx,electronAPIis derived fromglobalThisbut added as a dependency to theuseEffecthook; if this reference is effectively static at runtime, consider wrapping it inuseMemoor suppressing the exhaustive-deps rule locally to avoid implying that the effect depends on dynamic state.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new flat config uses `@eslint/js` 9.x features while `eslint` in package.json is still pinned to 8.x; consider bumping `eslint` to a compatible 9.x version so that the flat config and `js.configs.recommended` are officially supported and behave consistently across environments.
- In `App.tsx`, `electronAPI` is derived from `globalThis` but added as a dependency to the `useEffect` hook; if this reference is effectively static at runtime, consider wrapping it in `useMemo` or suppressing the exhaustive-deps rule locally to avoid implying that the effect depends on dynamic state.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Dependency ReviewThe following issues were found:
Scanned Files
|
There was a problem hiding this comment.
Code Review
This pull request migrates the ESLint configuration to the new flat config format, which is a great improvement for maintainability. The new configuration is well-structured and enables several useful linting rules for React, Jest, and import ordering. The fix for the react-hooks/exhaustive-deps warning is also correct.
I've found a couple of areas for improvement in the new eslint.config.js file. One is a potential issue with JSX parsing for .jsx files, and the other is a minor redundancy in the TypeScript configuration. My detailed comments are below.
| { | ||
| files: ['**/*.{jsx,tsx}'], | ||
| plugins: { | ||
| react: reactPlugin, | ||
| 'react-hooks': reactHooksPlugin, | ||
| }, | ||
| settings: { | ||
| react: { | ||
| version: 'detect', | ||
| }, | ||
| }, | ||
| rules: { | ||
| ...reactPlugin.configs.recommended.rules, | ||
| 'react-hooks/rules-of-hooks': 'error', | ||
| 'react-hooks/exhaustive-deps': 'error', | ||
| 'react/react-in-jsx-scope': 'off', | ||
| 'react/prop-types': 'off', | ||
| }, | ||
| }, |
There was a problem hiding this comment.
The current configuration for JSX/TSX files might not enable JSX parsing for .jsx files, as the ecmaFeatures: { jsx: true } is only set within the TypeScript-specific configuration block. This could lead to parsing errors for .jsx files.
To ensure JSX is parsed correctly for all relevant files, you should add languageOptions to this block.
{
files: ['**/*.{jsx,tsx}'],
languageOptions: {
parserOptions: {
ecmaFeatures: {
jsx: true,
},
},
},
plugins: {
react: reactPlugin,
'react-hooks': reactHooksPlugin,
},
settings: {
react: {
version: 'detect',
},
},
rules: {
...reactPlugin.configs.recommended.rules,
'react-hooks/rules-of-hooks': 'error',
'react-hooks/exhaustive-deps': 'error',
'react/react-in-jsx-scope': 'off',
'react/prop-types': 'off',
},
},| ecmaVersion: 2022, | ||
| sourceType: 'module', |
Code Review by Qodo
✅ 1.
|
| const js = require('@eslint/js'); | ||
| const globals = require('globals'); | ||
| const importPlugin = require('eslint-plugin-import'); | ||
| const jestPlugin = require('eslint-plugin-jest'); | ||
| const reactPlugin = require('eslint-plugin-react'); | ||
| const reactHooksPlugin = require('eslint-plugin-react-hooks'); | ||
| const tsPlugin = require('@typescript-eslint/eslint-plugin'); | ||
| const tsParser = require('@typescript-eslint/parser'); | ||
| const unusedImportsPlugin = require('eslint-plugin-unused-imports'); | ||
|
|
||
| module.exports = [ | ||
| { | ||
| ignores: [ | ||
| 'node_modules/**', | ||
| 'dist/**', | ||
| 'build/**', | ||
| 'coverage/**', | ||
| 'scripts/**', | ||
| 'src/renderer/bundle.js', | ||
| 'src/renderer/bundle.js.map', | ||
| 'src/renderer/bundle.js.LICENSE.txt', | ||
| 'src/renderer/index.js', | ||
| 'src/renderer/index.js.map', | ||
| 'src/renderer/index.js.LICENSE.txt', | ||
| 'src/renderer/output.css', | ||
| ], | ||
| }, | ||
| js.configs.recommended, | ||
| { | ||
| files: ['src/**/*.{js,jsx,ts,tsx}', 'tests/**/*.{js,jsx,ts,tsx}'], |
There was a problem hiding this comment.
1. @eslint/js major mismatch 🐞 Bug ⛯ Reliability
eslint.config.js requires top-level @eslint/js, but the repo still uses ESLint 8.57.1 (which depends on @eslint/js 8.57.1). This dual-major setup can cause ESLint to load an incompatible recommended ruleset or fail at runtime when initializing lint.
Agent Prompt
## Issue description
`eslint.config.js` imports `@eslint/js`, but the repo installs `@eslint/js@9.x` while ESLint itself is `8.57.1` (and depends on `@eslint/js@8.57.1`). This major-version mismatch can break lint startup or yield unexpected rule compatibility.
## Issue Context
- Flat config explicitly uses `const js = require('@eslint/js')` and `js.configs.recommended`.
- `package-lock.json` shows ESLint 8.57.1 expects `@eslint/js@8.57.1`, but top-level `@eslint/js@9.39.2` is present.
## Fix Focus Areas
- eslint.config.js[1-30]
- package.json[145-183]
- package-lock.json[2635-2647]
- package-lock.json[9749-9762]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@eslint.config.js`:
- Around line 1-9: Legacy .eslintrc.js files are taking precedence over the new
flat config in eslint.config.js so your new rules (see symbols like js,
tsParser, importPlugin, jestPlugin, reactPlugin) aren’t applied; delete the
legacy .eslintrc.js files (root and tests/.eslintrc.js) from the repo, commit
that removal, and re-run the linter/CI to ensure eslint.config.js is now picked
up and the new flat config rules are enforced.
In `@package.json`:
- Around line 20-25: The lint pipeline fails because package.json pins "eslint"
to ^8.56.0 while the repo uses a flat config and an incompatible plugin version;
either upgrade ESLint to v9 (bump "eslint" to a 9.x release in devDependencies
so eslint.config.js is supported and keep existing lint scripts like "lint" /
"lint:tests"), or keep ESLint 8 and make two changes: downgrade
"eslint-plugin-unused-imports" to a 3.x release and prepend ES
LINT_USE_FLAT_CONFIG=true to the npm lint scripts (e.g., the "lint" and
"lint:tests" scripts) so the flat config is enabled at runtime; update
package.json devDependencies and the "lint" / "lint:tests" script entries
accordingly.
| const js = require('@eslint/js'); | ||
| const globals = require('globals'); | ||
| const importPlugin = require('eslint-plugin-import'); | ||
| const jestPlugin = require('eslint-plugin-jest'); | ||
| const reactPlugin = require('eslint-plugin-react'); | ||
| const reactHooksPlugin = require('eslint-plugin-react-hooks'); | ||
| const tsPlugin = require('@typescript-eslint/eslint-plugin'); | ||
| const tsParser = require('@typescript-eslint/parser'); | ||
| const unusedImportsPlugin = require('eslint-plugin-unused-imports'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== ESLint and `@eslint/js` versions in package.json ==="
cat package.json | grep -A 50 '"devDependencies"' | grep -E '"eslint|@eslint/js' | head -10
echo ""
echo "=== Lint scripts in package.json ==="
cat package.json | grep -A 5 '"lint"'
echo ""
echo "=== Check entire scripts section ==="
cat package.json | grep -A 20 '"scripts"'
echo ""
echo "=== Search for ESLINT_USE_FLAT_CONFIG ==="
grep -r 'ESLINT_USE_FLAT_CONFIG' . --include='*.json' --include='*.js' --include='*.sh' 2>/dev/null || echo "Not found"
echo ""
echo "=== Lock file type and ESLint versions ==="
if [ -f "package-lock.json" ]; then
echo "Found package-lock.json"
grep -A 5 '"eslint"' package-lock.json | head -20
elif [ -f "yarn.lock" ]; then
echo "Found yarn.lock"
grep -A 5 'eslint' yarn.lock | head -20
fiRepository: codingworkflow/ai-code-fusion
Length of output: 3384
🏁 Script executed:
#!/bin/bash
echo "=== Check for legacy ESLint config files ==="
find . -maxdepth 2 -name '.eslintrc*' -o -name '.eslintrc' 2>/dev/null | head -20
echo ""
echo "=== Check if eslint.config.js exists ==="
ls -la eslint.config.js 2>/dev/null || echo "File not found"
echo ""
echo "=== Check actual resolved ESLint version in node_modules ==="
if [ -d "node_modules/eslint" ]; then
cat node_modules/eslint/package.json | grep '"version"' | head -1
else
echo "ESLint not installed in node_modules"
fi
echo ""
echo "=== Check `@typescript-eslint` peer dependency requirement ==="
cat package-lock.json | grep -A 10 '@typescript-eslint/eslint-plugin' | grep -A 5 'peerDependencies' | head -15Repository: codingworkflow/ai-code-fusion
Length of output: 716
🏁 Script executed:
#!/bin/bash
echo "=== Check lock file for resolved ESLint version ==="
cat package-lock.json | grep -A 3 '"eslint": {' | grep '"version"' | head -1
echo ""
echo "=== Check `@typescript-eslint/eslint-plugin` peerDependencies ==="
cat package-lock.json | grep -B 5 '@typescript-eslint/eslint-plugin' | grep -A 15 '"peerDependencies"' | head -20
echo ""
echo "=== List the legacy .eslintrc.js files ==="
echo "1. Root .eslintrc.js:"
head -5 .eslintrc.js
echo ""
echo "2. tests/.eslintrc.js:"
head -5 tests/.eslintrc.js
echo ""
echo "=== Check if both configs exist and what's in eslint.config.js ==="
echo "eslint.config.js first few lines:"
head -15 eslint.config.jsRepository: codingworkflow/ai-code-fusion
Length of output: 1867
🏁 Script executed:
#!/bin/bash
echo "=== Check if this is a migration or dual-config situation ==="
echo "Checking git history for eslint config files..."
git log --oneline --all -- '.eslintrc.js' 'eslint.config.js' 2>/dev/null | head -20
echo ""
echo "=== Full content of root .eslintrc.js ==="
cat .eslintrc.js
echo ""
echo "=== Full content of tests/.eslintrc.js ==="
cat tests/.eslintrc.js
echo ""
echo "=== ESLint auto-detection support in different versions ==="
echo "ESLint 8.57+ auto-detects eslint.config.js"
echo "ESLint 8.56 requires ESLINT_USE_FLAT_CONFIG=true"
echo "ESLint 9+ uses flat config by default"
echo ""
echo "Package.json declares: eslint ^8.56.0 (allows 8.56.0 through <9.0.0)"Repository: codingworkflow/ai-code-fusion
Length of output: 2582
🏁 Script executed:
#!/bin/bash
echo "=== Check ESLint behavior with dual configs ==="
echo "ESLint precedence: .eslintrc.* (legacy) > eslint.config.js (flat)"
echo ""
echo "Current state:"
echo "1. Legacy .eslintrc.js exists: YES"
echo "2. Legacy tests/.eslintrc.js exists: YES"
echo "3. Flat eslint.config.js exists: YES"
echo "4. ESLINT_USE_FLAT_CONFIG in scripts: NO"
echo "5. ESLint version range: ^8.56.0 (8.56.0 to <9.0.0)"
echo ""
echo "=== Determining which config will actually be used ==="
echo "With ESLint 8.56-8.56.x: .eslintrc.js (legacy) - flat config ignored unless ESLINT_USE_FLAT_CONFIG=true"
echo "With ESLint 8.57+: auto-detects eslint.config.js, but .eslintrc.js still takes precedence"
echo "With ESLint 9+: eslint.config.js by default, but .eslintrc.js still takes precedence if present"
echo ""
echo "CONCLUSION: Legacy .eslintrc.js will be used regardless of ESLint version."
echo "The new eslint.config.js will be IGNORED."
echo ""
echo "=== Check if PR intended to replace or migrate configs ==="
git show 8ac13b2 --stat 2>/dev/null | head -30Repository: codingworkflow/ai-code-fusion
Length of output: 2607
🏁 Script executed:
#!/bin/bash
echo "=== Verify that old .eslintrc.js was NOT deleted in the commit ==="
git show 8ac13b2 --name-only | grep -E '\.eslintrc|eslint\.config' | head -20
echo ""
echo "=== Check if .eslintrc.js was modified or added ==="
git show 8ac13b2 -- .eslintrc.js | head -50
echo ""
echo "=== Check what the diff shows for .eslintrc.js ==="
git diff 8ac13b2^..8ac13b2 -- .eslintrc.js | head -100Repository: codingworkflow/ai-code-fusion
Length of output: 1485
🏁 Script executed:
#!/bin/bash
echo "=== Confirm ESLint config precedence behavior ==="
echo "According to ESLint docs, when both .eslintrc.* and eslint.config.js exist,"
echo "ESLint will use .eslintrc.* and ignore eslint.config.js"
echo ""
echo "=== Show which config files exist ==="
echo "Legacy configs (will be used):"
ls -la .eslintrc.js tests/.eslintrc.js 2>/dev/null
echo ""
echo "Flat config (will be IGNORED):"
ls -la eslint.config.js
echo ""
echo "=== Verify commit intention ==="
git log --oneline --all -5 | head -10
</script>Repository: codingworkflow/ai-code-fusion
Length of output: 1015
🏁 Script executed:
#!/bin/bash
echo "=== Web search verification: ESLint config precedence ==="
echo "According to official ESLint documentation:"
echo "When both eslint.config.js (flat config) and .eslintrc.* (legacy config) exist,"
echo "ESLint will prefer .eslintrc.* and ignore eslint.config.js"
echo ""
echo "Source: ESLint v8+ and v9+ documentation on configuration files"
echo ""
echo "=== Current state analysis ==="
echo "Files present:"
echo "1. .eslintrc.js (legacy) ✓"
echo "2. tests/.eslintrc.js (legacy) ✓"
echo "3. eslint.config.js (flat) ✓"
echo ""
echo "Result: Legacy .eslintrc.js WILL BE USED"
echo " eslint.config.js WILL BE IGNORED"
echo ""
echo "=== Verify by checking file content differences ==="
echo "Legacy .eslintrc.js extends: eslint:recommended"
grep -i 'extends' .eslintrc.js
echo ""
echo "Flat eslint.config.js uses: js.configs.recommended"
grep -i 'configs.recommended' eslint.config.js || echo "Different approach in flat config"Repository: codingworkflow/ai-code-fusion
Length of output: 1037
Delete legacy .eslintrc.js and tests/.eslintrc.js to complete the flat config migration.
The new eslint.config.js is currently being ignored because ESLint prioritizes legacy .eslintrc.* files over flat config files when both exist. This means all the rule configurations in the new flat config—including the stricter rules, import ordering, and Jest relaxations—are not actually being applied. To activate the flat config, remove the legacy configuration files.
🤖 Prompt for AI Agents
In `@eslint.config.js` around lines 1 - 9, Legacy .eslintrc.js files are taking
precedence over the new flat config in eslint.config.js so your new rules (see
symbols like js, tsParser, importPlugin, jestPlugin, reactPlugin) aren’t
applied; delete the legacy .eslintrc.js files (root and tests/.eslintrc.js) from
the repo, commit that removal, and re-run the linter/CI to ensure
eslint.config.js is now picked up and the new flat config rules are enforced.
|
Addressed Sourcery's review feedback in commit
Re-ran validation:
|
|



Summary
eslint.config.js)src/**, and unused-import cleanupESLINT_USE_FLAT_CONFIG=falseusage from lint scriptseslintto^9.39.2react-hooks/exhaustive-depsfinding inAppwith an explicit mount-only suppression rationale for the stable preload bridge referencejsxBracketSameLine).eslintignore(renamed to.eslintignore.legacy) to avoid ESLint 9 migration warningsScope choices
scripts/**excluded from lint in this phase to avoid dev-tooling noiseimport/orderonly tosrc/**for low-churn rolloutexpect-expect,no-conditional-expect,no-standalone-expect)Validation
npm run lint✅npm test -- --runInBand✅npm run sonarHTTP 401 Unauthorized)Notes
Summary by CodeRabbit