Skip to content
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

feat: upgrade viem to support heterogeneous calls #1527

Merged
merged 21 commits into from
Nov 20, 2024

Conversation

abcrane123
Copy link
Contributor

What changed? Why?

Notes to reviewers

How has it been tested?

Copy link

vercel bot commented Oct 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
onchainkit-coverage ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 20, 2024 4:35pm
onchainkit-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 20, 2024 4:35pm
onchainkit-routes ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 20, 2024 4:35pm

@abcrane123 abcrane123 changed the title chore: upgrade chore: upgrade viem to support heterogeneous calls Oct 28, 2024
@abcrane123 abcrane123 changed the title chore: upgrade viem to support heterogeneous calls feat: upgrade viem to support heterogeneous calls Oct 28, 2024
@abcrane123 abcrane123 marked this pull request as ready for review October 28, 2024 03:17
package.json Outdated
@@ -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",
Copy link
Contributor

@alessey alessey Oct 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we still using this dependency after these changes (and under dependencies)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

@@ -93,75 +84,45 @@ export function TransactionProvider({

const { switchChainAsync } = useSwitchChain();

// Validate `calls` and `contracts` props
if (!contracts && !calls) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this is the same conditional

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reverted

Comment on lines -139 to -160
// For batched, use statusSendCalls or statusWriteContracts
// For single, use statusSendCall or statusWriteContract
// For batched, use statusSendCalls
// For single, use statusSendCall
const transactionStatus = useMemo(() => {
const transactionStatuses = walletCapabilities[Capabilities.AtomicBatch]
?.supported
? {
[TRANSACTION_TYPE_CALLS]: statusSendCalls,
[TRANSACTION_TYPE_CONTRACTS]: statusWriteContracts,
}
: {
[TRANSACTION_TYPE_CALLS]: statusSendCall,
[TRANSACTION_TYPE_CONTRACTS]: statusWriteContract,
};
return transactionStatuses[transactionType];
}, [
statusSendCalls,
statusWriteContracts,
statusSendCall,
statusWriteContract,
transactionType,
walletCapabilities[Capabilities.AtomicBatch],
]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

god bless

Comment on lines 115 to 118
transactions:
| Call[]
| ContractFunctionParameters[]
| (Call | ContractFunctionParameters)[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the same type?

Copy link
Contributor

@0xAlec 0xAlec Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
transactions:
| Call[]
| ContractFunctionParameters[]
| (Call | ContractFunctionParameters)[];
transactions: (Call | ContractFunctionParameters)[];

@abcrane123 abcrane123 merged commit 3e7c9ad into main Nov 20, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

6 participants