feat: unifies api for both otp signers #1313
Conversation
🦋 Changeset detectedLatest commit: 26b7718 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 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 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
| } | ||
|
|
||
| if (!context.sendOtp || !context.verifyOtp || !context.reject) { | ||
| throw new Error("Otp signer functions are not available. Make sure you're using an otp signer wallet."); |
There was a problem hiding this comment.
This error seems a bit hard to parse, do we call these "otp signer wallet" when you create a wallet?
Doesnt a wallet have multple signers? If so does only one of them need to have OTP?
There was a problem hiding this comment.
Somehting like "The wallet in context doesn't have any OTP signer configured. Register an OTP signer first before initializing this hook."
| throw new Error("useWalletOtpSigner must be used within CrossmintWalletProvider"); | ||
| } | ||
|
|
||
| if (!context.sendOtp || !context.verifyOtp || !context.reject) { |
There was a problem hiding this comment.
nit - re these nullable checks? If so, can you pls use context.sendOtp == null etc to be explicit (Crossmint code convention)
| const loggedInUserEmail = experimental_customAuth?.email ?? null; | ||
| const { wallet, getOrCreateWallet, status: walletStatus } = useWallet(); | ||
| const { needsAuth, sendEmailWithOtp, verifyOtp } = useWalletEmailSigner(); | ||
| const { needsAuth, sendOtp, verifyOtp } = useWalletOtpSigner(); |
There was a problem hiding this comment.
needsAuth -- dont use Auth, name it something like isInitialized (and invert it)
| const loggedInUserEmail = experimental_customAuth?.email ?? null; | ||
| const { wallet, getOrCreateWallet, status: walletStatus } = useWallet(); | ||
| const { needsAuth, sendEmailWithOtp, verifyOtp } = useWalletEmailSigner(); | ||
| const { needsAuth, sendOtp, verifyOtp } = useWalletOtpSigner(); |
There was a problem hiding this comment.
walletSignerNeedsSetup
otpSignerNeedsSetup
otpSignerInitialized
| const loggedInUserEmail = experimental_customAuth?.email ?? null; | ||
| const { wallet, getOrCreateWallet, status: walletStatus } = useWallet(); | ||
| const { needsAuth, sendEmailWithOtp, verifyOtp } = useWalletEmailSigner(); | ||
| const { needsAuth, sendOtp, verifyOtp } = useWalletOtpSigner(); |
There was a problem hiding this comment.
I may be wrong but iirc its an anti pattern to have read and write properties in the same hook
There was a problem hiding this comment.
where does that come from? The most used hook in react has both a read and write state:
const [count, setCount] = useState(0);
3 participants
Description
Our onAuthRequired callbacks were specific to email signer, this uses the
OtpSignernaming to bring both email and phone under the same umbrellaTest plan
Tested with demo apps
Package updates
react-ui: minor
react-native: minor
wallets: minor