chore(prettier): run prettier on lint-staged (blocked by #6594) #6596
Conversation
|
@crutch12 is attempting to deploy a commit to the cow-dev Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis PR consolidates and restructures Prettier configuration across the monorepo by introducing a root Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/cowswap-frontend/src/modules/application/containers/App/FaviconAnimationUpdater.test.ts (1)
1-2: Samesort-imports-ignorecomment consideration as other testsSame note as in
fetchAndProcessQuote.test.ts: consider putting// sort-imports-ignoreon its own line with the explanation on the next line to ensure the Prettier sort-imports plugin reliably honors the ignore.apps/cowswap-frontend/src/modules/application/utils/faviconAnimation/controller.test.ts (1)
1-2: Consistent handling ofsort-imports-ignoreTo stay consistent and avoid any chance of the plugin missing the directive, consider using:
// sort-imports-ignore // disabled for testsas in the earlier recommendation.
apps/cowswap-frontend/src/modules/sounds/utils/sound.test.ts (1)
1-2: Apply the samesort-imports-ignorepattern hereAs with the other test files, consider:
// sort-imports-ignore // disabled for teststo ensure the sort-imports Prettier plugin reliably respects the ignore in this file too.
🧹 Nitpick comments (2)
apps/cowswap-frontend/prettier.config.js (1)
1-5: Thin wrapper over root Prettier config is fine (optional simplification)This correctly reuses the root config. If you don’t plan per‑app overrides here, you could simplify to
module.exports = require('../../prettier.config'), but the current form is perfectly acceptable.apps/cow-fi/prettier.config.js (1)
1-15: Harden plugin pruning against non‑array / non‑string entriesThe intent (dropping the sort‑imports plugin and all
importOrder*keys) matches the comment. To make this more future‑proof, guard againstpluginsbeing missing or containing non‑string entries:-// cow-fi eslint config has been ignoring import/order rules, so we disable prettier import order rules -config.plugins = [...config.plugins].filter((p) => !p.includes('@trivago/prettier-plugin-sort-imports')) +// cow-fi eslint config has been ignoring import/order rules, so we disable prettier import order rules +if (Array.isArray(config.plugins)) { + config.plugins = config.plugins.filter( + (p) => typeof p === 'string' && !p.includes('@trivago/prettier-plugin-sort-imports'), + ) +}The
Object.keys(config)…delete config[key]block is otherwise a clear way to strip allimportOrder*options.
📜 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 (17)
.prettierignore(1 hunks).prettierrc(0 hunks)apps/cow-fi/prettier.config.js(1 hunks)apps/cowswap-frontend/.prettierrc(0 hunks)apps/cowswap-frontend/prettier.config.js(1 hunks)apps/cowswap-frontend/src/modules/application/containers/App/FaviconAnimationUpdater.test.ts(1 hunks)apps/cowswap-frontend/src/modules/application/utils/faviconAnimation/controller.test.ts(1 hunks)apps/cowswap-frontend/src/modules/sounds/utils/sound.test.ts(1 hunks)apps/cowswap-frontend/src/modules/tradeQuote/services/fetchAndProcessQuote.test.ts(1 hunks)apps/cowswap-frontend/vite.config.mts(1 hunks)eslint.config.js(2 hunks)libs/abis/src/index.ts(1 hunks)libs/core/src/jotaiStore.ts(1 hunks)lint-staged.config.js(1 hunks)package.json(2 hunks)prettier.config.js(1 hunks)tools/scripts/install-sdk-preview.js(2 hunks)
💤 Files with no reviewable changes (2)
- .prettierrc
- apps/cowswap-frontend/.prettierrc
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-07-28T16:26:08.051Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6034
File: apps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.ts:44-47
Timestamp: 2025-07-28T16:26:08.051Z
Learning: In the cowswap codebase, using trivial placeholder tests like `it('should be true', () => { expect(true).to.be.true })` in e2e test files is an intentional pattern when disabling broken tests to keep CI green while maintaining build efficiency.
Applied to files:
apps/cowswap-frontend/src/modules/application/utils/faviconAnimation/controller.test.tsapps/cowswap-frontend/src/modules/sounds/utils/sound.test.tsapps/cowswap-frontend/src/modules/application/containers/App/FaviconAnimationUpdater.test.tsapps/cowswap-frontend/src/modules/tradeQuote/services/fetchAndProcessQuote.test.ts
📚 Learning: 2025-09-25T08:48:53.495Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:58-60
Timestamp: 2025-09-25T08:48:53.495Z
Learning: In the cowswap-frontend codebase, when writing SWR tests, the team prefers maximum test isolation by using `provider: () => new Map()` in SWRConfig wrappers, even if it recreates cache on every render, to ensure tests don't share any state.
Applied to files:
apps/cowswap-frontend/src/modules/sounds/utils/sound.test.ts
📚 Learning: 2025-09-25T08:49:32.256Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:62-67
Timestamp: 2025-09-25T08:49:32.256Z
Learning: In the cowswap-frontend codebase, when testing hooks that use multiple bridge providers, both providers are always properly mocked as complete jest.Mocked<BridgeProvider<BridgeQuoteResult>> objects with all required methods stubbed, ensuring no undefined returns that could break the hook logic.
Applied to files:
apps/cowswap-frontend/src/modules/sounds/utils/sound.test.tsapps/cowswap-frontend/src/modules/tradeQuote/services/fetchAndProcessQuote.test.ts
📚 Learning: 2025-09-11T08:25:51.460Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 6234
File: libs/tokens/src/index.ts:1-4
Timestamp: 2025-09-11T08:25:51.460Z
Learning: In the cowprotocol/cowswap project, there is currently no SSR (Server-Side Rendering) support, so localStorage access at module import time does not cause SSR-related issues.
Applied to files:
apps/cowswap-frontend/src/modules/sounds/utils/sound.test.ts
📚 Learning: 2025-08-08T13:56:18.009Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6125
File: libs/tokens/src/updaters/TokensListsUpdater/index.tsx:29-31
Timestamp: 2025-08-08T13:56:18.009Z
Learning: In libs/tokens/src/updaters/TokensListsUpdater/index.tsx, the project’s current Jotai version requires using `unstable_getOnInit` (not `getOnInit`) in atomWithStorage options; keep `{ unstable_getOnInit: true }` until Jotai is upgraded.
Applied to files:
libs/core/src/jotaiStore.ts
🧬 Code graph analysis (2)
apps/cowswap-frontend/prettier.config.js (1)
apps/cow-fi/prettier.config.js (1)
config(1-3)
apps/cow-fi/prettier.config.js (1)
apps/cowswap-frontend/prettier.config.js (1)
config(1-1)
⏰ 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: Socket Security: Pull Request Alerts
🔇 Additional comments (10)
apps/cowswap-frontend/vite.config.mts (1)
8-8: LGTM! Import consolidation improves code organization.These imports were deduplicated and moved to the top of the file, which follows best practices and improves maintainability.
Also applies to: 17-17
libs/core/src/jotaiStore.ts (1)
3-4: AsyncStringStorage import aligns the type usageWiring up the explicit
AsyncStringStorageimport to match the existingstorage: AsyncStringStorageannotation inatomWithIdbStorageis correct and keeps typings in sync without changing runtime behavior..prettierignore (1)
5-7: Ignoring generated and tooling files from Prettier looks appropriateAdding
.cursor/, the generated ABIs under/libs/abis/src/generated, andCHANGELOG.mdto.prettierignoreis reasonable to prevent unwanted formatting noise on non-source or generated content.libs/abis/src/index.ts (1)
4-14: Legacy ABI imports reorganization is non-functional and consistentThe added/relocated legacy ABI imports (_ArgentWalletContractAbi, _Erc20Abi, etc.) line up with the existing exports below, so this is just an import-structure cleanup with no behavioral impact.
eslint.config.js (2)
18-19: Expanded ESLint ignores for build artifacts look goodIgnoring
.nx/,**/.next/,build/, anddist/alongsidestatic-files/is a sensible way to keep ESLint focused on source rather than generated output.
144-149: Turning offimport/orderin favor of Prettier is consistent with the new setupDocumenting that
import/orderis now handled viaprettier.config.jsand disabling the ESLint rule avoids double-enforcement and conflicting fixes. Just make sure the Prettier config (with the sort-imports plugin) runs in both lint-staged and CI for all JS/TS files so import ordering stays enforced.apps/cowswap-frontend/src/modules/tradeQuote/services/fetchAndProcessQuote.test.ts (1)
1-1: Keepsort-imports-ignoreon its own line for plugin compatibilityThe @trivago/prettier-plugin-sort-imports plugin requires the comment to be exactly
// sort-imports-ignoreto recognize the directive. Trailing text prevents the plugin from recognizing it. Update to:// sort-imports-ignore // disabled for teststools/scripts/install-sdk-preview.js (1)
16-45: run() wrapper and token guard look correctMoving logic into
run(), short‑circuiting whenhasSdkPrVersionis false, and failing fast ifPACKAGE_READ_AUTH_TOKENis missing all look sound for this script’s usage;.npmrcis now only written when needed. No changes requested.package.json (1)
273-327: Prettier plugin + lint‑staged versioningAdding
@trivago/prettier-plugin-sort-importsand bumpinglint-stagedaligns with the new Prettier + lint‑staged setup; nothing looks off in this file. Please just confirm these versions match the Node/Prettier versions you run in CI and locally.lint-staged.config.js (1)
2-14: lint‑staged pipeline looks correct (Prettier then ESLint on JS/TS only)The glob, quoting, and conditional ESLint invocation all look good; this will format all targeted files with Prettier and only lint JS/TS when present, with concurrency enabled. Please just double‑check that your installed ESLint version supports
--concurrency autoin all environments (CI and local).
| importOrder: [ | ||
| // react / jotai first | ||
| '^(react|jotai)(/.*)?$', | ||
|
|
||
| // scope packages like @cowprotocol, @uniswap, @safe-global, etc. | ||
| '^@(cowprotocol|uniswap|safe-global|ethersproject|web3-react)(/.*)?$', | ||
|
|
||
| // third parties (analogous to group: external) | ||
| '<THIRD_PARTY_MODULES>', | ||
|
|
||
| // legacy/** | ||
| '^legacy/.*$', | ||
|
|
||
| // built-in Node modules (analogous to group: builtin) | ||
| '<BUILTIN_MODULES>', | ||
|
|
||
| // modules/** | ||
| '^modules/.*$', | ||
|
|
||
| // internal project folders: api, abis, common, constants, etc. | ||
| '^(api|abis|common|constants|lib|pages|types|utils)/.*$', | ||
|
|
||
| // sibling imports (without index) | ||
| '^\\./(?!index$).+$', | ||
|
|
||
| // parent imports | ||
| '^\\.\\.(/.*)?$', | ||
|
|
||
| // index imports | ||
| '^\\.(/)?(index)?$', | ||
|
|
||
| // type imports | ||
| '<THIRD_PARTY_TS_TYPES>', | ||
| '<TS_TYPES>^[./]', | ||
| ], | ||
| importOrderSeparation: true, | ||
| importOrderSortSpecifiers: false, | ||
| importOrderCaseInsensitive: true, | ||
| importOrderSideEffects: false, | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "package.json" -type f | head -5Repository: cowprotocol/cowswap
Length of output: 182
🏁 Script executed:
cat package.json | grep -A 5 -B 5 "prettier-plugin-sort-imports\|prettier"Repository: cowprotocol/cowswap
Length of output: 182
🌐 Web query:
@trivago/prettier-plugin-sort-imports supported tokens documentation
💡 Result:
The plugin supports these special tokens (used inside importOrder):
- <THIRD_PARTY_MODULES> — place third‑party imports. [1]
- <BUILTIN_MODULES> — place Node.js builtin (and node:‑prefixed) modules. [1]
- — explicit group separator when importOrderSeparation is enabled. [1]
(importOrder otherwise accepts an array of regex strings for grouping/sorting). See the plugin README for details. [1][2]
Sources:
[1] trivago/prettier-plugin-sort-imports README (GitHub).
[2] @trivago/prettier-plugin-sort-imports package page (npm).
🌐 Web query:
@trivago/prettier-plugin-sort-imports THIRD_PARTY_TS_TYPES TS_TYPES tokens
💡 Result:
Short answer: @trivago/prettier-plugin-sort-imports does not define tokens named <THIRD_PARTY_TS_TYPES> or <TS_TYPES>. The plugin supports special tokens such as <THIRD_PARTY_MODULES> and <BUILTIN_MODULES> (and other plugins may support a token to group type-only imports). See the plugin README for <THIRD_PARTY_MODULES>/<BUILTIN_MODULES> and an alternative plugin that documents a token. [1][2]
Sources:
[1] @trivago/prettier-plugin-sort-imports README — special words <THIRD_PARTY_MODULES> and <BUILTIN_MODULES>.
[2] IanVS/prettier-plugin-sort-imports README — documents a grouping token used by that plugin.
Type‑import placeholders in importOrder are ineffective with the current plugin
@trivago/prettier-plugin-sort-imports documents only three special importOrder tokens: <THIRD_PARTY_MODULES>, <BUILTIN_MODULES>, and <SEPARATOR>. The entries <THIRD_PARTY_TS_TYPES> and <TS_TYPES>^[./] are not recognized tokens, so they will be treated as literal regex patterns and never match anything, leaving no dedicated type-import grouping despite the comment.
If type-import grouping is desired, consider either removing these entries or migrating to the IanVS prettier-plugin-sort-imports package, which supports a <TYPES> token for that purpose.
🤖 Prompt for AI Agents
In prettier.config.js around lines 12 to 51, the entries
'<THIRD_PARTY_TS_TYPES>' and '<TS_TYPES>^[./]' in the importOrder are not
recognized by @trivago/prettier-plugin-sort-imports and thus act as literal
regexes; remove these two invalid tokens or replace them by migrating to a
plugin that supports a types token (e.g., switch to IanVS's
prettier-plugin-sort-imports and use its '<TYPES>' token), and ensure
importOrder only contains valid special tokens like '<THIRD_PARTY_MODULES>',
'<BUILTIN_MODULES>' or '<SEPARATOR>' so type-import grouping behaves as
intended.
kernelwhisperer
left a comment
kernelwhisperer
left a comment
There was a problem hiding this comment.
As discussed in #6594 (comment) please rebase your changes on the new develop branch
|
Closed in favor of #6716 |
2 participants
Summary
Continuation of this PR: #6594
To Test
git commitEvery new commit should trigger
prettier --writewith changed filesBackground
Performance on my pc for the whole project:
npx eslint . --fix- 65 secondsnpx eslint . --fix --concurrency auto- 45 secondsSummary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.