ci: enable perfectionist/sort-modules#7419
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
WalkthroughThe ESLint configuration enables the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Deploying swap-dev with
|
| Latest commit: |
58ec860
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fa654ff6.swap-dev-5u6.pages.dev |
| Branch Preview URL: | https://fix-enable-lint-rule.swap-dev-5u6.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
eslint.config.js (1)
93-111:⚠️ Potential issue | 🟡 MinorInclude
.mtsin the module-sorting glob.This block re-enables
perfectionist/sort-modules, but it only coversjs/ts/jsx/tsx. The repository contains 12.mtsfiles (vite.config.mts across apps and libs) that will bypass the new ordering rule, even though the rest of the ESLint config already treats.mtsas in-scope elsewhere. Update the glob to['**/*.{js,ts,mts}', '**/*.{jsx,tsx}']for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eslint.config.js` around lines 93 - 111, The perfectionist/sort-modules rule's file glob currently excludes `.mts` files so those (e.g., vite.config.mts) bypass ordering; update the `files` glob entry that is paired with the `perfectionist/sort-modules` rule (the array literal assigned to files near the `plugins: { perfectionist }` and `rules: { 'perfectionist/sort-modules': ... }` block) to include `.mts`—change the pattern to include js, ts, and mts (e.g., '**/*.{js,ts,mts}') so `.mts` files are linted by this rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@eslint.config.js`:
- Around line 93-111: The perfectionist/sort-modules rule's file glob currently
excludes `.mts` files so those (e.g., vite.config.mts) bypass ordering; update
the `files` glob entry that is paired with the `perfectionist/sort-modules` rule
(the array literal assigned to files near the `plugins: { perfectionist }` and
`rules: { 'perfectionist/sort-modules': ... }` block) to include `.mts`—change
the pattern to include js, ts, and mts (e.g., '**/*.{js,ts,mts}') so `.mts`
files are linted by this rule.
… fix/enable-lint-rule
fairlighteth
left a comment
fairlighteth
left a comment
There was a problem hiding this comment.
AI review findings to consider:
Medium: eslint.config.js:93 enables perfectionist/sort-modules only for js/ts/jsx/tsx, so .mts files still bypass the new rule. The repo has 12 vite.config.mts files, and I confirmed with pnpm exec eslint --print-config apps/cowswap-frontend/vite.config.mts that perfectionist/sort-modules is absent there. The Nx lint targets also exclude .mts, e.g. apps/cowswap-frontend/project.json:78, so CodeRabbit’s PR comment is still unresolved. Include .mts in both the ESLint rule glob and affected lint patterns, or explicitly document that config modules are intentionally out of scope.
Non-blocking note: apps/cowswap-frontend/src/cow-react/index.tsx:5 moves Messages before the ./sentry side-effect import. Messages is only used as a type, so this should be erased by the TS build, but using import type { Messages } from '@lingui/core' would make that explicit and avoid any side-effect-order ambiguity.
3 participants
Summary
Since Viem migration is done, we can re-enable #7098
In addition I've done:
I've applied
lint:fixto the whole project.Summary by CodeRabbit