-
Notifications
You must be signed in to change notification settings - Fork 0
v2 #18
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
v2 #18
Conversation
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.
Pull Request Overview
This PR refactors the codebase to improve API consistency and maintainability by standardizing parameter naming conventions and simplifying function signatures across the SDK.
Key Changes:
- Renamed all function parameter interfaces from
*Paramsto*Argsfor consistency - Changed several functions to accept direct arguments instead of wrapped parameter objects
- Removed unused dependencies (
@uniswap/router-sdk,@uniswap/universal-router-sdk) - Relaxed peer dependency version constraints for better compatibility
Reviewed Changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/preparePermit2Data.ts | Extracted allowance ABI to module level, renamed parameter type, reordered validation logic |
| src/utils/preparePermit2BatchData.ts | Renamed parameter type from Params to Args |
| src/utils/getTokens.ts | Renamed parameter type from Params to Args |
| src/utils/getQuote.ts | Renamed parameter type to SwapExactInSingle, updated type casting and removed unnecessary destructuring |
| src/utils/getPosition.ts | Added getPositionDetails function, refactored getPosition to use it |
| src/utils/getPoolKeyFromPoolId.ts | Changed to accept poolId directly instead of wrapped params object |
| src/utils/getPool.ts | Renamed parameter type, changed from tokens array to individual currencyA/currencyB, removed unused code |
| src/utils/buildSwapCallData.ts | Removed instance dependency, added custom actions support, removed quote calculation |
| src/utils/buildRemoveLiquidityCallData.ts | Renamed parameter type, removed try-catch wrapper |
| src/utils/buildCollectFeesCallData.ts | Renamed parameter type, removed slippageTolerance parameter, updated parameter order |
| src/utils/buildAddLiquidityCallData.ts | Renamed parameter type, removed validation that's now handled by Position methods |
| src/types/utils/*.ts | Renamed all parameter interfaces from *Params to *Args |
| src/test/utils/*.test.ts | Updated tests to match new signatures and improved test coverage |
| src/core/uniDevKitV4.ts | Renamed all method parameters, updated documentation, removed deprecated methods |
| package.json | Removed unused dependencies and relaxed peer dependency constraints |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { calldata, value } = V4PositionManager.collectCallParameters(positionData.position, { | ||
| tokenId, | ||
| recipient, | ||
| slippageTolerance: percentFromBips(0), |
Copilot
AI
Oct 27, 2025
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.
Hardcoded slippage tolerance of 0 may cause transaction failures. Consider using a configurable value or a small default (e.g., DEFAULT_SLIPPAGE_TOLERANCE) to account for price changes between transaction submission and execution.
| "react-dom": "^19.1.1" | ||
| "@tanstack/react-query": "^5", | ||
| "react": "^18 || ^19", | ||
| "react-dom": "^18 || ^19" |
Copilot
AI
Oct 27, 2025
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.
The peer dependency range for @tanstack/react-query is too broad (^5) and may include breaking changes across minor versions. Consider using a more specific range like '^5.0.0' or document the minimum tested version to ensure compatibility.
| "react-dom": "^18 || ^19" | |
| "react-dom": "^18.0.0 || ^19.0.0" |
|
🎉 This PR is included in version 1.0.17 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This PR refactors the codebase to improve API consistency and maintainability by standardizing parameter naming conventions and simplifying function signatures across the SDK.
Key Changes:
Renamed all function parameter interfaces from *Params to *Args for consistency
Changed several functions to accept direct arguments instead of wrapped parameter objects
Removed unused dependencies (@uniswap/router-sdk, @uniswap/universal-router-sdk)
Relaxed peer dependency version constraints for better compatibility