feat: pin all versions and harden build#7450
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
WalkthroughThis pull request updates dependency versions across the entire monorepo by pinning most packages to exact versions (removing semver ranges), upgrading numerous libraries to newer releases, converting tarball URLs to registry versions, and adjusting Node.js/pnpm engine constraints and pnpm configuration settings. ChangesDependency Pinning and Version Upgrade
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
| "@apollo/client": "3.8.8", | ||
| "@cowprotocol/analytics": "workspace:*", | ||
| "@cowprotocol/assets": "workspace:*", | ||
| "@cowprotocol/cms": "https://registry.npmjs.org/@cowprotocol/cms/-/cms-0.11.0.tgz", |
There was a problem hiding this comment.
This one I might have to revert.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| "@cowprotocol/common-hooks": "workspace:*", | ||
| "@cowprotocol/common-utils": "workspace:*", | ||
| "@cowprotocol/core": "workspace:*", | ||
| "@cowprotocol/cow-runner-game": "https://registry.npmjs.org/@cowprotocol/cow-runner-game/-/cow-runner-game-0.2.9.tgz", |
There was a problem hiding this comment.
This one I might have to revert.
| packages: | ||
| - 'apps/*' | ||
| - 'libs/*' | ||
|
|
There was a problem hiding this comment.
These were already defined in .npmrc which is not specific to PNPM, so I prefer to have it there.
| "bufferutil" | ||
| ] | ||
| }, | ||
| "onlyBuiltDependencies": [ |
There was a problem hiding this comment.
This is for PNPM < 10. For PNPM >= 10 (as defined in the project), strictDepBuilds + allowBuilds take care of this.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
libs/analytics/package.json (1)
35-35: 💤 Low valueLGTM — pinning; consider upgrading
web-vitalsto v4.xPinning
web-vitalsto the already-resolved2.1.4is safe. As an optional follow-up,web-vitalsis now at v4.x, which adds support for Core Web Vitals INP and attribution data. Staying on v2.1.4 is fine for this PR but worth a separate tracking issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/analytics/package.json` at line 35, Package.json currently pins the dependency "web-vitals": "2.1.4"; update the dependency entry in libs/analytics package.json to a v4.x release (e.g., "web-vitals": "^4.0.0") if you want INP/attribution support, or leave as-is and create a tracking issue; modify the "web-vitals" value and run yarn/npm install to regenerate lockfile and ensure tests/build pass.libs/tokens/package.json (1)
38-38: 💤 Low value
@uniswap/token-listspinned to an outdated betaThe
latestdist-tag on npm is now1.0.0-beta.35, and the package has always shipped only pre-release versions (starting from1.0.0-beta.0). Pinning to1.0.0-beta.33freezes two patch-level beta updates. Since no stable release exists for this package, this is perfectly acceptable, but consider bumping to1.0.0-beta.35to pick up any fixes landed since beta.33.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/tokens/package.json` at line 38, Update the pinned dependency "@uniswap/token-lists" from "1.0.0-beta.33" to "1.0.0-beta.35" in package.json so the project picks up the latest beta fixes; locate the dependency entry for "@uniswap/token-lists" and change its version string accordingly, then run your package manager install (npm/yarn/pnpm) and lockfile update to ensure the new version is recorded.libs/common-utils/package.json (1)
40-40: ⚡ Quick win
jotai@2.2.0here diverges significantly from2.16.2used everywhere else in the monorepo.
libs/core,apps/cowswap-frontend,apps/explorer, andlibs/walletall declarejotai@2.16.2, whilelibs/common-utilsstays at2.2.0. Pinning to2.2.0(removing the caret) makes this divergence permanent. In a pnpm monorepo these cannot be deduped — both versions end up in the bundle — and any jotai atoms created incommon-utilsand read in a package running2.16.2may fail silently due to incompatible internal store structures.Consider aligning this to
2.16.2to prevent bundle duplication and potential atom incompatibility.🔧 Align jotai version
- "jotai": "2.2.0", + "jotai": "2.16.2",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/common-utils/package.json` at line 40, The dependency entry "jotai": "2.2.0" in libs/common-utils/package.json diverges from the rest of the monorepo and will produce multiple bundled versions; update that dependency to match the repo standard (e.g. "jotai": "^2.16.2") so it can be deduped, then run the workspace package manager (pnpm install) to refresh the lockfile; locate the "jotai" entry in libs/common-utils/package.json to make this change.package.json (1)
88-117: ⚡ Quick winRemove
onlyBuiltDependencies— it duplicatesallowBuildsand contains 6 copies of"nx"
allowBuildsreplacesonlyBuiltDependenciesandignoredBuiltDependencies(both deprecated), providing a single source of truth. Having both settings simultaneously is redundant and theonlyBuiltDependenciesarray is also clearly a copy-paste artifact —"nx"appears six times (lines 111–116).Drop
onlyBuiltDependenciesentirely;allowBuilds: { "nx": true }already covers the intent.🔧 Proposed fix
"pnpm": { "strictDepBuilds": true, "allowBuilds": { "nx": true }, "overrides": { ... }, - "onlyBuiltDependencies": [ - "nx", - "nx", - "nx", - "nx", - "nx", - "nx" - ] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 88 - 117, Remove the redundant onlyBuiltDependencies setting from package.json: delete the entire onlyBuiltDependencies array (which currently contains repeated "nx" entries) because allowBuilds already expresses the intended configuration (allowBuilds: { "nx": true }); ensure no other references to onlyBuiltDependencies remain in the file so configuration relies solely on allowBuilds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/snackbars/package.json`:
- Line 29: Update the `@react-spring/web` dependency to ^10.0.3 in
libs/snackbars/package.json (and apps/cowswap-frontend/package.json) and then
migrate code that uses react-spring APIs: replace any use of SpringContext with
SpringContextProvider, update imports/usages of animated, useSpring,
useSpringValue, and useTransition in components like SnackbarPopup and Modal
according to the v10 migration guide, and run thorough animation tests to
address React 19-related bugs reported in issues `#2376` and `#2404`; fix any API
signature/prop changes exposed by the upgrade.
---
Nitpick comments:
In `@libs/analytics/package.json`:
- Line 35: Package.json currently pins the dependency "web-vitals": "2.1.4";
update the dependency entry in libs/analytics package.json to a v4.x release
(e.g., "web-vitals": "^4.0.0") if you want INP/attribution support, or leave
as-is and create a tracking issue; modify the "web-vitals" value and run
yarn/npm install to regenerate lockfile and ensure tests/build pass.
In `@libs/common-utils/package.json`:
- Line 40: The dependency entry "jotai": "2.2.0" in
libs/common-utils/package.json diverges from the rest of the monorepo and will
produce multiple bundled versions; update that dependency to match the repo
standard (e.g. "jotai": "^2.16.2") so it can be deduped, then run the workspace
package manager (pnpm install) to refresh the lockfile; locate the "jotai" entry
in libs/common-utils/package.json to make this change.
In `@libs/tokens/package.json`:
- Line 38: Update the pinned dependency "@uniswap/token-lists" from
"1.0.0-beta.33" to "1.0.0-beta.35" in package.json so the project picks up the
latest beta fixes; locate the dependency entry for "@uniswap/token-lists" and
change its version string accordingly, then run your package manager install
(npm/yarn/pnpm) and lockfile update to ensure the new version is recorded.
In `@package.json`:
- Around line 88-117: Remove the redundant onlyBuiltDependencies setting from
package.json: delete the entire onlyBuiltDependencies array (which currently
contains repeated "nx" entries) because allowBuilds already expresses the
intended configuration (allowBuilds: { "nx": true }); ensure no other references
to onlyBuiltDependencies remain in the file so configuration relies solely on
allowBuilds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 10d2d2bd-3995-4e79-a6d5-a2eab092c546
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
apps/cow-fi/package.jsonapps/cowswap-frontend-e2e/package.jsonapps/cowswap-frontend/package.jsonapps/explorer/package.jsonapps/sdk-tools/package.jsonapps/widget-configurator/package.jsonlibs/analytics/package.jsonlibs/assets/package.jsonlibs/balances-and-allowances/package.jsonlibs/common-const/package.jsonlibs/common-hooks/package.jsonlibs/common-utils/package.jsonlibs/core/package.jsonlibs/currency/package.jsonlibs/ens/package.jsonlibs/iframe-transport/package.jsonlibs/multicall/package.jsonlibs/permit-utils/package.jsonlibs/snackbars/package.jsonlibs/tokens/package.jsonlibs/types/package.jsonlibs/ui-utils/package.jsonlibs/ui/package.jsonlibs/wallet-provider/package.jsonlibs/wallet/package.jsonpackage.jsonpnpm-workspace.yaml
💤 Files with no reviewable changes (1)
- pnpm-workspace.yaml
| "@cowprotocol/common-hooks": "workspace:*", | ||
| "@cowprotocol/ui": "workspace:*", | ||
| "@react-spring/web": "^9.6.1", | ||
| "@react-spring/web": "9.7.3", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@react-spring/web v10 breaking changes from v9 migration guide
💡 Result:
The main breaking change for @react-spring/web v10 from v9 is the SpringContext API update for React 19 support. Use SpringContextProvider instead of SpringContext. Before (v9): import { SpringContext } from 'react-spring' <SpringContext ...> ... After (v10): import { SpringContextProvider } from 'react-spring' <SpringContextProvider ...> ... v10 adds React 19 peer dependency support (^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0). Internal changes in useSprings for React 19 compatibility may affect some animations (e.g., scale or imperative API issues reported in bugs #2376, #2404), potentially requiring code adjustments like API usage changes. Event callbacks (onStart, onChange, etc.) unified to (animationResult, controller, item), with item only for useTransition. No comprehensive migration guide found beyond official docs' breaking changes section. Test animations thoroughly, especially with React 19.
Citations:
- 1: https://www.react-spring.dev/docs
- 2: feat!: react 19 support pmndrs/react-spring#2368
- 3: https://github.com/pmndrs/react-spring/releases/tag/v10.0.0
- 4: [bug]: animation broken on v10 pmndrs/react-spring#2376
- 5: [bug]: Version 10.0.1 broke useSpring animation pmndrs/react-spring#2404
- 6: fix(deps): update dependency @react-spring/web to v10 pmndrs/react-spring#2391
🏁 Script executed:
# Find all files that import from `@react-spring`
rg "from\s+['\"]@react-spring" --type ts --type tsx --type js --type jsx -A 2Repository: cowprotocol/cowswap
Length of output: 91
🏁 Script executed:
# Check libs/snackbars/package.json for full context
cat -n libs/snackbars/package.json | grep -A 5 -B 5 "@react-spring"Repository: cowprotocol/cowswap
Length of output: 427
🏁 Script executed:
# Search for SpringContext usage specifically
rg "SpringContext" --type ts --type tsx --type js --type jsx -A 2Repository: cowprotocol/cowswap
Length of output: 91
🏁 Script executed:
# Find package.json files to understand scope
fd package.json | head -20Repository: cowprotocol/cowswap
Length of output: 658
🏁 Script executed:
# Search for `@react-spring` imports across all files
rg "@react-spring" --include="*.ts" --include="*.js" -A 2Repository: cowprotocol/cowswap
Length of output: 505
🏁 Script executed:
# Search for SpringContext
rg "SpringContext" --include="*.ts" --include="*.js" -A 2Repository: cowprotocol/cowswap
Length of output: 505
🏁 Script executed:
# Find source files in libs/snackbars
find libs/snackbars/src -type f 2>/dev/null | head -20Repository: cowprotocol/cowswap
Length of output: 372
🏁 Script executed:
# Check what's imported from react-spring in the snackbars lib
ls -la libs/snackbars/Repository: cowprotocol/cowswap
Length of output: 878
🏁 Script executed:
# Search for react-spring in libs/snackbars source
rg "react-spring" libs/snackbars/src/Repository: cowprotocol/cowswap
Length of output: 167
🏁 Script executed:
# Check the main container component
cat -n libs/snackbars/src/containers/SnackbarsWidget/index.tsxRepository: cowprotocol/cowswap
Length of output: 4975
🏁 Script executed:
# Check if react-spring is used anywhere in the codebase
rg "react-spring" --max-count 20Repository: cowprotocol/cowswap
Length of output: 1632
@react-spring/web 9.x officially does not support React 19 — upgrade to v10 requires testing for breaking changes
@react-spring/web@9.7.3 declares peer dependency react@"^16.8.0 || ^17.0.0 || ^18.0.0" — React 19 is explicitly outside this range. The react-spring project released v10.0.0+ to address this, with the latest version being 10.0.3.
However, upgrading to v10 introduces breaking changes that affect the codebase:
- API change: SpringContext → SpringContextProvider
- Affected code: This library imports
animatedanduseSpring(SnackbarPopup), and the Modal component importsuseSpringValueanduseTransition. These core APIs may be impacted by React 19 compatibility changes. - Known issues: react-spring has reported animation bugs with React 19 (
#2376,#2404) that may require code adjustments.
Consider upgrading to @react-spring/web@^10.0.3, but plan for thorough testing of animations and review breaking changes in the react-spring migration guide. The upgrade also applies to apps/cowswap-frontend/package.json, which has the same dependency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/snackbars/package.json` at line 29, Update the `@react-spring/web`
dependency to ^10.0.3 in libs/snackbars/package.json (and
apps/cowswap-frontend/package.json) and then migrate code that uses react-spring
APIs: replace any use of SpringContext with SpringContextProvider, update
imports/usages of animated, useSpring, useSpringValue, and useTransition in
components like SnackbarPopup and Modal according to the v10 migration guide,
and run thorough animation tests to address React 19-related bugs reported in
issues `#2376` and `#2404`; fix any API signature/prop changes exposed by the
upgrade.
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
|
Some builds are using PNPM v9, so those are failing due to the engine mismatch. We either upgrade to v10 or change the allowed version to 9 or 9-10. I suggest the former. |
limitofzero
left a comment
There was a problem hiding this comment.
build is failed as serve(locally) too
| "@cowprotocol/analytics": "workspace:*", | ||
| "@cowprotocol/assets": "workspace:*", | ||
| "@cowprotocol/balances-and-allowances": "workspace:*", | ||
| "@cowprotocol/cms": "https://registry.npmjs.org/@cowprotocol/cms/-/cms-0.11.0.tgz", |
There was a problem hiding this comment.
This one I might have to revert.
|
We should also add the following In order to do that, it needs to be removed from And then also add Slack discussion: https://nomevlabs.slack.com/archives/C0361CDG8GP/p1777911281769109 |
| }, | ||
| "dependencies": { | ||
| "@cowprotocol/cow-sdk": "9.0.2", | ||
| "@cowprotocol/cms": "https://registry.npmjs.org/@cowprotocol/cms/-/cms-0.11.0.tgz", |
|
@alfetopito It looks like the builds that are failing do so because Vercel auto-resolves PNPM to v9 instead of v10. In the logs they say we can select 10 using Corepack, but that's what the updates in the However, it looks like it needs to be enabled with an ENV variable: https://vercel.com/docs/builds/configure-a-build#corepack Could you add that pls? |
@limitofzero What did you try exacly? |
|
kernelwhisperer
left a comment
There was a problem hiding this comment.
We're missing .npmrc and its list of minimumReleaseAgeExclude packages
Summary
To Test
Probably nothing. If the builds are passiing, this should not change anything functionality-wise.
Summary by CodeRabbit