Initialize the MetaMask extension provider internally #834
Conversation
| if ( | ||
| message?.target === 'metamask-inpage' && | ||
| message.data?.name === 'metamask-provider' && | ||
| message.data.data?.id === id |
There was a problem hiding this comment.
We're duck typing, so I don't think we care about the id here. I can't think of why we'd receive a message with a target of metamask-inpage, and a name of metamask-provider, and then the id would matter.
If we receive any message that quacks like Metamask, that means it's Metamask, and we can just proceed with checking for Snap support one time after that, and immediately stop trying if it fails.
| if (!provider) return false; | ||
| async function metamaskRequest(request: Record<string, any>) { | ||
| return new Promise((resolve, reject) => { | ||
| const id = Math.floor(Math.random() * 1000000).toString(); |
There was a problem hiding this comment.
Not sure we need this, or need it to be anything other than a simple counter, see below.
| return true; | ||
| const snaps = await Promise.race([ | ||
| metamaskRequest({ method: 'wallet_getSnaps' }), | ||
| new Promise((resolve, reject) => setTimeout(() => reject('MetaMask provider not found'), 1000)), |
There was a problem hiding this comment.
| new Promise((resolve, reject) => setTimeout(() => reject('MetaMask provider not found'), 1000)), | |
| new Promise((resolve, reject) => setTimeout(reject, 1000)), |
We don't do anything with the rejection cause, so no reason to have it. However, I'm not sure why we need this race at all. We wait for a message that looks like Metamask, we should only call this one time.
| import { registerWallet } from '@wallet-standard/wallet'; | ||
| import { SolflareMetaMaskWallet } from './wallet.js'; | ||
|
|
||
| let stopPolling = false; | ||
| let counter = 10; |
There was a problem hiding this comment.
I'm not sure we need polling, or any counter. That's If we add the message listener once on the window, then send a postMessage once, then we should be waiting for when Metamask is added. And if it's already been added, our message will reach it.
| window.removeEventListener('message', handleMessage); | ||
|
|
||
| if (message?.data.data.error) { | ||
| reject(message.data.data.error.message); | ||
| } else { | ||
| resolve(message.data.data.result); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return await isSnapSupported(provider); | ||
| } catch (error) { | ||
| return false; | ||
| } | ||
| window.addEventListener('message', handleMessage); |
There was a problem hiding this comment.
Every time you call metamaskRequest you'd adding a new event listener. This shouldn't happen. And you're only removing the event listener the one time the message matches. This means you have up to 9 dangling event listeners creating memory leaks.
|
@jordansexton, this commit broke usage of Helium wallet if dApp have Solflare adapter and connecting through Helium Suspect could be, helium is using postMessage as well but they never handled other cases of messages. Not sure if it affect any other as well |
This PR uses an internal postMessage based inpage provider for communicating with the MetaMask extension instead of using the injected
window.ethereuminterface.