feat: upgrade viem to support heterogeneous calls #1527
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
abcrane123
changed the title
abcrane123
changed the title
| @@ -70,7 +70,7 @@ | |||
| "graphql-request": "^6.1.0", | |||
| "jsdom": "^24.1.0", | |||
| "packemon": "3.3.1", | |||
| "permissionless": "^0.1.29", | |||
| "permissionless": "^0.2.10", | |||
There was a problem hiding this comment.
are we still using this dependency after these changes (and under dependencies)?
abcrane123
force-pushed
the
alissa.crane/viem-upgrade
branch
from
f296883
to
38c7640
Compare
| @@ -35,7 +34,6 @@ export function Transaction({ | |||
| calls={calls} | |||
| capabilities={capabilities} | |||
| chainId={accountChainId} | |||
| contracts={contracts} | |||
There was a problem hiding this comment.
could we still support this param and/or deprecate over time by just having anything passed in here be treated the same as the calls param?
There was a problem hiding this comment.
i'm leaning towards just removing it to avoid confusion. i.e devs might think they need to pass calls to calls and contracts to contracts which would cause the order of execution to be unreliable. plus it is a super small change on the developer side (just changing prop name to calls).
There was a problem hiding this comment.
If you left the calls and contracts error in place we could mark contracts as deprecated for a few releases before removing. it would at least give the devs a heads up over the next few releases before the forcing them to update due to a breaking change.
There was a problem hiding this comment.
curious what the rest of the team thinks but personally still leaning towards just removing it since it is such a small change on the dev side.
There was a problem hiding this comment.
+1 on marking as deprecated for a few releases, then removing. It should be a simple change for the dev but nice to give them a bit of time to switch over
There was a problem hiding this comment.
Actually, reconsidered this. Given it's a simple prop name change, I'd probably lean towards just making the breaking change now
|
|
||
| // Transaction hash for single transaction (non-batched) | ||
| const singleTransactionHash = | ||
| writeContractTransactionHash || sendCallTransactionHash; | ||
| const singleTransactionHash = sendCallTransactionHash; |
There was a problem hiding this comment.
Do we still need this assignment?
Can we remove this and update useSendCall data return value to be named singleTransactionHash?
…issa.crane/viem-upgrade
What changed? Why?
Notes to reviewers
How has it been tested?