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.

Sign up for GitHub

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

Jump to bottom

feat: update metamask sdk and implement connectSign #4323

Closed
wants to merge 8 commits into from
Closed

feat: update metamask sdk and implement connectSign #4323

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
226aa91
feat: update metamask sdk and implement connectSign
abretonc7s Oct 8, 2024
b88db8e
feat: cleanup
abretonc7s Oct 8, 2024
940bdf4
feat: cleanup
abretonc7s Oct 8, 2024
4a74e2d
sdk 0.29.1
abretonc7s Oct 8, 2024
338e59d
feat: cleanup
abretonc7s Oct 8, 2024
1b43aaa
feat: url default
abretonc7s Oct 8, 2024
1bb833d
feat: prevent breaking changes
abretonc7s Oct 11, 2024
51f778a
feat: cleanup
abretonc7s Oct 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/connectors/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
},
"dependencies": {
"@coinbase/wallet-sdk": "4.0.4",
"@metamask/sdk": "0.28.4",
"@metamask/sdk": "0.29.0",
"@safe-global/safe-apps-provider": "0.18.3",
"@safe-global/safe-apps-sdk": "9.1.0",
"@walletconnect/ethereum-provider": "2.17.0",
Expand Down
70 changes: 63 additions & 7 deletions packages/connectors/src/metaMask.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,27 @@ import {
withTimeout,
} from 'viem'

export type MetaMaskParameters = Compute<
ExactPartial<Omit<MetaMaskSDKOptions, '_source' | 'readonlyRPCMap'>>
type WagmiMetaMaskSDKOptions = Compute<
ExactPartial<
Omit<
MetaMaskSDKOptions,
| '_source'
| 'readonlyRPCMap'
| 'useDeeplink'
| 'injectProvider'
| 'forceInjectProvider'
| 'forceDeleteProvider'
Copy link
Member

Choose a reason for hiding this comment

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

Let's keep these around since otherwise it's a breaking change.

>
>
>

export type MetaMaskParameters = WagmiMetaMaskSDKOptions & {
// Shortcut to connect and sign a message
connectAndSign?: string
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
connectAndSign?: string
connectAndSign?: string | undefined

// Allow connectWith any rpc method
connectWith?: { method: string; params: unknown[] }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
connectWith?: { method: string; params: unknown[] }
connectWith?: { method: string; params: unknown[] } | undefined

What methods are supported? We should be able to strongly type this with the utility types in Viem (e.g. EIP1474Methods) if we want.

}

metaMask.type = 'metaMask' as const
export function metaMask(parameters: MetaMaskParameters = {}) {
type Provider = SDKProvider
Expand Down Expand Up @@ -76,11 +93,26 @@ export function metaMask(parameters: MetaMaskParameters = {}) {
if (isReconnecting) accounts = await this.getAccounts().catch(() => [])

try {
let signResponse: string | undefined
let connectWithResponse: unknown | undefined
if (!accounts?.length) {
const requestedAccounts = (await sdk.connect()) as string[]
accounts = requestedAccounts.map((x) => getAddress(x))
let requestedAccounts: `0x${string}`[]
abretonc7s marked this conversation as resolved.
Show resolved Hide resolved
if (parameters.connectAndSign) {
signResponse = await sdk.connectAndSign({
msg: parameters.connectAndSign,
})
accounts = await this.getAccounts()
} else if (parameters.connectWith) {
connectWithResponse = await sdk.connectWith({
method: parameters.connectWith.method,
params: parameters.connectWith.params,
})
accounts = await this.getAccounts()
} else {
requestedAccounts = await sdk.connect()
accounts = requestedAccounts.map((x) => getAddress(x))
}
}

// Switch to chain if provided
let currentChainId = (await this.getChainId()) as number
if (chainId && currentChainId !== chainId) {
Expand All @@ -96,6 +128,20 @@ export function metaMask(parameters: MetaMaskParameters = {}) {
displayUri = undefined
}

if (signResponse) {
provider.emit('connectAndSign', {
accounts,
signResponse,
chainId: currentChainId,
})
} else if (connectWithResponse) {
provider.emit('connectWith', {
accounts,
connectWithResponse,
chainId: currentChainId,
})
}

// Manage EIP-1193 event listeners
// https://eips.ethereum.org/EIPS/eip-1193#events
if (connect) {
Expand Down Expand Up @@ -168,11 +214,18 @@ export function metaMask(parameters: MetaMaskParameters = {}) {
// See: https://github.com/vitejs/vite/issues/9703
const MetaMaskSDK = await (async () => {
const { default: SDK } = await import('@metamask/sdk')
// @ts-ignore
if (typeof SDK !== 'function' && typeof SDK.default === 'function')
// @ts-ignore
return SDK.default
// @ts-ignore
Comment on lines +207 to +211
Copy link
Member

Choose a reason for hiding this comment

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

I'll take a crack on getting TS to play nice.

return SDK as unknown as typeof SDK.default
})()

const dappMetadata: MetaMaskSDKOptions['dappMetadata'] = {
url: `${window.location.protocol}//${window.location.host}`,
}

sdk = new MetaMaskSDK({
_source: 'wagmi',
// Workaround cast since MetaMask SDK does not support `'exactOptionalPropertyTypes'`
Expand All @@ -186,8 +239,11 @@ export function metaMask(parameters: MetaMaskParameters = {}) {
return [chain.id, url]
}),
),
dappMetadata: parameters.dappMetadata ?? { name: 'wagmi' },
useDeeplink: parameters.useDeeplink ?? true,
dappMetadata,
abretonc7s marked this conversation as resolved.
Show resolved Hide resolved
useDeeplink: true,
injectProvider: false,
forceInjectProvider: false,
forceDeleteProvider: false,
Comment on lines +237 to +240
Copy link
Member

Choose a reason for hiding this comment

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

What were the defaults internally before? Just want to make sure changes these isn't too disruptive for folks. Probably should allow folks to override these manually so it's not a breaking change and remove that ability in the next major version. (Very unlikely anyone will override manually since people just use defaults, but want to make sure semver is respected.)

Copy link

Choose a reason for hiding this comment

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

We force inject provider but it can conflict with the injected provider by wagmi (see our other PR.)

Copy link
Member

Choose a reason for hiding this comment

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

Is this PR dependent on that one? Haven't had a chance to wrap up our thinking on that quite yet.

})
await sdk.init()
return sdk.getProvider()!
Expand Down
28 changes: 14 additions & 14 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading