chore(prettier): import order #6594
Conversation
|
@crutch12 is attempting to deploy a commit to the cow Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughConsolidates Prettier configuration from multiple local files into a centralized root configuration with the 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)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ 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)
🔇 Additional comments (3)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 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 (20)
.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/common/hooks/useNavigate.ts(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)apps/explorer/src/storybook/decorators.tsx(1 hunks)apps/widget-configurator/src/app/embedDialog/index.tsx(1 hunks)eslint.config.js(1 hunks)libs/abis/src/index.ts(1 hunks)libs/core/src/gnosisSafe/index.ts(1 hunks)libs/core/src/jotaiStore.ts(1 hunks)package.json(1 hunks)prettier.config.js(1 hunks)tools/scripts/install-sdk-preview.js(2 hunks)
💤 Files with no reviewable changes (2)
- apps/cowswap-frontend/.prettierrc
- .prettierrc
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-08-05T14:27:05.023Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 5992
File: libs/wallet/src/web3-react/utils/switchChain.ts:36-38
Timestamp: 2025-08-05T14:27:05.023Z
Learning: In libs/wallet/src/web3-react/utils/switchChain.ts, the team prefers using Record<SupportedChainId, string | null> over Partial<Record<SupportedChainId, string>> for WALLET_RPC_SUGGESTION to enforce that all supported chain IDs have explicit values set, even if some might be null. This ensures compile-time completeness checking.
Applied to files:
libs/core/src/gnosisSafe/index.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:
libs/core/src/gnosisSafe/index.tsapps/explorer/src/storybook/decorators.tsxapps/cowswap-frontend/src/modules/sounds/utils/sound.test.tsapps/widget-configurator/src/app/embedDialog/index.tsxapps/cowswap-frontend/vite.config.mtsapps/cowswap-frontend/src/common/hooks/useNavigate.ts
📚 Learning: 2025-09-19T11:38:59.206Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6232
File: apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx:199-200
Timestamp: 2025-09-19T11:38:59.206Z
Learning: The makeBuildClickEvent function in apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx takes five parameters: defaultChainId, contextLabel, mode, isSwapMode, and chainsCount. The chainsCount parameter is used to determine the CrossChain flag in analytics events.
Applied to files:
libs/core/src/gnosisSafe/index.ts
📚 Learning: 2025-06-16T15:58:00.268Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 5830
File: apps/cowswap-frontend/src/modules/trade/containers/TradeWidget/index.tsx:1-2
Timestamp: 2025-06-16T15:58:00.268Z
Learning: JSX can be imported as a named export from React in modern React versions (React 17+). The import `import { JSX } from 'react'` is valid and does not cause compilation errors.
Applied to files:
apps/explorer/src/storybook/decorators.tsxapps/widget-configurator/src/app/embedDialog/index.tsxapps/cowswap-frontend/src/common/hooks/useNavigate.ts
📚 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/containers/App/FaviconAnimationUpdater.test.tsapps/cowswap-frontend/src/modules/sounds/utils/sound.test.tsapps/cowswap-frontend/src/modules/application/utils/faviconAnimation/controller.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/application/utils/faviconAnimation/controller.test.ts
📚 Learning: 2025-02-20T15:59:33.749Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5443
File: apps/cowswap-frontend/src/modules/swap/containers/ConfirmSwapModalSetup/index.tsx:71-71
Timestamp: 2025-02-20T15:59:33.749Z
Learning: The swap module in apps/cowswap-frontend/src/modules/swap/ is marked for deletion in PR #5444 as part of the swap widget unification effort.
Applied to files:
apps/cowswap-frontend/vite.config.mts
📚 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/cow-fi/prettier.config.js (1)
apps/cowswap-frontend/prettier.config.js (1)
config(1-1)
apps/cowswap-frontend/prettier.config.js (2)
apps/cow-fi/prettier.config.js (1)
config(1-3)eslint.config.js (1)
require(1-1)
🔇 Additional comments (18)
apps/explorer/src/storybook/decorators.tsx (1)
1-1: LGTM!The
// sort-imports-ignoredirective correctly excludes this Storybook decorators file from automatic import sorting, which is appropriate given the specific import ordering requirements for test/story files.apps/cowswap-frontend/src/modules/application/containers/App/FaviconAnimationUpdater.test.ts (1)
1-2: LGTM!The directive appropriately excludes this test file from import sorting. Test files with
jest.mock()declarations often require specific import ordering for clarity and to ensure mocks are set up before the mocked modules are imported.apps/cowswap-frontend/src/modules/sounds/utils/sound.test.ts (1)
1-2: LGTM!Consistent with the pattern established for test files—the directive prevents import sorting that would disrupt the intentional placement of imports after
jest.mock()declarations.libs/core/src/jotaiStore.ts (1)
3-5: LGTM!The blank line between the
jotaiimports and thelocalforageimport correctly groups related imports according to the new Prettier import ordering configuration.apps/cowswap-frontend/vite.config.mts (1)
1-1: LGTM!The directive correctly excludes this Vite configuration file from import sorting. Configuration files often have intentionally structured imports, and this file already contains ESLint overrides (line 19-20) for restricted imports, making it a suitable candidate for exclusion.
.prettierignore (1)
5-7: LGTM!The additions to
.prettierignoreare appropriate:
- Ignoring
/libs/abis/src/generatedprevents Prettier from reformatting auto-generated code.- Ignoring
CHANGELOG.mdpreserves its specific formatting.These align with the PR's objectives regarding handling files with special import ordering requirements.
libs/core/src/gnosisSafe/index.ts (1)
1-1: LGTM!The
// sort-imports-ignoredirective is appropriately placed to prevent the Prettier import sorting plugin from reordering imports in this file. The comment clarifies that this is due to theno-restricted-importsESLint rule on line 9, which requires theethersimport to be positioned after the disable comment.apps/cowswap-frontend/src/modules/tradeQuote/services/fetchAndProcessQuote.test.ts (1)
1-1: LGTM!The
// sort-imports-ignoredirective is correctly applied. Test files withjest.mock()calls that precede imports (lines 5-30) require this specific ordering to ensure mocks are properly set up before the actual modules are imported. Automatic import sorting would break the mock setup.libs/abis/src/index.ts (1)
1-1: LGTM!The
// sort-imports-ignoredirective correctly addresses the incompatibility between ESLint'simport/orderrule and the Prettier plugin's alphabetic sorting implementation. As noted in the PR description, this file was specifically identified as having sorting conflicts that would cause cyclic changes betweeneslint --fixandprettier --write.apps/widget-configurator/src/app/embedDialog/index.tsx (1)
1-1: LGTM!The
// sort-imports-ignoredirective is correctly applied. Lines 24-25 contain aneslint-disable-next-linecomment for theno-restricted-importsrule on thenightOwlimport. Without this directive, the Prettier plugin could reorder imports and separate the disable comment from its target import, breaking the ESLint override.apps/cowswap-frontend/src/common/hooks/useNavigate.ts (1)
1-1: LGTM!The
sort-imports-ignoredirective is appropriately placed to prevent the Prettier plugin from reordering imports in this file, which needs to bypass theno-restricted-importsrule foruseNavigate.eslint.config.js (2)
144-150: Good documentation addition.The comment linking ESLint's
import/orderrules toprettier.config.jsimproves maintainability by making the connection explicit.
156-164: LGTM!The
jotai/**pattern correctly matches nested subpaths (e.g.,jotai/vanilla/utils), and removing the duplicate@cowprotocolentry cleans up the configuration.apps/cowswap-frontend/src/modules/application/utils/faviconAnimation/controller.test.ts (1)
1-2: LGTM!The
sort-imports-ignoredirective is necessary here becausejest.mock()must be hoisted before the imports that depend on the mocked module. Automatic import sorting would break this test.tools/scripts/install-sdk-preview.js (1)
16-45: LGTM!The refactoring into a
run()function maintains identical behavior while improving code organization. MovingPACKAGE_READ_AUTH_TOKENinto the function scope is appropriate since it's only used there.apps/cowswap-frontend/prettier.config.js (1)
1-5: LGTM!Clean wrapper that forwards the root Prettier configuration to this app. The spread syntax allows for potential future app-specific overrides if needed.
apps/cow-fi/prettier.config.js (1)
1-15: LGTM!The configuration correctly disables import sorting for the cow-fi app by removing the plugin and all
importOrderkeys. This aligns with the PR description noting that cow-fi's ESLint config ignoresimport/orderrules.prettier.config.js (1)
14-52: Import order configuration looks well-structured.The
importOrderpatterns appropriately mirror the ESLintimport/orderrules, withimportOrderSeparation: truematching ESLint'snewlines-between: 'always'andimportOrderCaseInsensitive: truealigning with ESLint's case-insensitive sorting.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
prettier.config.js(1 hunks)
🔇 Additional comments (4)
prettier.config.js (4)
1-4: LGTM: Standard Prettier configuration.The basic Prettier settings are appropriate and commonly used.
49-52: LGTM: Plugin options configured appropriately.The import order options are set correctly. Note that
importOrderSortSpecifiers: falsemeans named imports within braces won't be alphabetically sorted (e.g.,import { b, a }remains as-is), which preserves developer intent.
5-5: Verify the plugin dependency is installed.Ensure that
@trivago/prettier-plugin-sort-importsis listed inpackage.jsondependencies or devDependencies.
28-28: Verify that built-in modules placement matches ESLint rules.The
<BUILTIN_MODULES>placeholder appears after third-party modules and legacy imports, which is unconventional. Typically, built-in Node modules are sorted early in import order. Confirm this ordering aligns with your ESLintimport/orderconfiguration.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
eslint.config.js(2 hunks)prettier.config.js(1 hunks)
🔇 Additional comments (3)
prettier.config.js (2)
1-5: LGTM! Basic Prettier configuration is well-structured.The core formatting options (semi, singleQuote, printWidth) and plugin declaration are appropriate for the project.
49-52: VerifyimportOrderSideEffects: falsebehavior.Setting
importOrderSideEffects: falseexcludes side-effect imports (e.g.,import './styles.css') from sorting. Ensure this is intentional, as side-effect imports will remain in their original positions rather than being grouped.eslint.config.js (1)
154-179: LGTM! Import order pathGroups are well-organized.The changes appropriately:
- Expand
jotai/*tojotai/**for matching nested subpaths- Remove the duplicate
@cowprotocolentry- Remove
legacyfrom the internal paths group (since it has its own dedicated group at lines 166-169)These adjustments ensure consistency with the Prettier
importOrderconfiguration.
crutch12
force-pushed
the
chore/prettier-import-order
branch
from
4529ff8 to
1f77b8f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
prettier.config.js (1)
45-47: Consider documenting the TS type placeholders usage for future maintainersUsing
<THIRD_PARTY_TS_TYPES>and<TS_TYPES>^[./]is a nice touch to keepimport typeblocks grouped, but since these placeholders are not obvious from the plugin README, future readers may not realize they exist or which plugin/version they rely on. Consider adding a brief inline comment with a link to the upstream issue/comment you referenced (prettier-plugin-sort-imports #202) so others understand why these entries are here and don’t “simplify” them away.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
eslint.config.js(2 hunks)prettier.config.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- eslint.config.js
🔇 Additional comments (1)
prettier.config.js (1)
1-5: Centralized Prettier/import-order config looks consistent and well-scopedThe overall Prettier setup (core options +
@trivago/prettier-plugin-sort-importswithimportOrder*flags) is coherent, and the grouping patterns line up with the described ESLintimport/orderrules. The structure should be easy to maintain as long as changes to import rules continue to be mirrored here.Also applies to: 14-44, 49-52
crutch12
force-pushed
the
chore/prettier-import-order
branch
from
1f77b8f to
2c51777
Compare
| // cow-fi eslint config ignores import/order rules, so we disable prettier import order rules | ||
| config.plugins = [...config.plugins].filter((p) => !p.includes('@trivago/prettier-plugin-sort-imports')) | ||
|
|
||
| Object.keys(config) |
There was a problem hiding this comment.
if you don't like this solution, I could extract base config into prettier.base.js file and use it in both configs
just didn't want to produce extra files...
|
Hey @crutch12 thank you very much for this PR. I was planning to integrate prettier into eslint using I think this where the convenience of using prettier's import sort coming from? Are there some other reason to use it? |
|
The main reason for me is to fix as much problems as possible during So Step 1. I want to offload I'm not sure if it's possible to do with |
|
Yes, makes sense, that's exactly what 
Unfortunately, I think we will stick with eslint import sorting, it's already setup, heavily configured, etc. Please take a look at #6637, as soon as it's merged prettier and eslint will run on file save and you can forget about it. Could you migrate your other changes and base them on that PR? 🙏 I like what you did with #6596 in particular to ignore patterns, |
Just want to clarify: new prettier config setup completely repeats current eslint -- Thanks for explanation, I'll check your pr and eslint prettier plugin. I hope it runs as fast as original prettier does. |
Got it, #6637 was merged into develop, please try that out and base your changes on the new develop version. |
kernelwhisperer
left a comment
kernelwhisperer
left a comment
There was a problem hiding this comment.
As discussed above
|
Closed in favor of #6716 |
3 participants
Summary
I believe it is much more convenient to sort imports with
prettier(e.g. on file save), so I've added @trivago/prettier-plugin-sort-imports settings toprettier.config.jsThese settings are fully compatible with current eslint's
import/orderrules.I've tested it with multiple runs:
And there are no cyclic repetitive changes.
To Test
npx prettier <filename> --write)Every import should be sorted accordingly to eslint config (you see no eslint
import/ordererrors)Here is the list of files I've been using to test new prettier settings:
Background
import/orderrules have different alphabetic sorting implementations, thus I have to ignorecowswap/libs/abis/src/index.tsfile (prettier uses javascript-natural-sort)apps/cow-fi/eslint.config.jsignoresimport/orderrules, so I've disabledimportOrdersettings there too (seeapps/cow-fi/prettier.config.js)eslint no-restricted-importsProposals
import/orderrulesSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.