disconnect api #1307
disconnect api #1307
Conversation
🦋 Changeset detectedLatest commit: 7df3a3e The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
turbocrime
force-pushed
the
turbocrime/disconnect-method-only
branch
5 times, most recently
from
f052cef
to
b607f24
Compare
| readonly manifest: string; | ||
| } | ||
|
|
||
| declare global { | ||
| interface Window { | ||
| /** Records upon this global should identify themselves by a field name | ||
| * matching the origin of the provider. */ | ||
| readonly [PenumbraSymbol]?: undefined | Readonly<Record<string, PenumbraInjection>>; |
There was a problem hiding this comment.
question: can we export some function like getInjectedWallet(walletOrigin) instead of using PenumbraSymbol?
I imagine external users implement the same API for wallets, so a set of function could help them:
getInjectedWallet(walletOrigin)– for dApp devsgetInjectedWallets()– list all browser walletsinjectWallet({ connect, disconnect, onConnectionChange ... })– for wallet developersonWalletInjected(callback)– a function that fires a callback each time a wallet is added (no need to wait for page reload). For thiswindow[PenumbraSymbol]should probably be converted to a proxy object to control additions and deletions, idk.
getInjectedWallet could become the beginning of this improved DX
There was a problem hiding this comment.
see utils in create.ts
other aspects of this should probably happen after externally_connectable
injection should generally happen before page scripts load unless not approved.
adding callback functionality seems less useful, as the wallet client and all requests will be entirely promise-based anyway
react wrapper in another PR implements context and hooks, for that style
There was a problem hiding this comment.
injection should generally happen before page scripts load unless not approved
agree except for the case when user installs the extension and the page is loaded. Real life scenario:
- user enters the frontend
- frontend says "to get started, install the extension"
- they install the extension
- page didn't react – for now
i know it's a small thing but, in my opinion, developers will find the onWalletInjected(callback) useful
There was a problem hiding this comment.
the page must reload before a new content script can appear.
There was a problem hiding this comment.
react wrapper in #1293
and another style, connect/query use, is visible in penumbra-zone/nextjs-penumbra-client-example@228b6eb
and the new readme for this package https://github.com/penumbra-zone/web/pull/1307/files#diff-7b3f04edbf937e50b69972d84ca0e92c9078c962fb4965bb7647b7c6ba74c296
turbocrime
force-pushed
the
turbocrime/disconnect-method-only
branch
from
cc0465a
to
31209d4
Compare
packages/client/README.md
Outdated
There was a problem hiding this comment.
elaborated comments
turbocrime
force-pushed
the
turbocrime/disconnect-method-only
branch
from
31209d4
to
5814d33
Compare
turbocrime
force-pushed
the
turbocrime/disconnect-method-only
branch
from
1284f3e
to
95db1fd
Compare
Screen.Recording.2024-06-25.at.15.26.33.mov
closes #1192
adds disconnect method to page api
page injection is refactored as a class to more explicitly represent and control injection state
origin-agnostic connection init helpers are exported from client package
transport may convey a simple 'false' indicating termination
disconnect minifront ui ready in merged #1276 but commented out
still needs a type guard for the provider manifest