Upgrade ESLint #529
Upgrade ESLint #529
Conversation
WalkthroughThe pull request removes the Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Script as PackageScripts
participant ESL as ESLint Engine
participant Conf as eslint.config.mjs
Dev->>Script: Run ESLint command
Script->>ESL: Execute linting process
ESL->>Conf: Load linting configuration
Conf-->>ESL: Provide rules and ignore patterns
ESL->>Dev: Return linting results
Possibly related PRs
Suggested reviewers
Poem
Warning Review ran into problems🔥 ProblemsGitHub Actions and Pipeline Checks: Resource not accessible by integration - https://docs.github.com/rest/actions/workflow-runs#list-workflow-runs-for-a-repository. Please grant the required permissions to the CodeRabbit GitHub App under the organization or repository settings. ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
alexeyr-ci2
force-pushed
the
alexeyr/upgrade-eslint
branch
from
f34a288 to
c56301a
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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 (1)
eslint.config.mjs (1)
42-92: Revisit disabled lint rules for tighter code hygiene.Turning off rules such as
no-console,no-restricted-syntax, andimport/no-extraneous-dependenciescan lead to less strict enforcement. Consider re-enabling them (at least setno-consoletowarn) if you wish to maintain stricter standards and catch potential errors or oversights.
📜 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 (4)
.eslintignore(0 hunks)eslint.config.mjs(1 hunks)package-scripts.yml(1 hunks)package.json(2 hunks)
💤 Files with no reviewable changes (1)
- .eslintignore
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-and-test
🔇 Additional comments (8)
package-scripts.yml (1)
59-59:❓ Verification inconclusive
Consider verifying lint coverage for all desired file types.
Removing the
--extoption causes ESLint to rely on default extension detection, which may cause unintentional files to be linted or some files to be skipped depending on your configuration. Ensure that the new approach aligns with your project needs and doesn't degrade lint coverage.
🏁 Script executed:
#!/bin/bash # This script checks for potential coverage of .jsx, .ts, and .tsx files # to see if they're being linted. Adjust the pattern if needed. eslint "packages/**/*.jsx" "packages/**/*.ts" "packages/**/*.tsx" --print-config | grep "filePath"Length of output: 195
Important: Manual Verification of ESLint Configuration Required
Removing the
--extoption means that ESLint now relies on its default extension handling, which could lead to unintended file inclusions or exclusions (e.g., .jsx, .ts, .tsx files). Note that the verification script failed because ESLint wasn’t found (it may not be installed globally). Please manually confirm that:
- ESLint is accessible (consider using
npx eslintif it’s only installed locally).- The lint configuration indeed covers all desired file types, including .jsx, .ts, and .tsx.
- The removal of the
--extoption doesn’t inadvertently degrade lint coverage.eslint.config.mjs (6)
1-17: Good adoption of FlatCompat for legacy configuration bridging.Defining a
FlatCompatinstance to bridge existing configs with the new flat config is a standard approach. This setup appears correct, ensuring your older.eslintrc.*features carry over smoothly.
18-41: Global ignore patterns look appropriate.Excluding coverage, node_modules, and other generated assets is a common best practice. No issues spotted here.
93-133: Strict TypeScript checks are solid.Including
typescript-eslint/configs/strictTypeCheckedand enabling rules like@typescript-eslint/no-shadowwill help catch subtle bugs and improve type safety. Good job!
134-156: Well-configured Jest plugin settings.Enforcing
jest/no-focused-testsand similar rules helps maintain proper test hygiene. This configuration aligns with best practices for test code.
157-170: Effective use of restricted imports for integrations.Prohibiting relative parent imports ensures that integration code only uses the public API, improving modularity and maintainability. Nicely done!
171-174: Ensuring Prettier plugin is last is correct.Placing
prettierRecommendedas the final configuration ensures that Prettier rules override any conflicting style settings from previous configs.package.json (1)
54-54:❓ Verification inconclusive
Check for potential version conflicts when upgrading ESLint and plugins.
You've introduced or updated several ESLint-related dependencies (
@eslint/compat,eslint, plugin packages, etc.). Verify that there are no unmet peer dependencies or version mismatches, especially if other packages still depend on older ESLint versions.Also applies to: 64-65, 67-73, 76-76
🏁 Script executed:
#!/bin/bash # Investigate installed versions of ESLint and its plugins for potential conflicts yarn list --pattern eslint yarn list --pattern eslint-pluginLength of output: 1709
ESLint Dependency Versions Review
The yarn output confirms that most ESLint-related dependencies—including
@eslint/compat(v1.2.8),eslint(v9.24.0), and various plugins—are consistently upgraded. However, note that there is a minor version discrepancy for@eslint/core: one instance is at v0.12.0 while@eslint/plugin-kitpulls in v0.13.0. Although nested dependencies sometimes legitimately use slightly different versions, please verify that this difference won’t lead to runtime issues or unmet peer dependency warnings, especially if any part of the codebase still expects the older version.Action Items:
- Confirm that the minor version difference in
@eslint/coreis intentional and compatible with your ESLint configuration.- Ensure that no other packages are enforcing older ESLint versions which might cause conflicts.
- Re-assess similar dependency updates on lines 64-65, 67-73, and 76-76 of
package.json.
| lint-js-and-ruby: | ||
| docker: | ||
| - image: &docker_image cimg/ruby:3.3.0-browsers | ||
| - image: &docker_image cimg/ruby:3.3.7-browsers |
There was a problem hiding this comment.
The previous version includes Node 20.10, which doesn't have import.meta.dirname. Other Ruby version changes are for consistency with it.
There was a problem hiding this comment.
Or I could replace import.meta.dirname with the older approach.
alexeyr-ci2
force-pushed
the
alexeyr/upgrade-eslint
branch
from
7b82e71 to
7cf6736
Compare
alexeyr-ci2
force-pushed
the
alexeyr/upgrade-eslint
branch
from
7cf6736 to
55275fc
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
eslint.config.mjs (6)
1-11: Confirm the correctness oftypescript-eslintimport.
Most setups rely on packages such as@typescript-eslint/eslint-pluginor@typescript-eslint/parserrather than a generictypescript-eslintimport. Double-check that you have the correct package installed and that this import path is supported by your dependencies.
19-41: Remove duplicate ignore patterns.
You appear to ignore both the top-level folders and their children (e.g.,**/coverageand**/coverage/**/*). This duplication is unnecessary. One ignore pattern per folder typically suffices.Apply this diff to simplify:
- '**/coverage', - '**/coverage/**/*', - '**/node_modules', - '**/node_modules/**/*', + '**/coverage/', + '**/node_modules/',
42-44: Include ctsx/mtsx files if needed.
Currently, patterns cover**/*.[cm][jt]sand**/*.[jt]sx, but not ctsx/mtsx (like.ctsxor.mtsx). If you have or foresee ctsx/mtsx usage, consider incorporating these extensions.
70-97: Reevaluate turning off key ESLint rules.
Rules likeno-console: off,no-restricted-syntax: off, andimport/no-extraneous-dependencies: offcan mask issues, especially for production builds or maintainability. Consider enabling them with lower severities (e.g.,warn) or adjusting them specifically rather than completely disabling.
99-136: Add recommended TypeScript rules for safer code.
You are extendingstrictTypeChecked, which is great. Consider adding additional rules such as@typescript-eslint/no-explicit-any,@typescript-eslint/explicit-function-return-type, or@typescript-eslint/strict-boolean-expressionsif you want stricter checks on your TS codebase.
137-173: Enhance test and integration rules.
For Jest, you may benefit from rules likejest/prefer-expect-assertionsorjest/prefer-lowercase-titleto further standardize test style. For integration files, ensure no unintentional imports are accidentally permitted.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
Gemfile.lockis excluded by!**/*.lockspec/dummy/Gemfile.lockis excluded by!**/*.lockspec/execjs-compatible-dummy/Gemfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
.circleci/config.yml(1 hunks).controlplane/Dockerfile(1 hunks).eslintignore(0 hunks).eslintrc(0 hunks)Gemfile.development_dependencies(1 hunks)eslint.config.mjs(1 hunks)package-scripts.yml(1 hunks)package.json(2 hunks)spec/execjs-compatible-dummy/Dockerfile(1 hunks)spec/execjs-compatible-dummy/Gemfile(1 hunks)
💤 Files with no reviewable changes (2)
- .eslintignore
- .eslintrc
🚧 Files skipped from review as they are similar to previous changes (2)
- package-scripts.yml
- package.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-and-test
🔇 Additional comments (6)
.controlplane/Dockerfile (1)
1-1: Ruby version update looks goodUpdating the base Ruby image from 3.3.0 to 3.3.7 ensures you get the latest security patches and bug fixes. This change aligns with the Ruby version updates in other files.
Gemfile.development_dependencies (1)
6-6: Ruby version update is consistentThe update to Ruby 3.3.7 in the Gemfile.development_dependencies matches the Docker image update, maintaining consistency across development and runtime environments.
spec/execjs-compatible-dummy/Gemfile (1)
5-5: Ruby version update is aligned with other filesThe Ruby version update to 3.3.7 in this test application Gemfile is consistent with the changes made to other files, ensuring uniform Ruby versions across all environments.
spec/execjs-compatible-dummy/Dockerfile (1)
4-4: Ruby version ARG updated properlyThe update of the RUBY_VERSION argument to 3.3.7 ensures that the test Docker container uses the same Ruby version as specified in other configuration files. This maintains consistency across all environments.
.circleci/config.yml (1)
113-116: Ruby Docker Image Version UpgradedThe Docker image for the
lint-js-and-rubyjob has been updated fromcimg/ruby:3.3.0-browserstocimg/ruby:3.3.7-browsers. This change aligns the CI configuration with Ruby version upgrades made elsewhere in the project, ensuring consistency across jobs. The update looks correct and should help avoid potential compatibility issues with newer Ruby enhancements.- - image: &docker_image cimg/ruby:3.3.0-browsers + - image: &docker_image cimg/ruby:3.3.7-browserseslint.config.mjs (1)
174-177: Nice placement of Prettier configuration.
KeepingprettierRecommendedat the end ensures its rules properly override other styling conflicts.
Part 3 of #489, needs #529 to be merged. Closes #489. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Chores** - Upgraded dependencies and streamlined build configurations for improved stability. - Added new steps for installing Node modules and Ruby gems in the CI workflow. - **Refactor** - Simplified component interfaces and standardized code styles. - Improved type safety and clarity in function parameters across various components. - Modified promise handling and control flow for better readability. - Updated object property merging syntax for better clarity. - **Style** - Enhanced accessibility with explicit image alt texts, button type definitions, and clearer label associations. - **Tests** - Refined test assertions for consistency and improved CI workflow for more reliable validations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Alexey Romanov <[email protected]>
Upgrade ESLint and plugins and switch to flat config.
Part 2 of #489, needs #528 to be merged.
Summary by CodeRabbit
Summary by CodeRabbit