feat: OnchainKitProvider default dependencies
#1589
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
src/createWagmiConfig.ts
Outdated
| coinbaseWallet({ | ||
| appName, | ||
| appLogoUrl, | ||
| preference: 'smartWalletOnly', |
There was a problem hiding this comment.
should we set this to all?
There was a problem hiding this comment.
seems like it'd be a more flexible implementation, unless we just really want to push to smart wallet.
There was a problem hiding this comment.
we need to add a second connector for EOA
coinbaseWallet({ appName: 'OnchainKit', preference: 'eoaOnly', }),
| [base.id]: apiKey | ||
| ? http(`https://api.developer.coinbase.com/rpc/v1/base/${apiKey}`) | ||
| : http(), | ||
| [baseSepolia.id]: apiKey | ||
| ? http( | ||
| `https://api.developer.coinbase.com/rpc/v1/base-sepolia/${apiKey}`, | ||
| ) | ||
| : http(), |
There was a problem hiding this comment.
uses Base networks by default and sets the RPC as the CDP Node
| </OnchainKitContext.Provider> | ||
| </QueryClientProvider> | ||
| </WagmiProvider> | ||
| ); |
There was a problem hiding this comment.
since Wagmi requires QueryClient, the only time 1 dep is missing should be when there is a QueryClient, but no Wagmi? Seems like an edge case, but would be possible to support this usecase, no? or, should we adjust the console.errors at least to only state using default if both don't exist and otherwise prompt the user to fix?
There was a problem hiding this comment.
if 1 dep is missing, we shouldn't modify the default behavior and allow the app to display the typical error message instead
There was a problem hiding this comment.
in my mind, if you provide 1 of the 2 deps then it should default to the same logic/behavior as before this PR which is Unhandled Runtime Error
src/createWagmiConfig.test.ts
Outdated
| expect(coinbaseWallet).toHaveBeenCalledWith({ | ||
| appName: undefined, | ||
| appLogoUrl: undefined, | ||
| preference: 'smartWalletOnly', |
There was a problem hiding this comment.
Is this going to force the components to use smart wallet ? thinking this might be related to the issue i'm having where i'm unable to connect to EOA
This reverts commit 4a51003.
What changed? Why?
WagmiProviderandQueryClientProviderif not provided in the React contextNotes to reviewers
please review the default
WagmiProviderconfiguration settings 🙏How has it been tested?
playground and unit test