docs: fix wallet discovery using fixed window.midnight.mnLace key - #1080
docs: fix wallet discovery using fixed window.midnight.mnLace key#1080UtkarshArjariya wants to merge 1 commit into
Conversation
The DApp Connector spec installs each wallet's Initial API under a wallet-chosen key on window.midnight (commonly a UUID), not a fixed "mnLace" property, so window.midnight.mnLace is undefined for users running current Lace builds and the Connect Wallet button throws "Cannot read properties of undefined (reading 'connect')". Look up the wallet by its name property instead, matching the discovery pattern already documented in api-reference/dapp-connector and sdks/community/wallets/integration.mdx. Fixes midnightntwrk#1070
|
@UtkarshArjariya is attempting to deploy a commit to the Midnight Foundation Team on Vercel. A member of the Team first needs to authorize it. |
oduameh
left a comment
There was a problem hiding this comment.
Thanks for tracking this down, @UtkarshArjariya. The core fix is correct: the DApp Connector API specification (Initial API, point 2) requires wallets to register under a freshly generated UUIDv4, so window.midnight.mnLace can resolve to undefined with v4-conformant builds. The Object.values(window.midnight ?? {}) discovery pattern aligns with the spec and the API reference.
Three items to address before this can merge:
1. react-wallet-connect.mdx already fixed on main
Since this PR was opened, docs/guides/react-wallet-connect.mdx was reworked on main. It now has a selectWallet() helper with the correct Object.values enumeration, a prose explanation of UUID keys, and a troubleshooting note warning against window.midnight.mnLace. The changes in this PR target code that no longer exists on main and will produce merge conflicts. Rebase onto main and drop the changes to this file.
2. Inline comment verbosity
The three-line code comments reference mnLace as a counter-example. Tutorial readers have never seen that key, so the reference adds confusion rather than clarity. See the inline suggestions for simplified alternatives.
3. Verify the name property value
The code uses .find((w) => w.name === 'Lace'). The exact display name the Lace wallet reports as its name property needs to be confirmed with a real install. The spec also offers rdns as a more stable identifier (point 8: "Wallet should keep stable rdns throughout lifecycle of the product"). Consider matching on rdns instead of name if the Lace rdns value is known.
| // Wallets inject their Initial API into `window.midnight` under a | ||
| // wallet-specific key (not a fixed key like `mnLace`), so look it up | ||
| // by the `name` property instead of assuming the key. |
There was a problem hiding this comment.
Tutorial readers have not seen the old mnLace key, so the counter-example adds confusion. A concise comment about what the code does is clearer.
| // Wallets inject their Initial API into `window.midnight` under a | |
| // wallet-specific key (not a fixed key like `mnLace`), so look it up | |
| // by the `name` property instead of assuming the key. | |
| // Discover the Lace wallet from the wallets injected into window.midnight |
| // Wallets inject their Initial API into `window.midnight` under a | ||
| // wallet-specific key (not a fixed key like `mnLace`), so look it up | ||
| // by the `name` property instead of assuming the key. |
There was a problem hiding this comment.
Same note: drop the mnLace counter-example and describe what the code does.
| // Wallets inject their Initial API into `window.midnight` under a | |
| // wallet-specific key (not a fixed key like `mnLace`), so look it up | |
| // by the `name` property instead of assuming the key. | |
| // Discover the Lace wallet from the wallets injected into window.midnight |
| // Wallets inject their Initial API into `window.midnight` under a | ||
| // wallet-specific key (not a fixed key like `mnLace`), so look it up | ||
| // by the `name` property instead of assuming the key. | ||
| const wallets = Object.values(window.midnight ?? {}); | ||
| const wallet: InitialAPI | undefined = wallets.find((w) => w.name === 'Lace'); | ||
| if (!wallet) { | ||
| throw new Error('Lace wallet not found. Please install the Lace wallet extension.'); | ||
| } |
There was a problem hiding this comment.
This entire block targets code that no longer exists on main. The React guide was reworked after this PR was opened and now uses a selectWallet() helper with the correct Object.values enumeration.
Please rebase onto main and drop the changes to react-wallet-connect.mdx.
Summary
Fixes #1070.
The DApp Connector API spec (SPECIFICATION.md) has wallets install their
InitialAPIunderwindow.midnightkeyed by a wallet-chosen identifier (typically a freshly-generated UUID), not a fixed property name. The official API reference itself documents discovery viaObject.values(window.midnight ?? {})(seeapi-reference/dapp-connector/README.md), andsdks/community/wallets/integration.mdxdocuments the sameObject.valuesscan as the recommended v4 pattern.Three guides/docs in this repo instead hardcoded
window.midnight.mnLace, which isundefinedagainst current Lace builds and produces exactly the error reported in #1070:docs/guides/react-wallet-connect.mdx— the guide named in the issuedocs/guides/nextjs-wallet-connect.mdx— same anti-pattern, same root causesdks/official/wallet-dev-guide.mdx— DApp integration example with the same bugAll three now discover the wallet via
Object.values(window.midnight ?? {}).find((w) => w.name === 'Lace'), consistent with the discovery pattern already documented elsewhere in this repo.Test plan
api-reference/dapp-connector/_media/SPECIFICATION.mdandapi-reference/dapp-connector/README.mdthat wallets are keyed by a wallet-chosen id (not a fixed property), and thatObject.values(window.midnight ?? {})is the documented discovery pattern.InitialAPI.nameandInitialAPI.rdnsare documented properties suitable for selecting a specific wallet (api-reference/dapp-connector/type-aliases/InitialAPI.md).