Closed mikemaccana closed 8 months ago
Thanks Mike.
The Ghost wallet example isn't a suggestion that wallets should implement window globals. That code demonstrates for wallets that already do this (almost all of them, especially ones that approximate Phantom's API) how to do it somewhat more safely, but primarily it demonstrates how to add support for the Wallet Standard (in a nonbreaking way for applications that may rely on the globals they already have).
To drive adoption for the Wallet Standard, we have to meet wallets where they are and make it easy to adopt. On the Wallet Adapter side, we have already stopped accepting PRs for new adapters that can instead use the Wallet Standard.
Got it. I've made a small PR making this explicit in the docs.
Describe the bug https://github.com/solana-labs/wallet-standard/blob/master/WALLET.md states, after registering the wallet implementation:
We should probably remove this:
window
, not justwindow.document
. As wallets are loaded on all web pages, and the element IDs being globals is expected behaviour (it goes all the way to Internet Explorer) there's a non-zero chance a wallet name will conflict with an element ID on an arbitrary web page.There's a smaller chance a wallet name may conflict with an existing well-known global. Maybe someone is new to web dev and thinks
navigator
is a good name for their new wallet (in this casedefineProperty()
will work, then things will start failing for the rest of the page).Happy to submit a PR just thought a discussion would be preferred in case I'm missing something.