-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
chore: adapter client improvements #3484
base: main
Are you sure you want to change the base?
Changes from 20 commits
cf1e0b1
995ee1e
90aa105
0a6286c
1c748ee
5b941a9
59c1480
9f00cd5
8779eee
2f89a13
17ece78
6c18680
a7901d4
cfed5ba
b709fb0
38f5768
3493bc4
4fe8143
c108ef3
0a8c9b0
e9524be
3152e62
5dd7107
2068b3c
dd62ee4
7cfba60
847cf4f
c7dda55
86ae60e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,11 +16,15 @@ import { | |
waitForTransactionReceipt, | ||
getAccount, | ||
watchPendingTransactions, | ||
http | ||
http, | ||
watchConnectors, | ||
mock | ||
} from '@wagmi/core' | ||
import * as wagmiCore from '@wagmi/core' | ||
import { mainnet } from '@wagmi/core/chains' | ||
import { CaipNetworksUtil } from '@reown/appkit-utils' | ||
import type UniversalProvider from '@walletconnect/universal-provider' | ||
import { mockAppKit } from './mocks/AppKit' | ||
|
||
vi.mock('@wagmi/core', async () => { | ||
const actual = await vi.importActual('@wagmi/core') | ||
|
@@ -97,6 +101,38 @@ describe('WagmiAdapter', () => { | |
expect(adapter.namespace).toBe('eip155') | ||
}) | ||
|
||
it('should set wagmi connectors', () => { | ||
const addWagmiConnector = vi.spyOn(adapter, 'addWagmiConnector' as any) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a nit and maybe hard thing, but I think we should not test private methods like that, we should test private methods through the public methods that call them, thats how it happens in the practice. https://softwareengineering.stackexchange.com/a/100966 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, maybe we should just initialize an appkit instance and see if it's calling this internally as it should There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea, yeah i'll do that. |
||
|
||
vi.spyOn(wagmiCore, 'watchConnectors').mockResolvedValue(vi.fn) | ||
;(adapter as any).syncConnectors({}, mockAppKit) | ||
|
||
expect(addWagmiConnector).toHaveBeenCalledWith( | ||
{ | ||
id: 'test-connector' | ||
}, | ||
{} | ||
) | ||
}) | ||
|
||
it('should listen for wagmi connector changes', () => { | ||
const onConnectors = vi.fn() | ||
|
||
vi.spyOn(wagmiCore, 'watchConnectors').mockImplementation(onConnectors) | ||
;(adapter as any).syncConnectors({}, mockAppKit) | ||
|
||
adapter.wagmiConfig._internal.connectors.setState(() => [ | ||
...adapter.wagmiConfig.connectors, | ||
adapter.wagmiConfig._internal.connectors.setup(mock({ accounts: ['0x123'] })) | ||
]) | ||
|
||
watchConnectors(adapter.wagmiConfig, { | ||
onChange: onConnectors | ||
}) | ||
|
||
expect(onConnectors).toHaveBeenCalled() | ||
}) | ||
|
||
it('should not set info property for injected connector', () => { | ||
const mockConnectors = [ | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,11 +24,17 @@ import type { AppKitOptions } from '../utils/index.js' | |
import type { AppKit } from '../client.js' | ||
import { snapshot } from 'valtio/vanilla' | ||
|
||
type EventName = 'disconnect' | 'accountChanged' | 'switchNetwork' | 'pendingTransactions' | ||
type EventName = | ||
| 'disconnect' | ||
| 'accountChanged' | ||
| 'switchNetwork' | ||
| 'connectors' | ||
| 'pendingTransactions' | ||
type EventData = { | ||
disconnect: () => void | ||
accountChanged: { address: string; chainId?: number | string } | ||
switchNetwork: { address?: string; chainId: number | string } | ||
connectors: ChainAdapterConnector[] | ||
pendingTransactions: () => void | ||
} | ||
type EventCallback<T extends EventName> = (data: EventData[T]) => void | ||
|
@@ -155,6 +161,8 @@ export abstract class AdapterBlueprint< | |
|
||
return true | ||
}) | ||
|
||
this.emit('connectors', this.availableConnectors) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what happens if multiple adapters emit this at the same time? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's fine since we already filter them here and it ConnectorController as well. |
||
} | ||
|
||
protected setStatus(status: AccountControllerState['status'], chainNamespace?: ChainNamespace) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
export { AdapterBlueprint } from './ChainAdapterBlueprint.js' | ||
export type { ChainAdapterConnector } from './ChainAdapterConnector.js' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happens if provider is undefined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is how we do it atm, we don't add
provider
in wagmi adapter here. This is because we're getting the connector id and find the connector in wagmi config instead, you can check it here