Chore: update eslint config as consist#276
Conversation
|
WalkthroughRefactors Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant DevConfig as "Config fragments"
participant Define as "defineConfig()"
participant ESLint as "ESLint engine"
DevConfig->>Define: pass presets (eslint.configs.recommended, tseslint.configs.recommended)
DevConfig->>Define: pass plugin configs (reactHooksPlugin..., reactRefreshPlugin)
DevConfig->>Define: pass file-specific fragments (tsParser parserOptions, ignores, overrides)
Define->>ESLint: produce merged ESLint configuration
ESLint-->>Define: resolved config (rules, parsers, languageOptions)
note over Define,ESLint `#DDFFDD`: New composition order: presets -> plugins -> per-file fragments -> ignores
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
|
Triggered from #276 by @black7375. Checking if we can fast forward Target branch ( commit 1bb50106844d162d26f1f470ef0cae0565461708 (HEAD -> main, origin/main)
Author: JeongJun <rgfdds98@gmail.com>
Date: Wed Oct 15 00:00:00 2025 +0900
Refactor: restrict `rules()` to function-style compoundVariants #245Pull request ( commit 44eef594861456dac2fda5a5faed0e3b0fb09a33 (pull_request/eslint-config)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Thu Oct 16 00:00:00 2025 +0900
Chore: update eslint config as consistIt is possible to fast forward |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
examples/react-babel/eslint.config.js (1)
26-26: Consider consistent file pattern syntax.The file patterns use different glob depths:
- Line 26:
'**/*.{ts,tsx}'(matches recursively)- Line 52:
"*.js"(matches only root level)If JavaScript files in subdirectories should also use
disableTypeChecked, consider using**/*.jsfor consistency. Otherwise, this appears intentional.Also applies to: 52-52
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/react-babel/eslint.config.js(1 hunks)examples/react-swc/eslint.config.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
examples/react-swc/eslint.config.js (1)
configs/eslint-config-custom/eslint.config.typescript.js (1)
eslintConfig(19-71)
examples/react-babel/eslint.config.js (2)
examples/react-swc/eslint.config.js (1)
PACKAGE_ROOT(11-11)configs/eslint-config-custom/eslint.config.typescript.js (1)
eslintConfig(19-71)
🔇 Additional comments (3)
examples/react-babel/eslint.config.js (2)
17-24: Clarify inconsistent parserOptions configurations.There are two separate
parserOptionsconfigurations with different values:
- Lines 20-21: Uses
PACKAGE_ROOT(which resolves toresolve(cwd())), noprojectarray- Lines 30-32: Uses
cwd()directly, includesproject: ["tsconfig.json"]This inconsistency is confusing. The top-level config (lines 17-24) applies to all files, then gets overridden for TypeScript files (lines 25-50). However:
- Why use
PACKAGE_ROOTin one place andcwd()in another when they're functionally equivalent?- Why does only the file-specific config include
project: ["tsconfig.json"]?Please clarify the intended behavior or align both configurations for consistency.
Also applies to: 29-32
44-44: Verify potential duplication of react-hooks rules.This config spreads
reactHooksPlugin.configs.recommended.ruleson line 44, but line 16 already includesreactHooksPlugin.configs.flat.recommendedat the top level. This might duplicate rules or cause conflicts depending on the plugin's structure.Run the verification script from the react-swc config review to check if these configs overlap.
examples/react-swc/eslint.config.js (1)
43-43: Remove redundant react-hooks rules spread on line 43.flat.recommended is a config object packaged for ESLint's flat config system, while recommended.rules is just the legacy rules map you manually copy into your flat config's rules property. Since
reactHooksPlugin.configs.flat.recommendedis already applied at line 16, spreadingreactHooksPlugin.configs.recommended.rulesagain on line 43 duplicates those rules. Remove the spread on line 43:rules: { "react-refresh/only-export-components": [ "warn", { allowConstantExport: true }, ], }⛔ Skipped due to learnings
Learnt from: black7375 Repo: mincho-js/mincho PR: 113 File: packages/css/src/index.ts:33-33 Timestamp: 2024-10-09T13:00:38.449Z Learning: `recipe` is properly exported from './rules' in `packages/css/src/rules/index.ts`.
| parserOptions: { | ||
| project: ["tsconfig.json"], | ||
| tsconfigRootDir: cwd(), | ||
| projectService: true, | ||
|
|
||
| ecmaVersion: "latest", | ||
| sourceType: "module", | ||
| globals: { ...globals.browser, ...globals.es2020 }, | ||
| } |
There was a problem hiding this comment.
Fix incorrect property nesting in languageOptions.
Lines 34-36 (ecmaVersion, sourceType, globals) are incorrectly nested under parserOptions. These properties belong directly under languageOptions, not within parserOptions.
Apply this diff to fix the nesting:
languageOptions: {
parser: tsParser,
parserOptions: {
project: ["tsconfig.json"],
tsconfigRootDir: cwd(),
projectService: true,
-
- ecmaVersion: "latest",
- sourceType: "module",
- globals: { ...globals.browser, ...globals.es2020 },
- }
+ },
+ ecmaVersion: "latest",
+ sourceType: "module",
+ globals: { ...globals.browser, ...globals.es2020 },
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| parserOptions: { | |
| project: ["tsconfig.json"], | |
| tsconfigRootDir: cwd(), | |
| projectService: true, | |
| ecmaVersion: "latest", | |
| sourceType: "module", | |
| globals: { ...globals.browser, ...globals.es2020 }, | |
| } | |
| languageOptions: { | |
| parser: tsParser, | |
| parserOptions: { | |
| project: ["tsconfig.json"], | |
| tsconfigRootDir: cwd(), | |
| projectService: true, | |
| }, | |
| ecmaVersion: "latest", | |
| sourceType: "module", | |
| globals: { ...globals.browser, ...globals.es2020 }, | |
| } |
🤖 Prompt for AI Agents
In examples/react-babel/eslint.config.js around lines 29 to 37, parserOptions
currently contains ecmaVersion, sourceType and globals which are incorrectly
nested; move ecmaVersion, sourceType and globals out of parserOptions and place
them directly under languageOptions at the same level as parserOptions so
languageOptions directly includes parserOptions plus ecmaVersion, sourceType,
and globals.
44eef59 to
26546b1
Compare
|
Triggered from #276 by @black7375. Checking if we can fast forward Target branch ( commit 1bb50106844d162d26f1f470ef0cae0565461708 (HEAD -> main, origin/main)
Author: JeongJun <rgfdds98@gmail.com>
Date: Wed Oct 15 00:00:00 2025 +0900
Refactor: restrict `rules()` to function-style compoundVariants #245Pull request ( commit 26546b1a08cecbbaba4a4895d09657bdc7e7a633 (pull_request/eslint-config)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Thu Oct 16 00:00:00 2025 +0900
Chore: update eslint config as consistIt is possible to fast forward |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
examples/react-babel/eslint.config.js (1)
44-44: Verify redundancy with flat.recommended config.Similar to the react-swc configuration, line 16 already includes
reactHooksPlugin.configs.flat.recommended, which should provide the react-hooks rules. Spreading...reactHooksPlugin.configs.recommended.rules(legacy format) here may be redundant and could potentially conflict with the flat config rules already applied.Consider verifying whether this spread is necessary:
#!/bin/bash # Check if react-hooks rules are already applied by flat.recommended echo "=== Checking react-hooks plugin flat config structure ===" node -e " import reactHooksPlugin from 'eslint-plugin-react-hooks'; console.log('flat.recommended:', Object.keys(reactHooksPlugin.configs.flat?.recommended || {})); console.log('flat.recommended.rules:', Object.keys(reactHooksPlugin.configs.flat?.recommended?.rules || {})); console.log('recommended.rules:', Object.keys(reactHooksPlugin.configs.recommended?.rules || {})); "If
flat.recommendedalready includes the rules, you can remove line 44. If not, consider using the flat config's rules format if available.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
examples/react-babel/eslint.config.js(1 hunks)examples/react-babel/package.json(1 hunks)examples/react-swc/eslint.config.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
examples/react-swc/eslint.config.js (1)
configs/eslint-config-custom/eslint.config.typescript.js (1)
eslintConfig(19-71)
examples/react-babel/eslint.config.js (2)
examples/react-swc/eslint.config.js (1)
PACKAGE_ROOT(11-11)configs/eslint-config-custom/eslint.config.typescript.js (1)
eslintConfig(19-71)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: typecheck
🔇 Additional comments (2)
examples/react-babel/package.json (1)
44-44: Addition of @typescript-eslint/parser dependency is correct.The parser is properly imported at line 7 of eslint.config.js and actively used at line 27 for TypeScript file linting. Version
^8.46.2matches the existingtypescript-eslintdependency. No issues detected.examples/react-swc/eslint.config.js (1)
43-43: Remove the redundant react-hooks rules spread.Line 16 already includes
reactHooksPlugin.configs.flat.recommended, which is the recommended way to configure react-hooks rules in flat config format. Spreading...reactHooksPlugin.configs.recommended.rulesat line 43 (a legacy format) is redundant and applies the same rules twice for*.{ts,tsx}files.Remove line 43 entirely and keep only the global flat config at line 16. The recommended presets automatically include new recommended rules as they are added in future versions, so the manual spread is unnecessary and potentially problematic when the plugin is updated.
| parser: tsParser, | ||
| parserOptions: { | ||
| project: ["tsconfig.json"], | ||
| tsconfigRootDir: cwd(), |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use PACKAGE_ROOT consistently.
Line 20 uses PACKAGE_ROOT for tsconfigRootDir, but line 31 uses cwd() directly. For consistency and clarity, use PACKAGE_ROOT here as well since it's already defined at line 11.
Apply this diff:
parserOptions: {
project: ["tsconfig.json"],
- tsconfigRootDir: cwd(),
+ tsconfigRootDir: PACKAGE_ROOT,
projectService: true,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tsconfigRootDir: cwd(), | |
| parserOptions: { | |
| project: ["tsconfig.json"], | |
| tsconfigRootDir: PACKAGE_ROOT, | |
| projectService: true, |
🤖 Prompt for AI Agents
In examples/react-babel/eslint.config.js around line 31, tsconfigRootDir is set
to cwd() while PACKAGE_ROOT (defined at line 11) should be used for consistency;
replace cwd() with PACKAGE_ROOT so tsconfigRootDir: PACKAGE_ROOT.
|
/fast-forward |
|
Triggered from #276 (comment) by @black7375. Trying to fast forward Target branch ( commit 1bb50106844d162d26f1f470ef0cae0565461708 (HEAD -> main, origin/main)
Author: JeongJun <rgfdds98@gmail.com>
Date: Wed Oct 15 00:00:00 2025 +0900
Refactor: restrict `rules()` to function-style compoundVariants #245Pull request ( commit 26546b1a08cecbbaba4a4895d09657bdc7e7a633 (pull_request/eslint-config)
Author: alstjr7375 <alstjr7375@daum.net>
Date: Thu Oct 16 00:00:00 2025 +0900
Chore: update eslint config as consistFast forwarding $ git push origin 26546b1a08cecbbaba4a4895d09657bdc7e7a633:main
To https://github.com/mincho-js/mincho.git
1bb5010..26546b1 26546b1a08cecbbaba4a4895d09657bdc7e7a633 -> main |
Description
Fix eslint setting.
Related Issue
Summary by CodeRabbit
Additional context
Checklist