!feat: Migrate to ESLint v9. #3
Conversation
- no-useless-assignment add back: no-console: off require-await: off -> warn
replace deprecated eslint rules with stylistic equivalent: - line-comment-position - multiline-comment-style
add new rule: - curly-newline
…g.mjs for linting sources in this repo.
@stylistic/eslint-plugin-jsx add new rule: - jsx-function-call-newline
add rules: - function-component-definition: warn - jsx-indent-props: off (stylistic) - jsx-no-useless-fragment: error - jsx-props-no-multi-spaces: off (stylistic) - no-multi-comp: error
…stic rules to TS.
…ect-property-newline
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (13)
JestConfig.mjs (2)
1-3: Consider explaining your usage of "globals" in a comment.While importing jest globals is straightforward, adding a brief explanation helps new contributors understand where they come from and how they're used.
Line range hint
10-19: Reassess disabling of "no-magic-numbers".Turning off "no-magic-numbers" can allow ephemeral numeric values that reduce code clarity. Consider enabling it with exceptions or disabling it on a case-by-case basis for improved maintainability.
eslint.config.mjs (1)
1-2: Avoid potential naming conflicts with "globals"."globals" is a generic name. Ensure that any future local variable or import doesn’t clash. A descriptive import alias may help avoid confusion.
TsConfigArray.mjs (1)
18-37: Revisit disabling "no-restricted-syntax" and "no-shadow".Although they can be noisy, these rules often detect subtle bugs. You might consider selectively disabling them only for safe scenarios.
ReactConfigArray.mjs (3)
7-23: Streamline plugin registration
You are declaring both the React and React Hooks plugins and referencing them. This is good practice. However, you could potentially import only the rules you need, thereby reducing overhead.
39-45: Clarify function-component-definition rule
Thefunction-component-definitionrule can improve consistency in your React codebase by standardizing how components are declared (arrow vs. function). If you have legacy code, ensure everyone is aware of this new standard.
120-131: Assess overall function length threshold for .jsx/.tsx
Themax-lines-per-functionrule enforces a function length limit of 120 lines, which is typically beneficial. However, confirm it does not cause friction in more complex React components.StylisticConfigArray.mjs (1)
1-1: File-level disabling of certain ESLint rules
Disablingmax-linesandno-magic-numbersis convenient when crafting a dedicated style config. Just ensure that you enable these rules in your final, integrated environment when code size or numeric literals become problematic.CommonConfig.mjs (3)
203-208: Explicitly disallow inline named exports
The restriction on direct exporting of declarations is interesting. This eliminates a common pattern (e.g.,export function foo()). Please ensure the team is aware that only named export lists and default exports are allowed.
250-250: Newly introduced 'no-useless-assignment'
This rule helps catch assignments that serve no purpose. Ensure this does not conflict with patterns like function stubs or placeholders used in rapid prototypes.
300-300: Warn on ‘require-await’
Switching from off to warn forrequire-awaitcan surface numerous warnings if the codebase occasionally uses async functions without awaits. Ensure your devs see these warnings as beneficial.README.md (2)
41-42: Fix incomplete sentence in StylisticConfigArray description.The description appears to be cut off mid-sentence.
-- `StylisticConfigArray`: A configuration for stylistic formatting. It is typically combined with -- `CommonConfig`, as shown in the [ESLint config examples](#eslint-config-examples). +- `StylisticConfigArray`: A configuration for stylistic formatting that is typically combined with + `CommonConfig`, as shown in the [ESLint config examples](#eslint-config-examples).
54-55: Standardize import paths across examples.The first example uses relative paths (
./CommonConfig.mjs), while the others use package paths (eslint-config-yscope/CommonConfig.mjs). This inconsistency might confuse users.-import CommonConfig from "./CommonConfig.mjs"; -import StylisticConfigArray from "./StylisticConfigArray.mjs"; +import CommonConfig from "eslint-config-yscope/CommonConfig.mjs"; +import StylisticConfigArray from "eslint-config-yscope/StylisticConfigArray.mjs";Also applies to: 81-83
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
CommonConfig.mjs(1 hunks)JestConfig.mjs(2 hunks)MeteorConfig.mjs(1 hunks)README.md(2 hunks)ReactConfigArray.mjs(1 hunks)StylisticConfigArray.mjs(1 hunks)TsConfigArray.mjs(1 hunks)common.js(0 hunks)eslint.config.mjs(1 hunks)meteor.js(0 hunks)package.json(2 hunks)react.js(0 hunks)typescript.js(0 hunks)
💤 Files with no reviewable changes (4)
- common.js
- meteor.js
- typescript.js
- react.js
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~34-~34: Loose punctuation mark.
Context: ...e of JavaScript projects. - JestConfig: A Jest-specific ESLint configuration in...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~36-~36: Loose punctuation mark.
Context: ...s for Jest test files. - MeteorConfig: A specialized ESLint configuration for ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~43-~43: Loose punctuation mark.
Context: ...t-config-examples). - ReactConfigArray: A specialized ESLint configuration tail...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~44-~44: Loose punctuation mark.
Context: ...or React applications. - TsConfigArray: A configuration designed specifically f...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (27)
JestConfig.mjs (2)
4-9: Ensure "languageOptions.globals" merges correctly with other configurations.
It's prudent to verify that the jest globals do not unintentionally override or conflict with any other environment-specific globals.
23-23: Properly transitioned to ES module syntax.
Exporting the configuration via export default aligns well with ESLint v9's recommended approach. Good job consolidating the vendored config.
eslint.config.mjs (3)
3-5: Great modular approach.
Splitting configuration into CommonConfig and StylisticConfigArray fosters better maintainability and clarity.
7-17: Confirm merging of node globals with other environment settings.
Your approach merges node globals with the previously defined language options. If you plan to add more environment-specific globals later, ensure there are no naming collisions.
20-20: Consistent export strategy confirmed.
Exporting EslintConfig as default keeps the style uniform across all config files.
MeteorConfig.mjs (4)
1-3: Check usage frequency of Meteor globals.
If Meteor globals are not widely required, consider using a narrower approach to keep the global namespace tidy and reduce risk of overshadowing.
4-11: Praise for layered approach to languageOptions.
Properly merging multiple globals (Jest, Meteor, Node) here is a good example of flexible configuration for multi-environment code.
12-19: Ensure "import/core-modules" references remain updated.
If Meteor packages or module names change, you'll have to adjust this array for accurate resolution by ESLint.
23-23: Well-structured default export.
Exporting the configuration object at the bottom is clear and consistent with project style.
TsConfigArray.mjs (5)
1-2: Double-check plugin versions for compatibility.
Make sure eslint-plugin-import and typescript-eslint are updated to versions supporting ESLint v9 and the strict type-checked configs.
5-7: Commendation for adopting "strictTypeChecked".
Using strictTypeChecked can catch nuanced type errors at an earlier stage, improving code quality.
9-14: Validate parser and parserOptions configuration.
Confirm that the tsconfig.json location is correct and ensure that ESLint is able to parse TS features in your repository’s structure.
38-45: Confirm TypeScript resolver paths for multi-project repositories.
If your repo has multiple TS projects or specialized build setups, ensure the "import/resolver" settings are correct for all subprojects.
50-50: Great job consolidating TS-specific configurations.
This array-based approach to modular TS config is a good pattern, making future expansions straightforward.
ReactConfigArray.mjs (2)
1-3: Consider verifying plugin versions for ESLint v9 compatibility
While this file correctly imports the React and React Hooks plugins, please confirm that the versions of these plugins in your dependencies or peerDependencies support ESLint 9 features.
47-51: Confirm alignment with React Hooks best practices
The react/hook-use-state rule is beneficial, but some teams rely on destructuring in specialized ways. Verify that these settings align with your project’s React Hooks usage patterns.
StylisticConfigArray.mjs (3)
25-26: Check for arrow function syntaxes
The "@stylistic/arrow-parens" and "@stylistic/arrow-spacing" rules unify arrow function formats. This is great for consistency. Ensure your team is on board with these styling conventions.
74-83: Verify indentation settings
You have set a 4-space indentation rule. Confirm your team’s editors and IDEs (e.g., VSCode, WebStorm) are configured to insert four spaces consistently, preventing recurring lint issues.
281-286: Confirm new JSX Curly Newline rule
The @stylistic/jsx-curly-newline rule is newly enabled and can be somewhat strict. Double-check that this does not disrupt multiline styling in existing code.
CommonConfig.mjs (3)
28-50: Applaud consistent usage of fundamental rules
You have a comprehensive set of fundamental ESLint rules like array-callback-return, block-scoped-var, and camelcase—this ensures overall code-base consistency and fewer runtime surprises.
71-73: Validate naming conventions
The id-match rule enforces a naming convention. Please make sure your codebase uses the pattern you desire (e.g., snake_case, camelCase).
146-146: Ensure correct usage of “no-invalid-this”
You have set "no-invalid-this" to ["off"]. If you rely on class-based or more complex this usage, confirm that disabling this rule is intentional.
package.json (2)
3-3: Major version bump
You have updated the version from 0.x to 1.0.0. This signals a new major release. Confirm that consumers of this package are aware of the breaking changes and new ESLint 9 requirement.
15-25: Check for peer dependency alignment
You have updated or added multiple peer dependencies, including eslint and @stylistic/eslint-plugin. Make sure that each project using this config meets the defined peer dependency constraints so that linting remains predictable.
README.md (3)
Line range hint 3-22: LGTM! Clear installation instructions and version compatibility notes.
The introduction and installation section effectively communicates the purpose of the configurations and provides clear guidance on version compatibility.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~34-~34: Loose punctuation mark.
Context: ...e of JavaScript projects. - JestConfig: A Jest-specific ESLint configuration in...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~36-~36: Loose punctuation mark.
Context: ...s for Jest test files. - MeteorConfig: A specialized ESLint configuration for ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~43-~43: Loose punctuation mark.
Context: ...t-config-examples). - ReactConfigArray: A specialized ESLint configuration tail...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~44-~44: Loose punctuation mark.
Context: ...or React applications. - TsConfigArray: A configuration designed specifically f...
(UNLIKELY_OPENING_PUNCTUATION)
31-46: LGTM! Clear distinction between configuration types.
The section effectively explains the difference between standalone and array-based configurations, making it easier for users to understand how to implement them correctly.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~34-~34: Loose punctuation mark.
Context: ...e of JavaScript projects. - JestConfig: A Jest-specific ESLint configuration in...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~36-~36: Loose punctuation mark.
Context: ...s for Jest test files. - MeteorConfig: A specialized ESLint configuration for ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~43-~43: Loose punctuation mark.
Context: ...t-config-examples). - ReactConfigArray: A specialized ESLint configuration tail...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~44-~44: Loose punctuation mark.
Context: ...or React applications. - TsConfigArray: A configuration designed specifically f...
(UNLIKELY_OPENING_PUNCTUATION)
128-143: LGTM! Well-structured additional information.
The customization, contributing, and license sections provide all necessary information concisely, with proper links to additional resources.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
package.json (1)
15-15: Update migration guide for stylistic plugins consolidation.The switch to
@stylistic/eslint-pluginand addition ofeslint-import-resolver-typescriptare good improvements, but they require documentation updates.Let's verify if the migration guide exists:
#!/bin/bash # Description: Check for migration guide # Expect: Documentation about migrating from individual stylistic plugins fd -g "MIGRATION*" -g "*.md" -x cat {} \;Also applies to: 17-17
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package.json(2 hunks)
🔇 Additional comments (2)
package.json (2)
3-3: Version bump to 1.0.0 is appropriate.
The major version increment correctly reflects the breaking changes introduced by the ESLint v9 migration and flat configuration structure.
Let's verify the changelog documentation:
15-26: Verify dependency compatibility and security.
The peer dependencies have been significantly updated for ESLint v9 compatibility. A few points to verify:
- The typescript-eslint v8.18.2 is a flat config compatible version
- All dependencies support ESLint v9's flat config system
Let's verify the compatibility and check for any known security issues:
✅ Verification successful
Dependencies are compatible with ESLint v9 and flat config
Based on the verification results:
- All ESLint plugins support ESLint v9:
- typescript-eslint: "^8.57.0 || ^9.0.0"
- @stylistic/eslint-plugin: ">=8.40.0"
- eslint-plugin-import: "^2 || ... || ^9"
- eslint-plugin-jsdoc: "^7.0.0 || ^8.0.0 || ^9.0.0"
- Other plugins are compatible through their minimum version requirements
- No security vulnerabilities were found in the dependencies
- The project uses flat config (eslint.config.mjs) which is the recommended format for ESLint v9
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify dependency compatibility and security
# Check if dependencies support ESLint v9 flat config
# Check typescript-eslint compatibility
curl -s https://raw.githubusercontent.com/typescript-eslint/typescript-eslint/main/packages/eslint-plugin/package.json | jq '.peerDependencies.eslint'
# Check for any security advisories
gh api graphql -f query='
query {
securityVulnerabilities(first: 10, ecosystem: NPM, package: "eslint-plugin-react") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 530
Script:
#!/bin/bash
# Check compatibility of other ESLint plugins with v9
for pkg in "@stylistic/eslint-plugin" "eslint-plugin-import" "eslint-plugin-import-newlines" "eslint-plugin-jsdoc" "eslint-plugin-no-autofix" "eslint-plugin-react-hooks" "eslint-plugin-simple-import-sort"; do
echo "Checking $pkg..."
curl -s "https://registry.npmjs.org/$pkg/latest" | jq -r '.peerDependencies.eslint'
done
# Check for any ESLint config files to verify flat config usage
fd -e js -e cjs -e mjs --full-path . -x echo "=== {} ===" \; cat {} \; | grep -A 5 "eslint"
Length of output: 3216
Script:
#!/bin/bash
# Check for ESLint config files
fd -e js -e cjs -e mjs -x sh -c 'echo "=== {} ==="; cat "{}"' | grep -A 5 "eslint.config"
# Check if there are any legacy ESLint config files
fd -g '.eslintrc*' -x echo "=== {} ===" \; cat {} \;
Length of output: 507
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
TsConfigArray.mjs (1)
5-25: Consider enhancing JSDoc with usage exampleThe function implementation is solid and follows the new flat config format correctly. Consider adding an @example section to the JSDoc to demonstrate typical usage.
Add an example to the JSDoc:
/** * Generates an override configuration for a TypeScript project specific to * file inclusion, parser options, and import resolution settings. * * @param {string[]} files Paths to include in the configuration. * @param {string} project Path to the TypeScript configuration file (tsconfig.json). * @return {object} The configuration object. + * @example + * const tsOverride = createTsConfigOverride( + * ['src/**/*.ts', 'src/**/*.tsx'], + * './tsconfig.json' + * ); */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
TsConfigArray.mjs(1 hunks)
🔇 Additional comments (4)
TsConfigArray.mjs (4)
1-2: LGTM! Imports are correctly using ESM format
The imports are properly structured for ESLint's flat config system and include the necessary dependencies for TypeScript support.
72-73: LGTM! Exports follow ES module best practices
The export pattern appropriately separates the helper function as a named export while making the config array the default export.
42-46: Verify JSDoc rule configurations
The JSDoc rules are configured to prefer TypeScript types over JSDoc types, which is good practice. However, let's verify this aligns with existing codebase patterns.
✅ Verification successful
JSDoc type rules are correctly configured for TypeScript usage
The configuration is appropriate for a TypeScript project. The file TsConfigArray.mjs is part of an ESLint configuration setup that properly configures TypeScript integration:
- The project uses TypeScript-ESLint parser and strict type checking
- JSDoc type-related rules are correctly disabled since TypeScript types are preferred
- The presence of
tsconfig.jsonreferences and TypeScript-specific ESLint rules confirms this is a TypeScript-oriented setup - The warning for
jsdoc/no-typeshelps maintain consistency by discouraging JSDoc type annotations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for JSDoc comments to verify current documentation patterns
rg -U '\/\*\*[\s\S]*?\*\/' --type ts
Length of output: 123
Script:
#!/bin/bash
# Search for JSDoc comments with fixed regex pattern
rg -U '\\/\\*\\*[\\s\\S]*?\\*\\/' --type ts
# Also search for .js files as the project might use JavaScript
rg -U '\\/\\*\\*[\\s\\S]*?\\*\\/' --type js
# Check for any ESLint config files to understand JSDoc configuration patterns
fd -e js -e json -e yaml -e yml -e cjs -e mjs '.eslintrc'
# Look for tsconfig files to understand TypeScript setup
fd 'tsconfig'
Length of output: 479
Script:
#!/bin/bash
# Let's examine the content of TsConfigArray.mjs to understand the project's TypeScript configuration
cat TsConfigArray.mjs
# Search for any JavaScript files to understand if this is a JavaScript or TypeScript project
fd -e js -e mjs
# Search for TypeScript files
fd -e ts -e tsx
Length of output: 2293
51-51: Specify member ordering configuration
The @typescript-eslint/member-ordering rule is enabled but lacks specific ordering configuration, which might lead to inconsistent code organization.
Consider adding explicit member ordering configuration. I'll verify the current TypeScript codebase's member ordering patterns:
CommonConfig.mjs
Outdated
| ["^node:"], | ||
| [ | ||
| "^meteor", | ||
| "^prop-types", | ||
| "^react", | ||
| ], | ||
| ["^@?\\w"], | ||
| ["^@mui/icons-material"], | ||
| ["^/"], | ||
| ["^"], | ||
| ["^\\."], | ||
| ["^\\u0000"], |
There was a problem hiding this comment.
Can you write a quick example of imports that would be sorted by each rule?
There was a problem hiding this comment.
Also removed group ["^"]
CommonConfig.mjs
Outdated
| // deprecated in ESLint v9.0.0 and replaced by the no-new-native-nonconstructor | ||
| "no-new-symbol": ["error"], |
There was a problem hiding this comment.
If this is deprecated, why are we keeping it?
There was a problem hiding this comment.
Hm, can we remove it entirely or is there a benefit to keeping it?
CommonConfig.mjs
Outdated
| "no-restricted-syntax": [ | ||
| "error", | ||
| { | ||
| message: "Direct exporting of declarations is not allowed. Use export list or default exports instead.", |
CommonConfig.mjs
Outdated
| "warn", | ||
| { | ||
| contexts: [ | ||
| ":has(VariableDeclarator[init.callee.object.name='Object'][init.callee.property.name='freeze'])[declarations]", |
| "eslint-plugin-react-hooks": "^5.1.0", | ||
| "eslint-plugin-simple-import-sort": "^12.1.1", | ||
| "globals": "^15.14.0", | ||
| "typescript-eslint": "^8.18.2" |
There was a problem hiding this comment.
(not directly related to this comment)
How do you feel about enabling Dependabot version updates in our web repos? Reference: https://docs.github.com/en/code-security/dependabot/working-with-dependabot/dependabot-options-reference
There was a problem hiding this comment.
I've enabled all the code security settings for all public repos. If we need to customize them for a particular repo, we can use the local dependabot file, but I guess we won't need to any time soon.
There was a problem hiding this comment.
Right, enabling the code security Dependabot is good enough for receiving PRs for the dependencies that contains security vulnerabilities, but in order to get updates for other dependencies, I believe the local dependabot file is the only missing piece.
With that said, whether we want to keep all our dependencies up to date can be quite opinionated though, lol
There was a problem hiding this comment.
Oh, I see. Sure, I'm not opposed to enabling that. The way I see it, dependabot serves as a notifier and we'd still have the option to reject the update if we have concerns.
StylisticConfigArray.mjs
Outdated
| allowEnumEnd: true, | ||
| allowEnumStart: true, | ||
| allowInterfaceEnd: true, | ||
| allowInterfaceStart: true, | ||
| allowModuleEnd: true, | ||
| allowModuleStart: true, | ||
| allowTypeEnd: true, | ||
| allowTypeStart: true, |
There was a problem hiding this comment.
Start before End to maintain consistency with the attributes above.
TsConfigArray.mjs
Outdated
| * Generates an override configuration for a TypeScript project specific to | ||
| * file inclusion, parser options, and import resolution settings. |
There was a problem hiding this comment.
Can you provide a description for someone who's less familiar with eslint, etc.?
There was a problem hiding this comment.
How do you think about
/**
* Creates a configuration object for a TypeScript project.
*
* This function helps tailor ESLint to work with our TypeScript config by defining the files to
* include, pointing to the TypeScript project configuration file (tsconfig.json), and ensuring
* proper import resolution.
*
(updated inline
TsConfigArray.mjs
Outdated
| * @param {string} project Path to the TypeScript configuration file (tsconfig.json). | ||
| * @return {object} The configuration object. | ||
| */ | ||
| const createTsConfigOverride = (files, project) => ({ |
There was a problem hiding this comment.
How about projectConfigPath instead of project?
TsConfigArray.mjs
Outdated
| * Generates an override configuration for a TypeScript project specific to | ||
| * file inclusion, parser options, and import resolution settings. |
eslint.config.mjs
Outdated
There was a problem hiding this comment.
This is just for this repo, right? Or is my understanding wrong?
There was a problem hiding this comment.
Right, this is for linting sources in this repo.
Co-authored-by: kirkrodrigues <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
README.md (1)
31-46: Enhance clarity of configuration typesConsider adding a brief explanation of why some configurations are standalone while others are array-based. This would help users better understand the architectural decisions and choose the appropriate configuration type for their needs.
Add a note explaining the technical reasons, such as:
Some configurations are provided as standalone configuration objects, which can be directly -referenced once imported: +referenced once imported. These configurations contain core rules that don't require complex setup:🧰 Tools
🪛 LanguageTool
[uncategorized] ~34-~34: Loose punctuation mark.
Context: ...e of JavaScript projects. -JestConfig: A specialized ESLint configuration for ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~35-~35: Loose punctuation mark.
Context: ...uration for Jest files. -MeteorConfig: A specialized ESLint configuration for ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~42-~42: Loose punctuation mark.
Context: ...t-config-examples). -ReactConfigArray: A specialized ESLint configuration for ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~43-~43: Loose punctuation mark.
Context: ...or React applications. -TsConfigArray: A specialized ESLint configuration for ...(UNLIKELY_OPENING_PUNCTUATION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(2 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~34-~34: Loose punctuation mark.
Context: ...e of JavaScript projects. - JestConfig: A specialized ESLint configuration for ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~35-~35: Loose punctuation mark.
Context: ...uration for Jest files. - MeteorConfig: A specialized ESLint configuration for ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~42-~42: Loose punctuation mark.
Context: ...t-config-examples). - ReactConfigArray: A specialized ESLint configuration for ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~43-~43: Loose punctuation mark.
Context: ...or React applications. - TsConfigArray: A specialized ESLint configuration for ...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (3)
README.md (3)
Line range hint
3-22: Well-structured introduction and installation instructions!The introduction clearly explains the purpose and the installation requirements, including backwards compatibility information for older Node.js versions.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~34-~34: Loose punctuation mark.
Context: ...e of JavaScript projects. -JestConfig: A specialized ESLint configuration for ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~35-~35: Loose punctuation mark.
Context: ...uration for Jest files. -MeteorConfig: A specialized ESLint configuration for ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~42-~42: Loose punctuation mark.
Context: ...t-config-examples). -ReactConfigArray: A specialized ESLint configuration for ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~43-~43: Loose punctuation mark.
Context: ...or React applications. -TsConfigArray: A specialized ESLint configuration for ...(UNLIKELY_OPENING_PUNCTUATION)
172-173: Remove extraneous newlinesThese blank lines are unnecessary and should be removed for better readability.
187-188: Retain the License sectionWhile the license is specified in GitHub and package.json, keeping this section in the README is valuable as it:
- Makes the license immediately visible to users viewing the documentation
- Provides a direct link to the full license text
- Follows documentation best practices for open-source projects
README.md
Outdated
| const EslintConfig = [ | ||
| CommonConfig, | ||
| // NOTE: The map below must be placed directly before the spread `StylisticConfigArray`. |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Expand the note about configuration order
The note about placement is crucial but lacks context. Consider expanding it to explain why this ordering is necessary.
-// NOTE: The map below must be placed directly before the spread `StylisticConfigArray`.
+// NOTE: The map below must be placed directly before the spread `StylisticConfigArray`.
+// This ordering is required because TsConfigArray extends from a recommended preset that
+// might conflict with stylistic rules. StylisticConfigArray contains overrides to
+// disable these conflicting rules, so it must come after TsConfigArray.📝 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.
| // NOTE: The map below must be placed directly before the spread `StylisticConfigArray`. | |
| // NOTE: The map below must be placed directly before the spread `StylisticConfigArray`. | |
| // This ordering is required because TsConfigArray extends from a recommended preset that | |
| // might conflict with stylistic rules. StylisticConfigArray contains overrides to | |
| // disable these conflicting rules, so it must come after TsConfigArray. |
There was a problem hiding this comment.
This is good for clarity but also could be overly verbose. @kirkrodrigues what do you think?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
README.md (1)
107-107: 🛠️ Refactor suggestionExpand the note about configuration order
The current note about placement lacks context about why this ordering is necessary.
-// NOTE: The map below must be placed directly before the spread `StylisticConfigArray`. +// NOTE: The map below must be placed directly before the spread `StylisticConfigArray`. +// This ordering is required because TsConfigArray extends from a recommended preset that +// might conflict with stylistic rules. StylisticConfigArray contains overrides to +// disable these conflicting rules, so it must come after TsConfigArray.
🧹 Nitpick comments (1)
README.md (1)
37-45: Add a note about configuration orderingWhile the explanation of configuration types is comprehensive, consider adding a note about the importance of configuration order when combining multiple configurations. This would help users avoid potential rule conflicts.
Add the following paragraph after line 45:
+The order of configurations matters when combining multiple types. For example, when using TypeScript +with stylistic rules, the TypeScript configuration should come before stylistic configurations to +ensure proper rule precedence.🧰 Tools
🪛 LanguageTool
[uncategorized] ~42-~42: Loose punctuation mark.
Context: ...t-config-examples). -ReactConfigArray: A specialized ESLint configuration for ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~43-~43: Loose punctuation mark.
Context: ...or React applications. -TsConfigArray: A specialized ESLint configuration for ...(UNLIKELY_OPENING_PUNCTUATION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(2 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~34-~34: Loose punctuation mark.
Context: ...e of JavaScript projects. - JestConfig: A specialized ESLint configuration for ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~35-~35: Loose punctuation mark.
Context: ...uration for Jest files. - MeteorConfig: A specialized ESLint configuration for ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~42-~42: Loose punctuation mark.
Context: ...t-config-examples). - ReactConfigArray: A specialized ESLint configuration for ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~43-~43: Loose punctuation mark.
Context: ...or React applications. - TsConfigArray: A specialized ESLint configuration for ...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (2)
README.md (2)
Line range hint
3-22: Well-structured introduction and installation guide!The introduction clearly explains the purpose of the package and the installation section effectively addresses version compatibility, including a helpful reference to older versions.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~34-~34: Loose punctuation mark.
Context: ...e of JavaScript projects. -JestConfig: A specialized ESLint configuration for ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~35-~35: Loose punctuation mark.
Context: ...uration for Jest files. -MeteorConfig: A specialized ESLint configuration for ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~42-~42: Loose punctuation mark.
Context: ...t-config-examples). -ReactConfigArray: A specialized ESLint configuration for ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~43-~43: Loose punctuation mark.
Context: ...or React applications. -TsConfigArray: A specialized ESLint configuration for ...(UNLIKELY_OPENING_PUNCTUATION)
175-190: Clear and well-organized final sectionsThe customization, contributing, and license sections provide valuable information, and the links section is well-maintained with proper references.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
ReactConfigArray.mjs (1)
129-134: Consider adjusting the function size limit.The 120-line limit for functions in JSX/TSX files might be too restrictive for complex React components that include prop types, styles, and business logic.
Consider:
- Increasing the limit to 150-200 lines
- Or adding exceptions for specific patterns using
overridesrules: { "max-lines-per-function": ["error", { - max: 120, + max: 150, skipBlankLines: true, skipComments: true, }], },README.md (1)
185-185: Remove unused license link reference.The license link reference is not used in the document and can be removed.
-[license]: https://github.com/y-scope/eslint-config-yscope/blob/main/LICENSE🧰 Tools
🪛 Markdownlint (0.37.0)
185-185: Unused link or image reference definition: "license"
Link and image reference definitions should be needed(MD053, link-image-reference-definitions)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(2 hunks)ReactConfigArray.mjs(1 hunks)package.json(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~34-~34: Loose punctuation mark.
Context: ...e of JavaScript projects. - JestConfig: A specialized ESLint configuration for ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~35-~35: Loose punctuation mark.
Context: ...uration for Jest files. - MeteorConfig: A specialized ESLint configuration for ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~42-~42: Loose punctuation mark.
Context: ...t-config-examples). - ReactConfigArray: A specialized ESLint configuration for ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~43-~43: Loose punctuation mark.
Context: ...or React applications. - TsConfigArray: A specialized ESLint configuration for ...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 Markdownlint (0.37.0)
README.md
185-185: Unused link or image reference definition: "license"
Link and image reference definitions should be needed
(MD053, link-image-reference-definitions)
🔇 Additional comments (5)
package.json (3)
3-3: LGTM! Version bump is appropriate.The major version bump to 1.0.0 aligns with the significant changes introduced by migrating to ESLint v9 and the flat configuration format.
12-17: LGTM! Metadata updates are comprehensive.The addition of author email, license, bugs URL, and homepage information improves the package's metadata completeness.
19-30: Verify typescript-eslint version compatibility.The specified version of typescript-eslint (^8.18.2) seems unusually high, as the latest stable version is in the 6.x range.
ReactConfigArray.mjs (1)
53-56: LGTM! Well-documented rule management.The comment clearly explains the relationship between eslint-plugin-react and @stylistic/eslint-plugin-jsx rules, helping future maintainers understand the configuration decisions.
README.md (1)
3-5: LGTM! Clear introduction and purpose statement.The introduction effectively communicates the purpose and benefits of the configuration package.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
StylisticConfigArray.mjs (2)
4-5: Remove extra blank lineThere's an unnecessary extra blank line after the import statement.
import StylisticPlugin from "@stylistic/eslint-plugin"; -
347-350: Consider relaxing the curly-newline ruleSetting
@stylistic/curly-newlineto "always" might be too restrictive. Consider using "consistent" instead to allow more flexible formatting while maintaining consistency."@stylistic/curly-newline": [ "error", - "always", + "consistent",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
StylisticConfigArray.mjs(1 hunks)
🔇 Additional comments (2)
StylisticConfigArray.mjs (2)
6-12: Configuration setup looks good!The configuration properly initializes the stylistic plugin and disables legacy rules as required for ESLint v9.
266-346: Well-structured JSX formatting rulesThe JSX-specific rules are comprehensive and follow modern React best practices. The configuration ensures consistent formatting of JSX elements, props, and multiline components.
| "error", | ||
| "always", | ||
| ], | ||
| "@stylistic/func-call-spacing": ["off"], |
There was a problem hiding this comment.
Remove duplicate rule definition
The @stylistic/function-call-spacing rule is defined twice, on lines 62 and 67. Remove one of the definitions.
- "@stylistic/func-call-spacing": ["off"],Also applies to: 67-67
There was a problem hiding this comment.
Added a comment to explain why it is off: Deprecated by @stylistic/function-call-spacing.
There was a problem hiding this comment.
@junhaoliao, understood! Thanks for clarifying that @stylistic/func-call-spacing is intentionally off because it’s deprecated by @stylistic/function-call-spacing. We can keep it as is.
✏️ Learnings added
Learnt from: junhaoliao
PR: y-scope/eslint-config-yscope#3
File: StylisticConfigArray.mjs:64-64
Timestamp: 2025-01-04T16:58:29.571Z
Learning: The `@stylistic/func-call-spacing` rule is set to "off" because it was replaced by or deprecated in favour of `@stylistic/function-call-spacing`, as explained in the StylisticConfigArray.mjs file.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
TsConfigArray.mjs (2)
16-28: Consider adding input parameter validation.The function could benefit from validating that
filesis a non-empty array andprojectConfigPathis a non-empty string.const createTsConfigOverride = (files, projectConfigPath) => ({ + if (!Array.isArray(files) || files.length === 0) { + throw new TypeError('files must be a non-empty array'); + } + if (typeof projectConfigPath !== 'string' || !projectConfigPath.trim()) { + throw new TypeError('projectConfigPath must be a non-empty string'); + } files: files,
31-73: Extract duplicated tsconfig path to a constant.The path "tsconfig.json" appears in both the parser options and import resolver settings. Consider extracting it to a constant to follow the DRY principle.
+const DEFAULT_TSCONFIG_PATH = "tsconfig.json"; + const TsConfigArray = [ ...TsEslint.configs.strictTypeChecked, importPlugin.flatConfigs.typescript, { languageOptions: { parser: TsEslint.parser, parserOptions: { - project: "tsconfig.json", + project: DEFAULT_TSCONFIG_PATH, }, }, // ... other config settings: { "import/resolver": { typescript: { alwaysTryTypes: true, - project: "tsconfig.json", + project: DEFAULT_TSCONFIG_PATH, }, }, }, }, ];CommonConfig.mjs (1)
444-444: Consider breaking long line into multiple lines.The context selector in the require-jsdoc rule exceeds the typical line length limit.
- ":has(VariableDeclarator[init.callee.object.name='Object'][init.callee.property.name='freeze'])[declarations]", + [ + ":has(VariableDeclarator", + "[init.callee.object.name='Object']", + "[init.callee.property.name='freeze']", + ")[declarations]" + ].join(""),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CommonConfig.mjs(1 hunks)TsConfigArray.mjs(1 hunks)
🔇 Additional comments (4)
TsConfigArray.mjs (2)
1-15: Well-structured imports and documentation!The JSDoc comments are clear and informative, providing good documentation for the
createTsConfigOverridefunction.
75-76: Clean and clear exports!The export structure is well-organized, providing both named and default exports.
CommonConfig.mjs (2)
1-8: Well-organized imports and structure!The disable directive for max-lines is appropriate for a configuration file of this nature.
473-484: Well-configured JSDoc settings!The TypeScript mode and tag name preferences are appropriately configured.
CommonConfig.mjs
Outdated
| // deprecated in ESLint v9.0.0 and replaced by the no-new-native-nonconstructor | ||
| "no-new-symbol": ["error"], |
There was a problem hiding this comment.
Remove deprecated rule.
The no-new-symbol rule is deprecated in ESLint v9.0.0 and has been replaced by no-new-native-nonconstructor. Remove the deprecated rule since its replacement is already configured.
- // deprecated in ESLint v9.0.0 and replaced by the no-new-native-nonconstructor
- "no-new-symbol": ["error"],There was a problem hiding this comment.
@junhaoliao Understood. We'll keep the rule in place for now as per your preference. If you’d like to remove it in the future, just let me know.
CommonConfig.mjs
Outdated
| const CommonConfig = { | ||
| languageOptions: { | ||
| ecmaVersion: "latest", | ||
| sourceType: "module", | ||
| parserOptions: { | ||
| ecmaFeatures: { | ||
| experimentalObjectRestSpread: true, | ||
| }, | ||
| }, | ||
| }, | ||
| plugins: { | ||
| "import": importPlugin, | ||
| "import-newlines": ImportNewlinesPlugin, | ||
| "jsdoc": JsdocPlugin, | ||
| "no-autofix": NoAutofixPlugin, | ||
| "simple-import-sort": SimpleImportSortPlugin, | ||
| }, |
There was a problem hiding this comment.
💡 Codebase verification
Remove deprecated experimentalObjectRestSpread configuration
The experimentalObjectRestSpread option should be removed as it's no longer needed. Object rest/spread is a stable feature in modern JavaScript and is fully supported when using ecmaVersion: "latest". The option was deprecated several versions ago and is redundant in current ESLint configurations.
- Remove the following configuration block from
CommonConfig.mjs:
parserOptions: {
ecmaFeatures: {
experimentalObjectRestSpread: true,
},
},🔗 Analysis chain
Verify compatibility with experimental features.
The use of experimentalObjectRestSpread with ecmaVersion: "latest" might need verification, as this feature might be stable in recent versions and not require the experimental flag.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the experimental flag is still needed
ast-grep --pattern 'experimentalObjectRestSpread: true' | while read -r match; do
echo "Found experimental flag usage at: $match"
done
Length of output: 73
Script:
#!/bin/bash
# Let's search for the config file and check its content
fd -e mjs CommonConfig | xargs cat
# Also search for any ESLint configuration files that might provide insights
fd -e js -e json -e yaml -e yml -g '*eslint*' | xargs cat
Length of output: 16658
There was a problem hiding this comment.
@junhaoliao, thanks for confirming the removal of the deprecated experimentalObjectRestSpread configuration; that resolves the issue promptly. Great work!
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
Description
Migrate to ESLint v9 flat config
CommonConfigandStylisticConfigArray)ReactConfigArrayTsConfigArrayJestConfigMeteorConfigno-useless-assignment@stylistic/curly-newline@stylistic/jsx-function-call-newlineeslintConfigin package.json and addeslint.config.mjsfor linting sources in this repo.Other rule changes
no-console: off(bookkeeping)require-await: off -> warn@stylistic/member-delimiter-style@stylistic/object-property-newline@stylistic/comma-dangle@stylistic/lines-around-commentpadding-line-between-statementssimple-import-sort/imports, group^@imports with^\\w.Validations
Summary by CodeRabbit
Release Notes
New Features
Dependencies
Configuration
Breaking Changes
eslint.config.mjssetup for projects.