Add react hooks for usePayer and useIdentity - #1876
Conversation
🦋 Changeset detectedLatest commit: 722cab6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 48 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 |
BundleMonFiles updated (3)
Unchanged files (147)
Total files change +459B +0.08% Final result: ✅ View report in BundleMon website ➡️ |
8e5979d to
a568f1f
Compare
trevor-cortex
left a comment
There was a problem hiding this comment.
Summary
Adds two new public hooks to @solana/react: usePayer(client) and useIdentity(client). Both read the corresponding signer off the client, and — when the client also advertises subscribeToPayer/subscribeToIdentity per the ClientWithSubscribeTo* convention — subscribe reactively via useSyncExternalStore so the returned value always reflects the latest signer. Clients without the subscribe hook fall back to a no-op subscribe and read the value once. To make the hooks robust against wallet-plugin getters that throw when no wallet is connected, both return TransactionSigner | undefined and swallow any error thrown by the getter into undefined — a deliberate departure from the non-optional plugin type. Ships with test-utility renderHook browser tests, __typetests__ for the public signature, and a minor changeset.
Also rolls in a batch of unrelated prettier reformatting across several existing test files and adds a blank line to examples/react-app/src/routes/root.tsx — presumably from pnpm style:fix. Harmless drive-by cleanup.
Key things to watch out for
- Swallowing all errors from the getter is a design choice, not just wallet-not-connected. The
try { return client.<x>; } catch { return undefined; }treats any exception — including bugs in a plugin's getter or a corrupted state — as "signer unavailable". The PR description justifies this viauseSyncExternalStore's incompatibility with throwing snapshots. It's the right call for the wallet-disconnected path, but it does mean genuine bugs will silently surface as an absent signer rather than an error boundary hit. Might be worth a note in the docblock and/or a__DEV__console.warnon caught errors so developers can spot unexpected throws during development. - Reactive contract requires a stable
client.<x>reference for equal snapshots.useSyncExternalStoreusesObject.isbetween snapshots; a getter that returns a new signer object on every read would cause infinite re-renders. That's the plugin's contract to uphold (and it's implied bysubscribe-to.ts's "consumers that bail on reference-equal snapshots… will filter redundant notifications out for free"), but worth calling out for plugin authors. - README not updated. The
@solana/reactREADME's "Kit client bindings" section documentsClientProvider,useClient,useClientCapability, etc. and the repo's TypeScript READMEs skill says new public APIs should be added there. These two hooks should get a short section — especially because theundefined-on-throw behaviour and the "mutation withoutsubscribeTo*isn't observed until the next render" semantic (which the last test in each file exercises) aren't obvious from the type signature alone.
Notes for subsequent reviewers
- The hooks intentionally take
clientas an argument rather than reading it fromClientProviderviauseClient/useClientCapability. That's consistent with the newer hook style (usePlanTransaction,useSendTransaction, etc.) but does differ from theuseClient*-based pattern documented in the README's "Kit client bindings" intro — worth checking whether this is the intended direction for capability hooks going forward, and whether the README's framing should be adjusted. - The type tests use
ClientWithSubscribeToIdentity/ClientWithSubscribeToPayeralone as the negative case (@ts-expect-errorbecause the base capability is missing). That correctly verifies the base capability is required; there's no negative test for passing an object that has neither, which would also be worth locking in. NOOP_UNSUBSCRIBE = () => {}is duplicated across both files. Fine as-is (one line, module-local), but if a thirduse<Capability>hook lands with the same pattern it would be worth pulling into a shared internal helper.- Changeset file has no trailing newline (
\ No newline at end of filein the diff). Trivial —style:fixor the changeset CLI will normalize it. - Prettier reformatting in
useAction-test.browser.tsx,useReactiveStoreLifecycle-test.browser.tsx,useSubscription-test.browser.tsx,useRequestQuery-test.browser.tsx,useRequestSWR-test.browser.tsx, andClientProvider-test.browser.tsxis behaviour-neutral; safe to skim past.
|
Documentation Preview: https://kit-docs-7y7nkuz0x-anza-tech.vercel.app |
a568f1f to
52df81f
Compare
trevor-cortex
left a comment
There was a problem hiding this comment.
Re-review
(Submitting as COMMENT since I don't have approve permissions on this repo — consider this an approval.)
Since my last pass, the main change is the new "Payer & identity" README section (packages/react/README.md), which addresses the primary point from my previous review. It documents both hooks, the undefined-while-unavailable return, and the reactive vs. fixed-client subscription behaviour — and the example is framed around useClient<AppClient>(), which also resolves my earlier note about aligning with the README's useClient-based intro. The changeset was also expanded to explicitly call out that a throwing plugin getter surfaces as undefined, which covers the discoverability concern about the swallow-all-errors behaviour at the release-notes level.
Everything else is unchanged from my prior review; I won't re-raise the minor points already noted there (NOOP_UNSUBSCRIBE duplication, missing trailing newline in the changeset, the optional __DEV__ warn on caught getter errors, and the absent negative typetest for a client with neither capability). All were explicitly optional and none block merging.
One truly optional thought: the docblocks and README say the hooks return undefined "while no payer/identity is available", which is accurate for the intended wallet-disconnected case but doesn't spell out that any exception from the getter is coerced to undefined. The changeset covers it, so I'm fine shipping as-is — just flagging in case you'd like the docblock @returns to mention it too.
Implementation, tests, and typetests all look solid. LGTM. 🚀
| export function usePayer(client: ClientWithPayer & Partial<ClientWithSubscribeToPayer>): TransactionSigner | undefined { | ||
| const subscribe = useCallback( | ||
| (onStoreChange: () => void) => | ||
| client.subscribeToPayer ? client.subscribeToPayer(onStoreChange) : NOOP_UNSUBSCRIBE, |
There was a problem hiding this comment.
nit: Shouldn't NOOP_UNSUBSCRIBE be called NOOP_SUBSCRIBE? I'd be also happy to dissolve it for () => {} since it's pretty self-explanatory.
| ): TransactionSigner | undefined { | ||
| const subscribe = useCallback( | ||
| (onStoreChange: () => void) => | ||
| client.subscribeToIdentity ? client.subscribeToIdentity(onStoreChange) : NOOP_UNSUBSCRIBE, |
Merge activity
|
34797f3 to
8d77e0e
Compare
52df81f to
5308c32
Compare
5308c32 to
722cab6
Compare

This PR adds
usePayeranduseIdentityreact hooks, to surface theclient.payerandclient.identityvaluesThese hooks are reactive, if the client implements the
subscribeTo[Payer/Identity]then the hooks subscribe to it (usinguseSyncExternalStore) and always provide the latest value.Unlike
payerandidentitywhich are not defined as optional on the client (the wallet plugin throws when it owns them and doesn't have a connected wallet to provide), I've made the hooks returnTransactionSigner | undefined. This is becauseuseSyncExternalStoredoesn't really work with exceptions, it seems to tear down the subscription if the value it's trying to read throws, so it doesn't recover properly from a wallet simply being disconnected and reconnected etc. even with anErrorBoundaryin place. I think in react it makes more sense to just make these values optional.