-
-
Notifications
You must be signed in to change notification settings - Fork 591
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
fix(types): payable value inference ts value #2307
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: db22244 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
size-limit report 📦
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2307 +/- ##
==========================================
- Coverage 96.60% 92.90% -3.71%
==========================================
Files 675 644 -31
Lines 57392 52672 -4720
Branches 2439 1990 -449
==========================================
- Hits 55443 48934 -6509
- Misses 1919 3692 +1773
- Partials 30 46 +16 ☔ View full report in Codecov by Sentry. |
test('args: value', () => { | ||
// payable function | ||
simulateContract(clientWithAccount, { | ||
abi: baycContractConfig.abi, | ||
address: '0x', | ||
functionName: 'mintApe', | ||
args: [69n], | ||
value: 5n, | ||
}) | ||
|
||
// payable function (undefined) | ||
simulateContract(clientWithAccount, { | ||
abi: baycContractConfig.abi, | ||
address: '0x', | ||
functionName: 'mintApe', | ||
args: [69n], | ||
}) | ||
|
||
// nonpayable function | ||
simulateContract(clientWithAccount, { | ||
abi: baycContractConfig.abi, | ||
address: '0x', | ||
functionName: 'approve', | ||
// @ts-expect-error | ||
value: 5n, | ||
}) | ||
}) |
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.
Adding more coverage for GetValue
@@ -309,10 +309,10 @@ export type GetValue< | |||
_Narrowable extends boolean = IsNarrowable<TAbi, Abi>, | |||
> = _Narrowable extends true | |||
? TAbiFunction['stateMutability'] extends 'payable' | |||
? { value?: NoUndefined<TValueType> | undefined } |
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.
NoUndefined
is necessary anymore as the property is marked as optional and is a union with undefined
.
@@ -309,10 +309,10 @@ export type GetValue< | |||
_Narrowable extends boolean = IsNarrowable<TAbi, Abi>, | |||
> = _Narrowable extends true | |||
? TAbiFunction['stateMutability'] extends 'payable' | |||
? { value?: NoUndefined<TValueType> | undefined } | |||
? { value?: NoInfer<TValueType> | undefined } |
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.
We add NoInfer
so that TValueType
does not get inferred based on the property value.
Before
writeContract({
functionName: 'nonpayable',
value: '123',
// ^? value (property): string
})
writeContract({
functionName: 'payable',
value: 123n,
// ^? value (property): bigint
})
After
writeContract({
functionName: 'nonpayable',
value: '123',
// ^? value (property): undefined
})
writeContract({
functionName: 'payable',
value: 123n,
// ^? value (property): bigint | undefined
})
Might not look like a lot, but downstream this causes an issue with type transformations.
: TAbiFunction['payable'] extends true | ||
? { value?: NoUndefined<TValueType> | undefined } | ||
: { value?: never | undefined } |
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.
never | undefined
becomes undefined
so removing it so the compiler can have an extra vacation
Upstream issue in
wagmi
and@wagmi/vue
with payablevalue
property.wevm/wagmi#3984
PR-Codex overview
This PR improves type inference for the
value
property in contracts and updates test cases forsimulateContract
.Detailed summary
value
property in contractsNoInfer
type utilitysimulateContract
withvalue
property