feat: unify OTP signer API with showOtpSignerPrompt and otpSignerState#1699
Conversation
- Replace headlessSigningFlow with showOtpSignerPrompt (defaults true) - Add showOtpSignerPrompt to react-ui CrossmintWalletProvider - Rename emailSignerState to otpSignerState in context - Update useWalletOtpSigner to use otpSignerState - Update demo app and README references Co-Authored-By: Alberto Elias <alberto.elias@paella.dev>
Original prompt from Alberto Elias
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Prompt To Fix All With AIThis is a comment left during a code review.
Path: .changeset/use-wallet-otp-signer.md
Line: 3
Comment:
**Breaking default-behavior change marked as `minor`**
`@crossmint/client-sdk-react-native-ui` is marked as a `minor` bump, but the change from `headlessSigningFlow=true` (default: headless, no UI) to `showOtpSignerPrompt=true` (default: shows UI) is a **breaking change** for existing react-native consumers.
Any consumer who did **not** previously set `headlessSigningFlow` was relying on the default headless behavior. After this upgrade, they will suddenly see OTP UI dialogs during signing flows without changing any code. Under semver, a change in default behavior that can break existing consumers warrants a `major` version bump.
```suggestion
"@crossmint/client-sdk-react-native-ui": major
```
At minimum, the migration guide / changeset description should prominently call out that react-native consumers relying on the headless default must now explicitly pass `showOtpSignerPrompt={false}` to preserve their previous behavior.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/client/ui/react-ui/src/providers/CrossmintWalletProvider.tsx
Line: 69
Comment:
**Disabling OTP prompt also silently disables passkey helper UI**
When `showOtpSignerPrompt` is `false`, `renderUI` is set to `undefined` on this line, so `renderWebUI` is never invoked. Because `renderWebUI` renders **both** the OTP dialogs **and** `PasskeyPrompt`, opting out of built-in OTP handling inadvertently suppresses the passkey helper UI even when `showPasskeyHelpers` is `true`.
Before this PR, `renderUI` was unconditionally set to `renderWebUI` in react-ui (headless OTP was not supported), so the passkey prompt was always rendered. A developer who sets `showOtpSignerPrompt` to `false` (to handle OTP manually via the hook) while expecting passkey helpers to remain active will silently lose that UI.
A cleaner fix would be to always pass `renderUI={renderWebUI}` and let `renderWebUI` (or `CrossmintWalletUIBaseProvider`) conditionally skip the email/phone dialogs based on `showOtpSignerPrompt`, while always rendering `PasskeyPrompt` when passkey helpers are enabled.
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: "feat: unify OTP sign..." |
packages/client/ui/react-ui/src/providers/CrossmintWalletProvider.tsx
Outdated
Show resolved
Hide resolved
🔥 Smoke Test Results✅ Status: Passed Statistics
✅ All smoke tests passed!All critical flows are working correctly. This is a non-blocking smoke test. Full regression tests run separately. |
…s to true Co-Authored-By: Alberto Elias <alberto.elias@paella.dev>
Prompt To Fix All With AIThis is a comment left during a code review.
Path: .changeset/use-wallet-otp-signer.md
Line: 3-5
Comment:
**Breaking default behavior for React Native warrants major bump**
The React Native `CrossmintWalletProvider` inverts its default behavior in this PR: `headlessSigningFlow` previously defaulted to `true` (no UI rendered), while `showOtpSignerPrompt` now defaults to `true` (UI **is** rendered). Existing React Native consumers who omitted `headlessSigningFlow` — relying on the headless default — will now unexpectedly see the built-in OTP dialogs after upgrading.
Under semver, a default-behavior reversal that causes consumers' apps to silently change at runtime is a breaking change. `@crossmint/client-sdk-react-native-ui` (and potentially `@crossmint/client-sdk-react-base`) should be bumped to `major` rather than `minor`.
```suggestion
"@crossmint/wallets-sdk": minor
"@crossmint/client-sdk-react-ui": minor
"@crossmint/client-sdk-react-native-ui": major
"@crossmint/client-sdk-react-base": major
```
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: "fix: remove explicit..." |
Move OTP dialog gating into the renderUI functions themselves so that passkey prompts are always rendered regardless of showOtpSignerPrompt. renderUI is now always passed to CrossmintWalletBaseProvider. Co-Authored-By: Alberto Elias <alberto.elias@paella.dev>
Prompt To Fix All With AIThis is a comment left during a code review.
Path: packages/client/react-base/src/providers/CrossmintWalletUIBaseProvider.tsx
Line: 12-13
Comment:
**Unused prop `showOtpSignerPrompt` in interface and component**
`showOtpSignerPrompt` is declared in `CrossmintWalletUIBaseProviderProps` (line 12) and destructured in the component (line 53), but it's never referenced in the component body. The conditional rendering logic was moved into the `renderUI` callbacks in both react-ui (`createRenderWebUI`) and react-native (`renderNativeUI`), making this prop dead code in the base provider.
Consider removing it from the interface and destructuring to keep the code clean.
```suggestion
showPasskeyHelpers?: boolean;
```
**Rule Used:** Remove unused code from PRs to keep them lean. Add... ([source](https://app.greptile.com/review/custom-context?memory=83771aea-3c5f-4a37-9170-8ac881cd5efd))
**Learnt From**
[Paella-Labs/crossbit-main#20960](https://github.com/Paella-Labs/crossbit-main/pull/20960)
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/client/ui/react-ui/src/providers/CrossmintWalletProvider.tsx
Line: 69
Comment:
**New function created on every render**
`createRenderWebUI(showOtpSignerPrompt)` is called inline on every render of `CrossmintWalletProvider`, producing a fresh function reference each time. Since this is a provider that sits high in the component tree, consider memoizing the result to maintain referential stability:
```
const renderUI = useMemo(() => createRenderWebUI(showOtpSignerPrompt), [showOtpSignerPrompt]);
```
Then pass `renderUI={renderUI}` to `CrossmintWalletBaseProvider`.
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: "fix: showOtpSignerPr..." |
Description
Unifies the OTP signing API across react-ui and react-native SDKs by:
Replacing
headlessSigningFlowwithshowOtpSignerPrompt— semantically inverted:true(default) means "show built-in OTP UI",falsemeans "handle manually viauseWalletOtpSignerhook". This prop is now available in both react-ui and react-native (previously react-native only).Renaming
emailSignerState→otpSignerState— reflects that the state handles both email and phone OTP signers, not just email.useWalletOtpSigneralready works in both SDKs (exports were in place); this PR ensures the underlying context property it reads from is correctly renamed.OTP gating is scoped to OTP dialogs only —
showOtpSignerPrompt={false}suppresses Email/Phone signer dialogs but does not suppress passkey helper UI. This is achieved by moving the gating logic into the platform-specific render functions (createRenderWebUIin react-ui,renderNativeUIin react-native) rather than nulling outrenderUIentirely.headlessSigningFlowdefaulted totrue(no UI rendered). NowshowOtpSignerPromptdefaults totrue(UI rendered). This inverts the default for React Native consumers. Intentional for wallets-v1 API unification, but worth confirming.renderUIis now always passed: Both react-ui and react-native always provide arenderUIfunction toCrossmintWalletBaseProvider. The OTP/passkey separation happens inside the render functions themselves. Verify this doesn't cause issues if downstream code assumedrenderUIcould be undefined when OTP UI was disabled.createRenderWebUIcreates a new function reference each render (react-ui, line 69). This is a minor perf concern — could be wrapped inuseMemoif re-render frequency becomes an issue. Worth a quick check thatCrossmintWalletUIBaseProviderdoesn't do deep equality checks onrenderUI.showOtpSignerPromptis still accepted byCrossmintWalletUIBaseProviderbut is no longer used there (gating moved to platform render functions). Could be cleaned up in a follow-up.Human review checklist
renderUIisundefinedwhen OTP is disabledcreateRenderWebUIrecreating on each render causes any perf issuesTest plan
pnpm lint— passes with no errorspnpm test:vitest— all 11 test suites pass (298+ tests)pnpm build:libs— all 17 packages build successfullyemailSignerStateorheadlessSigningFlowremain in.ts/.tsxfilesuseWalletOtpSigneris exported from bothreact-uiandreact-nativehooks/index.tsPackage updates
Updated changeset (
use-wallet-otp-signer.md) covering minor bumps for:@crossmint/wallets-sdk@crossmint/client-sdk-react-ui@crossmint/client-sdk-react-native-ui@crossmint/client-sdk-react-baseLink to Devin session: https://crossmint.devinenterprise.com/sessions/98bcfbc9efce43389139305a3f7b3c5f
Requested by: @albertoelias-crossmint