-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reducing dependency bloat #3411
Conversation
WalkthroughThe pull request includes modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
packages/synapse-constants/package.json (1)
37-38
: Standardize version constraintsThe dependencies use inconsistent version constraints:
ethers
uses fixed version5.7.2
viem
uses caret range^2.21.6
Consider adopting a consistent versioning strategy across dependencies to make updates more predictable.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
packages/sdk-router/package.json
(1 hunks)packages/synapse-constants/package.json
(1 hunks)
🔇 Additional comments (3)
packages/synapse-constants/package.json (1)
41-41
: LGTM: Correctly moved build tool to devDependencies
Moving @codecov/rollup-plugin
to devDependencies is correct as it's only needed during development/testing, not in production. This change aligns with the PR objective of reducing dependency bloat.
packages/sdk-router/package.json (2)
48-49
: LGTM! Correct categorization of build and test dependencies.
The movement of @babel/core
, @babel/plugin-transform-modules-commonjs
, babel-jest
, and jest
to devDependencies
is appropriate as these are build-time and test-time dependencies not required at runtime.
Also applies to: 54-54, 58-58
48-49
: Verify build and test processes still work as expected.
Since build-related dependencies have been moved, let's verify that the build and test processes continue to function correctly.
Also applies to: 54-54, 58-58
✅ Verification successful
Build dependencies are properly configured and should work as expected
The package uses tsdx
as its build tool, which internally manages Babel configuration. The necessary Babel dependencies are correctly defined:
@babel/core
and@babel/plugin-transform-modules-commonjs
for transpilationbabel-jest
for test file processing
The build and test infrastructure is well-established with:
- Comprehensive test suite (25 test files)
- Proper TypeScript configuration
- Jest configuration with necessary module resolution
- Build scripts using
tsdx
for both development and production
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the package defines all necessary build and test dependencies
# by checking for any missing peer dependencies or version conflicts.
# Check if all babel dependencies are properly defined
rg -A 1 "babel" package.json
# Check if there are any peer dependency warnings in package-lock.json
if [ -f package-lock.json ]; then
rg "requires a peer" package-lock.json
fi
# Check if jest config exists and references babel
if [ -f jest.config.js ]; then
rg "babel" jest.config.js
fi
Length of output: 204
Script:
#!/bin/bash
# Let's check for babel configuration and usage in the codebase
# Check for babel config files
fd "babel.config.js|.babelrc"
# Check for babel usage in package scripts
rg "babel" packages/sdk-router/package.json
# Check for any test files that might be using babel
fd ".test.js|.spec.js" packages/sdk-router/
# Check for build scripts that might use babel
rg "build.*script" packages/sdk-router/package.json
Length of output: 367
Script:
#!/bin/bash
# Let's check the package.json scripts section and build configuration
cat packages/sdk-router/package.json
# Check if there are any test files in the src directory
fd ".js" packages/sdk-router/src/
# Look for build or test related configuration files
fd "jest.config|tsconfig|rollup.config" packages/sdk-router/
Length of output: 3087
Script:
#!/bin/bash
# Let's check the build tool (tsdx) and test configurations
cat packages/sdk-router/jest.config.js
# Check if there are any TypeScript test files
fd ".test.ts|.spec.ts" packages/sdk-router/src/
# Check the content of tsconfig.json for babel configuration
cat packages/sdk-router/tsconfig.json
Length of output: 3364
"ethers": "5.7.2", | ||
"viem": "^2.21.6" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider removing redundant ethers sub-packages
The package includes both the full ethers
package and individual @ethersproject/*
packages. This could lead to duplicate installations and increased bundle size.
Consider removing the individual packages if they're not specifically needed:
"dependencies": {
- "@ethersproject/address": "5.7.0",
- "@ethersproject/bignumber": "5.7.0",
"ethers": "5.7.2",
"viem": "^2.21.6"
}
Committable suggestion skipped: line range outside the PR's diff.
Deploying sanguine-fe with Cloudflare Pages
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3411 +/- ##
===================================================
+ Coverage 3.10368% 34.90814% +31.80446%
===================================================
Files 44 78 +34
Lines 2932 2667 -265
Branches 0 82 +82
===================================================
+ Hits 91 931 +840
+ Misses 2838 1733 -1105
Partials 3 3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Bundle ReportChanges will increase total bundle size by 358.3kB (1.12%) ⬆️. This is within the configured threshold ✅ Detailed changes
|
Moves certain dependencies to devDependencies so that the packages weigh less.
Summary by CodeRabbit
New Features
ethers
andviem
dependencies to enhance functionality in the@synapsecns/synapse-constants
package.Chores
@synapsecns/sdk-router
and@synapsecns/synapse-constants
packages for better clarity and maintenance.