feat: re-type walletCapabilities object
#1238
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| AtomicBatch = 'atomicBatch', | ||
| AuxiliaryFunds = 'auxiliaryFunds', | ||
| PaymasterService = 'paymasterService', | ||
| } |
There was a problem hiding this comment.
we'll still need to use this enum for any capabilities we support in OnchainKit but now developers can use non-OCK supported capabilities since it isn't custom typed anymore
const { walletCapabilities } = useOnchainKit()
if (walletCapabilities[someOtherCapability]){
...
}
There was a problem hiding this comment.
For each of them, we should probably add a comment that links on where the EIP is.
Just because I can imagine a convo in 2 months where someone else tries to figure out what those are and they take a few to find the correct docs.
src/OnchainKitConfig.ts
Outdated
| hasAuxiliaryFunds: false, | ||
| hasPaymasterService: false, | ||
| }, | ||
| walletCapabilities: {}, |
There was a problem hiding this comment.
We should not expose this at all here, but instead being part of his own Hook.
Also this change will trigger a braking change, so we need to time this merge correctly.
src/OnchainKitProvider.tsx
Outdated
| hasAuxiliaryFunds: false, | ||
| hasPaymasterService: false, | ||
| }, | ||
| walletCapabilities: walletCapabilities ?? {}, |
There was a problem hiding this comment.
This should not be store here anymore!
But instead our useCapabilities hook should memoize this info.
And long story short we should have a
import { useCapabilities } from `@coinbase/onchainkit`;
and just use that around the library to get the data.
| import { useCapabilities } from 'wagmi/experimental'; | ||
| import type { UseCapabilitiesSafeParams } from '../../types'; | ||
|
|
||
| export function useCapabilitiesSafe({ |
There was a problem hiding this comment.
I still feel like we should rename this useCapabilities, as the fact that we are not exporting this externally adding the word Safe just feels odd.
There was a problem hiding this comment.
I think it will make more sense once we add the functionality to debounce or stop subsequent calls to non-EIP5792 wallets on this hook
There was a problem hiding this comment.
(which isn't supported in the vanilla Wagmi hook - hence unsafe)
There was a problem hiding this comment.
I'd also say the vanilla Wagmi hook is unsafe because it does not return an empty object for non-EIP5792 wallets
| @@ -18,7 +19,7 @@ export const useSendWalletTransactions = ({ | |||
| if (!transactions) { | |||
| return; | |||
| } | |||
| if (walletCapabilities.hasAtomicBatch) { | |||
| if (walletCapabilities[Capabilities.AtomicBatch]?.supported) { | |||
There was a problem hiding this comment.
Yeah I like the enum, is very clean
| @@ -77,7 +78,9 @@ export function TransactionProvider({ | |||
| : TRANSACTION_TYPE_CONTRACTS; | |||
|
|
|||
| // Retrieve wallet capabilities | |||
| const { walletCapabilities } = useOnchainKit(); | |||
| const walletCapabilities = useCapabilitiesSafe({ | |||
| chainId: chainId || 84532, | |||
There was a problem hiding this comment.
I feel like using baseSepolia.id from Viem is a bit more clean
0xAlec
force-pushed
the
eip5792-retype
branch
from
02986c5
to
2fb8811
Compare
| @@ -77,7 +79,9 @@ export function TransactionProvider({ | |||
| : TRANSACTION_TYPE_CONTRACTS; | |||
|
|
|||
| // Retrieve wallet capabilities | |||
| const { walletCapabilities } = useOnchainKit(); | |||
| const walletCapabilities = useCapabilitiesSafe({ | |||
| chainId: chainId || baseSepolia.id, | |||
There was a problem hiding this comment.
We need to make sure we handle this correctly where:
- we prioritize the
chainIdfrom the Transaction component - if that not present we use the
chainIdfromuseOnchainKit() - which if not present at OnchainKit config defaults to BaseSepolia.
We can not merge this PR, if this is not present! Just FYI
cc @0xAlec @alessey @cpcramer @abcrane123
| import type { Config } from 'wagmi'; | ||
| import type { | ||
| SendTransactionMutateAsync, | ||
| WriteContractMutateAsync, | ||
| } from 'wagmi/query'; | ||
| import type { WalletCapabilities as OnchainKitWalletCapabilities } from '../types'; | ||
| // 🌲☀🌲 |
There was a problem hiding this comment.
Not big deal, but this is an easter egg that should alway be as first line only :)
So you probably want to remove it from here
What changed? Why?
viemnative typing forwalletCapabilitiesobjectuseCapabilitiesSafetointernal/hooksand call on a per-component basisuseCapabilitiesSafenow developers can use capabilities that are not explicitly supported by OnchainKit since we are using the native
viemtypingNotes to reviewers
How has it been tested?