fixes #2531 value type on function overload #2561

Yuripetusko · 2024-07-30T03:13:04Z

fixes #2531 - infer correct type of value on function overload, by matching correct function signature from a union type, given the function args

PR-Codex overview

This PR focuses on improving type inference for function values in contracts.

Detailed summary

Added type definitions for GetMutabilityAwareValue and SimulateContractParameters in simulateContract.ts
Updated imports and types in simulateContract.ts and writeContract.ts
Fixed issue Fix: #2531 value types function overload #2560 related to function overloads in simulateContract.test-d.ts

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

changeset-bot · 2024-07-30T03:13:08Z

🦋 Changeset detected

Latest commit: 3001b93

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
viem	Patch

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

vercel · 2024-07-30T03:13:08Z

@Yuripetusko is attempting to deploy a commit to the Wevm Team on Vercel.

A member of the Team first needs to authorize it.

jxom

Looks fine to me. Would be good to get @tmm eyes on it too!

Yuripetusko · 2024-07-30T09:01:44Z

src/types/contract.ts

@@ -303,10 +303,19 @@ export type EventDefinition = `${string}(${string})`

 export type GetValue<
  abi extends Abi | readonly unknown[],
-  functionName extends string,
+  mutability extends AbiStateMutability = AbiStateMutability,


Ideally mutability should be inferred also, but it's hard to do because of the order of generics arguments (args would be needed to infer mutability on overloads, and mutability is needed to infer args). Also it sucks that it's a second argument of this Generic (because subsequent arguments require it), as it makes it hard to skip it if you don't want to pass it. But I couldn't think of a better way right now, or it would require a bigger refactor

Since GetValue is a public API (exported in index.ts), let's pull this logic directly into simulateContract.ts for now.

Yuripetusko · 2024-07-30T09:05:24Z

test/src/abis.ts

@@ -1363,6 +1363,16 @@ export const wagmiContractConfig = {
      stateMutability: 'nonpayable',
      type: 'function',
    },
+    {


I see that this config has a contract address, do I need to add this function to the actual contract? or it's fine to just have this dummy/placeholder abi for a contract that is not actually deployed?

Let's remove this addition to the ABI.

tmm · 2024-07-31T02:30:22Z

Will take a look tomorrow!

Yuripetusko · 2024-08-03T13:57:13Z

Will take a look tomorrow!

bump

…om/Yuripetusko/viem into fix/2532-value-type-overload-types

tmm

Let's revert the change to GetValue and update SimulateContractParameters directly with this logic. Also, let's add the following test. (Haven't found a quick way to get @ts-expect-error to work correctly in this case so might be best to remove that for now. There's a larger type refactor coming to Viem, which will fix this so allowing it to pass for the time being isn't the end of the world.)

test('https://github.com/wevm/viem/issues/2531', async () => {
  const abi = parseAbi([
    'function safeTransferFrom(address, address, uint256)',
    'function safeTransferFrom(address, address, uint256, bytes) payable',
  ])

  const res1 = await simulateContract(client, {
    address: '0x',
    abi,
    functionName: 'safeTransferFrom',
    args: ['0x', '0x', 123n],
    // @ts-expect-error
    value: 123n,
  })
  assertType<void>(res1.result)
  expectTypeOf(res1.request.abi).toEqualTypeOf(
    parseAbi(['function safeTransferFrom(address, address, uint256)']),
  )

  const res2 = await simulateContract(client, {
    address: '0x',
    abi,
    functionName: 'safeTransferFrom',
    args: ['0x', '0x', 123n, '0x'],
    value: 123n,
  })
  assertType<void>(res2.result)
  expectTypeOf(res2.request.abi).toEqualTypeOf(
    parseAbi([
      'function safeTransferFrom(address, address, uint256, bytes) payable',
    ]),
  )
})

In summary:

Remove GetValue related changes
Add logic directly to SimulateContractParameters
Add test to simulateContract.test-d.ts

tmm · 2024-08-06T13:40:45Z

src/types/contract.ts

@@ -303,10 +303,19 @@ export type EventDefinition = `${string}(${string})`

 export type GetValue<
  abi extends Abi | readonly unknown[],
-  functionName extends string,
+  mutability extends AbiStateMutability = AbiStateMutability,


Since GetValue is a public API (exported in index.ts), let's pull this logic directly into simulateContract.ts for now.

tmm · 2024-08-06T13:41:02Z

test/src/abis.ts

@@ -1363,6 +1363,16 @@ export const wagmiContractConfig = {
      stateMutability: 'nonpayable',
      type: 'function',
    },
+    {


Let's remove this addition to the ABI.

tmm · 2024-08-06T13:41:31Z

src/types/contract.test-d.ts

Let's revert this change.

Yuripetusko · 2024-08-06T14:11:36Z

Let's revert the change to GetValue and update SimulateContractParameters directly with this logic. Also, let's add the following test. (Haven't found a quick way to get @ts-expect-error to work correctly in this case so might be best to remove that for now. There's a larger type refactor coming to Viem, which will fix this so allowing it to pass for the time being isn't the end of the world.)

test('https://github.com/wevm/viem/issues/2531', async () => {
  const abi = parseAbi([
    'function safeTransferFrom(address, address, uint256)',
    'function safeTransferFrom(address, address, uint256, bytes) payable',
  ])

  const res1 = await simulateContract(client, {
    address: '0x',
    abi,
    functionName: 'safeTransferFrom',
    args: ['0x', '0x', 123n],
    // @ts-expect-error
    value: 123n,
  })
  assertType<void>(res1.result)
  expectTypeOf(res1.request.abi).toEqualTypeOf(
    parseAbi(['function safeTransferFrom(address, address, uint256)']),
  )

  const res2 = await simulateContract(client, {
    address: '0x',
    abi,
    functionName: 'safeTransferFrom',
    args: ['0x', '0x', 123n, '0x'],
    value: 123n,
  })
  assertType<void>(res2.result)
  expectTypeOf(res2.request.abi).toEqualTypeOf(
    parseAbi([
      'function safeTransferFrom(address, address, uint256, bytes) payable',
    ]),
  )
})

In summary:

Remove GetValue related changes
Add logic directly to SimulateContractParameters
Add test to simulateContract.test-d.ts

Hm the reason ts-expect-error doesn't do anything here is that parseAi only picks one of the mutability options when merging an overload

EDIT: Actually it doesn't merge them even. Doesn't look like it handles overloads, and just returns the first match 🤔 will check a bit more

tmm · 2024-08-06T14:17:22Z

@Yuripetusko nice find. Can drop parseAbi and add the object directly instead.

Yuripetusko · 2024-08-06T14:24:28Z

@Yuripetusko nice find. Can drop parseAbi and add the object directly instead.

Weirdly I can't reproduce this in here. So maybe it's my Jetbrains IDE messing with me again

https://stackblitz.com/edit/viem-getting-started-jixksa?file=package.json,index.ts

Yuripetusko · 2024-08-06T15:19:56Z

@tmm added it to writeContract too, but lmk if I should just add it to simulateContract. Unfortunately I keep getting http errors when running all tests locally. But selected tests seems to be fine. Ignore the previous comments about parseAbi, it was due to something else entirely (missing args on new Generic, which is required to find the correct function in case of overloads)

.changeset/new-hotels-sell.md

fixes wevm#2560 value type on function overload

0e3b207

Yuripetusko changed the title ~~fixes #2560 value type on function overload~~ fixes #2531 value type on function overload Jul 30, 2024

Create new-hotels-sell.md

e628a4b

jxom self-requested a review July 30, 2024 03:44

jxom approved these changes Jul 30, 2024

View reviewed changes

Yuripetusko commented Jul 30, 2024

View reviewed changes

Merge branch 'main' into fix/2532-value-type-overload-types

b405cc9

Yuripetusko and others added 3 commits August 6, 2024 15:18

Merge branch 'main' into fix/2532-value-type-overload-types

1160ad5

chore: merge

367a1ae

Merge branch 'fix/2532-value-type-overload-types' of https://github.c…

ff1ffa5

…om/Yuripetusko/viem into fix/2532-value-type-overload-types

tmm requested changes Aug 6, 2024

View reviewed changes

Yuripetusko added 4 commits August 6, 2024 17:02

revert GetValue changes and create new GetMutabilityAwareValue type

0b2336c

remove un-used test

aa9f198

revert test abi

e53f67c

use mutability aware type util on writeContract too

e82edd7

Yuripetusko marked this pull request as draft August 6, 2024 15:16

fix: wrong import

6217b77

Yuripetusko marked this pull request as ready for review August 6, 2024 15:19

Yuripetusko requested a review from tmm August 6, 2024 15:20

Yuripetusko added 2 commits August 6, 2024 17:20

lint

78e9f02

fix import

edd438a

tmm reviewed Aug 6, 2024

View reviewed changes

.changeset/new-hotels-sell.md Outdated Show resolved Hide resolved

tmm approved these changes Aug 6, 2024

View reviewed changes

Update .changeset/new-hotels-sell.md

3001b93

tmm merged commit 1c5d64a into wevm:main Aug 6, 2024
19 of 32 checks passed

github-actions bot mentioned this pull request Aug 6, 2024

chore: version package #2582

Merged

Yuripetusko deleted the fix/2532-value-type-overload-types branch August 7, 2024 00:09

tmm mentioned this pull request Dec 21, 2024

refactor(siwe): ox #3151

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixes #2531 value type on function overload #2561

fixes #2531 value type on function overload #2561

Yuripetusko commented Jul 30, 2024 •

edited by pr-codex bot

Loading

changeset-bot bot commented Jul 30, 2024 •

edited

Loading

vercel bot commented Jul 30, 2024

jxom left a comment

Yuripetusko Jul 30, 2024 •

edited

Loading

tmm Aug 6, 2024

Yuripetusko Jul 30, 2024

tmm Aug 6, 2024

tmm commented Jul 31, 2024

Yuripetusko commented Aug 3, 2024

tmm left a comment

tmm Aug 6, 2024

tmm Aug 6, 2024

tmm Aug 6, 2024

Yuripetusko commented Aug 6, 2024 •

edited

Loading

tmm commented Aug 6, 2024

Yuripetusko commented Aug 6, 2024

Yuripetusko commented Aug 6, 2024

fixes #2531 value type on function overload #2561

fixes #2531 value type on function overload #2561

Conversation

Yuripetusko commented Jul 30, 2024 • edited by pr-codex bot Loading

PR-Codex overview

Detailed summary

changeset-bot bot commented Jul 30, 2024 • edited Loading

🦋 Changeset detected

vercel bot commented Jul 30, 2024

jxom left a comment

Choose a reason for hiding this comment

Yuripetusko Jul 30, 2024 • edited Loading

Choose a reason for hiding this comment

tmm Aug 6, 2024

Choose a reason for hiding this comment

Yuripetusko Jul 30, 2024

Choose a reason for hiding this comment

tmm Aug 6, 2024

Choose a reason for hiding this comment

tmm commented Jul 31, 2024

Yuripetusko commented Aug 3, 2024

tmm left a comment

Choose a reason for hiding this comment

tmm Aug 6, 2024

Choose a reason for hiding this comment

tmm Aug 6, 2024

Choose a reason for hiding this comment

tmm Aug 6, 2024

Choose a reason for hiding this comment

Yuripetusko commented Aug 6, 2024 • edited Loading

tmm commented Aug 6, 2024

Yuripetusko commented Aug 6, 2024

Yuripetusko commented Aug 6, 2024

Yuripetusko commented Jul 30, 2024 •

edited by pr-codex bot

Loading

changeset-bot bot commented Jul 30, 2024 •

edited

Loading

Yuripetusko Jul 30, 2024 •

edited

Loading

Yuripetusko commented Aug 6, 2024 •

edited

Loading