Feature: useBraintreePayPalOneTimePaymentSession hook#889
Conversation
🦋 Changeset detectedLatest commit: 8945074 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
There was a problem hiding this comment.
I pulled changes from main and ran npm install this updated a handful of packages to include a peer boolean.
| client: BraintreeClientInstance; | ||
| }) => Promise<BraintreePayPalCheckoutInstance>; | ||
| }; | ||
| [key: string]: unknown; |
There was a problem hiding this comment.
I added this b/c I am concerned with typing the Braintree namespace too tightly
| * @param setError - Error state setter | ||
| * @returns The payment session or null if creation fails | ||
| */ | ||
| export function createBraintreePaymentSession<T>( |
There was a problem hiding this comment.
Looks like this function is very similar to [createPaymentSession](https://github.com/paypal/paypal-js/blob/ffee35fcf23a510d691931ec237624edbb4762b2/packages/react-paypal-js/src/v6/utils.ts#L278) with same signature but a different error message. Can we consider reusing createPaymentSession with a configurable error message and we can remove this file.
There was a problem hiding this comment.
Yea I went back and forth on this. Mostly b/c I thought it meant updating every usage of the new util with an error message. Let me investigate further.
There was a problem hiding this comment.
We decided to take this up in a follow up, in order to keep the scope smaller.
| } | ||
|
|
||
| export interface BraintreeTokenizePaymentOptions { | ||
| payerID?: string; |
There was a problem hiding this comment.
Is the payerID/orderID naming in BraintreeTokenizePaymentOptions intentionally preserved? I notice BraintreeApprovalData was renamed to payerId/orderId in this PR. If the Braintree SDK's tokenizePayment() method still expects the old casing, then keeping it is correct. If not, this should be updated to match. Could you confirm which casing the SDK actually expects?
There was a problem hiding this comment.
Yea this is one of the wonky things about these values, tokenizePayment still expects the old casing. See here https://braintree.github.io/braintree-web/current/PayPalCheckoutV6.html#tokenizePayment
| payerID?: string; | ||
| orderID?: string; | ||
| payerId?: string; | ||
| orderId?: string; |
There was a problem hiding this comment.
I see in our test we still use
const mockApprovalData = {
payerID: "PAYER123",
orderID: "ORDER456",
};
Might be better to match this type?
const mockApprovalData: BraintreeApprovalData = {
payerId: "PAYER123",
orderId: "ORDER456",
};
There was a problem hiding this comment.
Yep, let me update that 👍
HackTheW2d
left a comment
HackTheW2d
left a comment
There was a problem hiding this comment.
LGTM! It's really great to see so many stable ref usage with useDeepCompareMemoize, useProxyCalls. 👍
nityasp
left a comment
nityasp
left a comment
There was a problem hiding this comment.
Looking good. Thanks for taking this up @EvanReinstein
4 participants
Adds a new React hook for managing one-time payment sessions with Braintree PayPal.
New hook: useBraintreePayPalOneTimePaymentSession
isPending, error, handleClickuseProxyPropsfor stable callback references anduseDeepCompareMemoizefor object/array options to avoid unnecessary session recreationfailedInstanceRefto prevent retry loopsNew utility:
createBraintreePaymentSession(braintreeUtils.ts)Braintree hooks
Reorganization
useBraintreePayPal hookand test into a Braintree/ subdirectory alongside the new hookType fix
BraintreeApprovalDatafields from payerID/orderID to payerId/orderId (camelCase)Exports
useBraintreePayPalOneTimePaymentSession,UseBraintreePayPalOneTimePaymentSessionProps, andUseBraintreePayPalOneTimePaymentSessionReturnare now exported from the package