cannon
cannon copied to clipboard
chore: update deps
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
Code Review: PR #1844 - Dependency Updates
Overview
This PR updates project dependencies with a version bump from 2.24.0 to 2.25.0. The changes include major version updates to dev tooling and security-related package overrides.
✅ Positive Aspects
-
Security Improvements: The PR adds extensive security overrides in
pnpm-workspace.yamladdressing multiple CVEs including:elliptic@<6.6.0→>=6.6.1(CVE fixes for elliptic curve cryptography)wsvulnerabilities across multiple version rangesvitesecurity patchesform-data@<2.5.4→>=2.5.4- Several other critical vulnerability patches
-
Code Quality Improvements:
- Type Safety Enhancement (
packages/hardhat-cannon/src/utils/index.ts:18): Added explicit return type togetContract()function:
This improves type safety and makes the API contract clearer.export function getContract(contractName: string): Pick<ContractData, 'address' | 'abi'>
- Type Safety Enhancement (
-
Browser Compatibility Fix (
packages/builder/src/ipfs.ts:78): Better handling of Buffer → Blob conversion:const content = typeof window !== 'undefined' && typeof Blob !== 'undefined' ? new Blob([new Uint8Array(data)]) : data;The addition of
new Uint8Array(data)wrapper ensures proper data conversion in browser environments. -
Modern Tooling: Updates to latest stable versions:
- ESLint v8 → v9
- TypeScript v5.8.3 → v5.9.3
- Prettier v2 → v3
- React v18 → v19
- Lerna v8 → v9
⚠️ Issues & Concerns
Critical: Lint Failures
The CI shows lint job is FAILING. This must be addressed before merge:
- The ESLint major version update (v8 → v9) likely introduced breaking changes
- ESLint v9 has a new flat config system and different plugin API
- Migration may require
.eslintrc.*→eslint.config.jsconversion
Recommendation: Check lint errors and update ESLint configuration for v9 compatibility.
Vercel Deployment Failure
The Vercel build check is showing FAILURE status. This needs investigation:
- Could be related to React 19 upgrade
- May need Next.js version compatibility check
- Possible build configuration adjustments needed
🔍 Potential Breaking Changes
-
ESLint v9 Migration
- Breaking: New flat config system
- Plugin system changes may break existing custom rules
- TypeScript ESLint parser upgraded from v5 → v8 (major changes)
-
Prettier v3
- May have different default formatting rules
- Plugin API changes could affect
prettier-plugin-solidity
-
React v19
- Major version with potential breaking changes
- May affect website and any React-dependent packages
- Should verify all React components still work correctly
-
Babel Plugin Rename
@babel/plugin-proposal-private-property-in-object→@babel/plugin-transform-private-property-in-object- Proposal → Transform indicates the feature is now stable
- Should verify Babel config references the new package name
📋 Security Review
Good: Extensive security overrides added. Key patches include:
- ✅ Elliptic curve crypto vulnerabilities
- ✅ WebSocket DOS vulnerabilities
- ✅ XML parser vulnerabilities
- ✅ Path traversal fixes (tar, tmp)
- ✅ ReDoS vulnerabilities (brace-expansion, path-to-regexp)
- ✅ Cookie security issues
Socket Security Report: Shows neutral/success status - no major security concerns from dependency scanner.
🧪 Testing Considerations
- Unit Tests: Currently running - should verify all pass with updated dependencies
- E2E Tests: Currently running - critical for catching integration issues
- Manual Testing Recommended:
- Test CLI commands with updated dependencies
- Verify hardhat-cannon plugin functionality
- Test website build and runtime
- Verify IPFS operations (changed code in
ipfs.ts)
📝 Recommendations
Before Merge:
- Fix lint failures - Update ESLint config for v9 compatibility
- Fix Vercel deployment - Investigate build failures
- Wait for CI - Ensure all tests pass (unit, e2e, cypress)
- Add changelog entry - Document breaking changes for consumers
- Test locally:
pnpm clean pnpm i pnpm build pnpm test pnpm lint
Post-Merge Considerations:
- Monitor for runtime issues with React 19 in production
- Watch for ESLint v9 edge cases in development
- Document any migration steps needed for contributors
🎯 Code Quality Score
| Aspect | Score | Notes |
|---|---|---|
| Security | ⭐⭐⭐⭐⭐ | Excellent - addresses multiple CVEs |
| Type Safety | ⭐⭐⭐⭐⭐ | Improved with explicit types |
| Browser Compat | ⭐⭐⭐⭐⭐ | Better Buffer handling |
| CI Status | ⭐⭐ | Failing - needs fixes |
| Breaking Changes | ⭐⭐⭐ | Major updates need careful testing |
✏️ Minor Suggestions
- Consider adding a migration guide for the ESLint v9 changes
- Document any new linting rules that developers should be aware of
- Test the form-data update thoroughly for IPFS uploads
- Verify all transitive dependency security overrides are actually being applied
Overall Assessment: This is a valuable security and modernization update, but should not be merged until lint and build failures are resolved. The code changes themselves are good, but the tooling migration needs completion.
The changes align with Cannon's development standards and improve overall code quality once CI issues are addressed.
Pull Request Review: Dependency Updates
Summary
This PR updates numerous dependencies across the monorepo, with 176 files changed (+8,939/-13,984 lines). The changes primarily consist of:
- Major version bumps for build tools (Babel, ESLint, TypeScript)
- Type definition updates throughout the codebase
- Solidity compiler file removal
- Configuration adjustments
Key Changes Reviewed
1. Root Package Dependencies
- Babel ecosystem: Upgraded from v7.16-7.22 to v7.27-7.28
- ESLint: Upgraded from v8.10 to v9.38 (major version bump)
- TypeScript ESLint: Upgraded from v5.13 to v8.46 (major version bump)
- Added: type module to root package.json
- Memory fix: Added NODE_OPTIONS for 8GB heap to lint scripts (good for large monorepos)
2. Code Quality Observations
POSITIVES:
- Systematic update approach across all packages
- Removed deprecated Babel plugin in favor of current one
- Cleaned up unused soljson-latest.js file (3,354 lines removed)
- Removed obsolete .eslintignore file
- Added compiler version to Prettier Solidity config
CONCERNS:
-
ESLint Major Version Jump (v8 to v9)
- This is a major breaking change with new flat config system
- Need to verify ESLint config has been properly migrated
- Recommend testing lint commands extensively
-
TypeScript-ESLint Major Version Jump (v5 to v8)
- Skipped v6 and v7 entirely
- May have rule changes or new strictness
- Could introduce new linting errors in CI
-
Type Import Changes Throughout Codebase
- Many files show changes from direct imports to type imports
- While correct, this is a large-scale refactor that should be tested
-
Test File Changes
- Test assertion changes observed
- Need to verify all tests still pass
3. Security and Performance
GOOD:
- Dependency updates generally improve security
- Memory limit increase helps with large codebases
REVIEW NEEDED:
- With 176 files changed, thorough testing is critical
- Type changes could hide runtime issues if tests do not have full coverage
4. Breaking Change Potential
HIGH RISK AREAS:
- ESLint v9 uses flat config (incompatible with v8)
- type module in package.json changes module resolution
- Major version jumps in core build tools
Recommendations
-
Before Merge:
- Run full test suite: pnpm test
- Run lint checks: pnpm lint
- Test build process: pnpm build
- Verify CLI works
- Test website build: pnpm build:website
-
Documentation:
- Consider adding a migration note in CHANGELOG if ESLint config changed
- Document the type module change if it affects usage
-
CI/CD:
- Ensure CI passes all checks before merge
- Watch for any new lint errors from stricter rules
-
Post-Merge:
- Monitor for any runtime issues related to type changes
- Be ready to fix any issues discovered by users
Test Coverage Verification
Given the scope of changes, please confirm:
- All existing tests pass
- No new type errors introduced
- Build succeeds for all packages
- E2E tests pass (especially for CLI)
Overall Assessment
Scope: Large-scale dependency update Risk Level: Medium-High (due to major version jumps) Code Quality: Changes appear systematic and well-structured Recommendation: APPROVE WITH CONDITIONS - merge after confirming all tests pass
The dependency updates are necessary for security and keeping the project modern. The systematic approach is good, but the major version jumps (especially ESLint v9 and TypeScript-ESLint v8) require careful verification.
Pull Request Review: chore: update deps
Summary
This PR updates project dependencies and applies automated code formatting across 176 files with 8,939 additions and 13,984 deletions. The changes are primarily:
- Dependency updates (major version bumps for tooling)
- Automated code formatting (trailing commas, spacing)
- Version bump from 2.24.0 to 2.25.0
Code Quality & Best Practices
Positive Observations:
- Consistent Formatting: The PR applies consistent code style across the entire codebase with prettier/eslint
- Following Project Standards: Changes align with the project's commitment to code quality mentioned in CLAUDE.md
- Test Updates: 31 test files were updated to maintain consistency
- Memory Management: Added NODE_OPTIONS memory limit to lint scripts to handle large codebases
Configuration Changes:
- Deleted .eslintignore (ESLint 9 uses different ignore system)
- Added "type": "module" to root package.json
- Updated Prettier Solidity compiler version to 0.8.30
- Version bump in lerna.json (2.24.0 to 2.25.0)
Potential Issues & Concerns
Breaking Changes:
-
ESLint 9.x Migration (v8.10.0 to v9.38.0)
- ESLint 9 has breaking changes in configuration format (flat config)
- Need to verify all eslint configs are compatible
- The removal of .eslintignore suggests migration to new ignore system
-
TypeScript-ESLint (v5.x to v8.x)
- Major version jump with breaking changes
- Some rules may behave differently or be deprecated
-
Prettier 3.x (v2.5.1 to v3.6.2)
- Prettier 3 dropped Node 12/14 support (project requires Node 18+, so OK)
- May have subtle formatting changes
-
React 19 (v18.2.0 to v19.2.0)
- Major version upgrade
- While this is a devDep, ensure website/examples are tested
-
Lerna 9 (v8.0.2 to v9.0.0)
- Major version change for monorepo tooling
- Verify publish workflows still function correctly
Missing Items:
- No ESLint Configuration Update: I don't see updates to eslint config files to support ESLint 9's flat config format. This could cause linting to fail.
- No Migration Guide Reference: PR description is empty - should document the breaking changes
- Package Lock Files: Ensure pnpm-lock.yaml is updated (not visible in diff)
Security Concerns
Good:
- All dependency updates follow semantic versioning with ^ ranges
- Using pnpm which has better security than npm
- engineStrict: true enforces Node/pnpm versions
To Verify:
- Run pnpm audit to check for vulnerabilities in updated dependencies
- Verify the large soljson-latest.js file changes (appears to be auto-generated, but worth checking)
Performance Considerations
Positive:
- Added memory limit flag for lint commands handles large monorepo
- Updated to latest tooling versions should have performance improvements
Concerns:
- ESLint 9 and TypeScript-ESLint 8 may have different performance characteristics
- Recommend running benchmarks on lint times
Test Coverage
Good:
- 31 test files updated with formatting changes
- Test logic appears unchanged (only formatting)
- No test files were deleted
Recommendations:
- Run Full Test Suite: Ensure all tests pass with updated dependencies
- E2E Testing: Run end-to-end tests, especially for CLI and website
- Integration Tests: Test the Hardhat plugin with updated dependencies
- Verify Build: Run pnpm build to ensure TypeScript compilation works
Recommendations
Critical (Should Address Before Merge):
- Add ESLint 9 Configuration: Create/update eslint.config.js with flat config format
- Test Build Pipeline: Verify all builds, tests, and linting pass in CI
- Document Breaking Changes: Add migration notes to PR description
Recommended:
- Test Publish Workflow: Verify Lerna 9 publish commands work correctly
- Update CI Configuration: Ensure CI uses compatible Node version (18+)
- Test Website Build: Verify React 19 doesn't break the website
- Add Changeset: Run pnpm changeset to document the dependency updates per project conventions
Nice to Have:
- Split PR: Consider separating dependency updates from code formatting
- Gradual Updates: Stage major version updates separately for easier troubleshooting
- Add Tests: Add a test to verify the formatting rules are maintained
Verdict
Recommendation: REQUEST CHANGES
While the code formatting is excellent and dependency updates are generally good, the missing ESLint 9 configuration and lack of verification testing make this PR risky to merge as-is.
Action Items:
- Add/update ESLint configuration for v9 compatibility
- Verify all CI checks pass
- Run manual testing for critical workflows (build, test, publish)
- Document breaking changes in PR description
- Consider running on a test branch first
Once these items are addressed, this will be a great modernization update for the codebase!