fix: update deployContract to use generics for constructor args with arrays#224
fix: update deployContract to use generics for constructor args with arrays#224jose-blockchain wants to merge 3 commits into1inch:masterfrom
Conversation
Fixes type mismatch when passing arrays (e.g., readonly number[]) to deployContract constructor parameters. Implemented Solution B using generics with Abi type parameter to align with ContractFactory.deploy types. The function now accepts arrays and complex types in constructor parameters, resolving the issue where Array<BigNumberish> didn't match ContractFactory.deploy(...args: ContractMethodArgs<A>). Related Issues: 1inch#208
There was a problem hiding this comment.
Pull request overview
This PR updates the deployContract helper in src/utils.ts to address a TypeScript type mismatch when passing array/tuple-like constructor arguments (e.g., readonly [100, 500, 3000, 10000]) to ContractFactory.deploy.
Changes:
- Added
Abi-genericdeployContractsignature intended to better align withethersfactory deployment argument typing. - Relaxed constructor-parameter typing to allow arrays/complex types by accepting a
readonly any[]. - Introduced helper types for deploy parameters/return types.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
removing `ContractFactory` import, unused Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
please review, this is an external collab. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/utils.ts
Outdated
| export async function deployContract<abi extends Abi = Abi>( | ||
| name: string, | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| parameters: DeployContractParameters<abi> = [] as any |
There was a problem hiding this comment.
The default value parameters = [] as any is likely unnecessary because [] is already assignable to the annotated type (readonly any[]). Dropping the cast reduces any usage without changing the function’s flexibility.
| parameters: DeployContractParameters<abi> = [] as any | |
| parameters: DeployContractParameters<abi> = [] |
| export async function deployContract<abi extends Abi = Abi>( | ||
| name: string, |
There was a problem hiding this comment.
The generic type parameter name abi is lowercase, which makes it easy to confuse with a runtime value and is inconsistent with other generics in this file (e.g., T). Consider renaming it to something like TAbi/TContractAbi for clarity.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
Fixes type mismatch error when passing arrays (e.g.,
readonly [100, 500, 3000, 10000]) as constructor parameters todeployContract.The previous type
Array<BigNumberish>was too restrictive and didn't matchContractFactory.deploy(...args: ContractMethodArgs<A>)when arrays were included in constructor parameters.Changes
deployContractfunction to use generics withAbitype parameter (Solution B)Array<BigNumberish>toreadonly any[]to allow arrays and complex typesDeployContractParametersandDeployContractReturnfor better type safetyreadonly [100, 500, 3000, 10000]as constructor parametersTesting
Related Issues
Fixes #208
Potenrial reviewers: @mayar-deeb-xi @k06a @ZumZoom
Type of Change