-
Notifications
You must be signed in to change notification settings - Fork 123
Small updates for react #752
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
92e3c5e
373dbd3
05d4948
18b4a69
26027a6
a73fc6d
33dbc12
18a2311
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| --- | ||
| "@paypal/react-paypal-js": patch | ||
| --- | ||
|
|
||
| - Default `PayPalProvider` `components` to `["paypal-payments"]`. | ||
| - Update session hooks to check `loadingStatus` before returning an error for no `sdkInstance`. | ||
| - `PayPalContext` export was removed since merchants won't need to use that directly. | ||
| - Check only `window` for `isServer` SSR function. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import { usePayPal } from "./usePayPal"; | |
| import { useIsMountedRef } from "./useIsMounted"; | ||
| import { useError } from "./useError"; | ||
| import { useProxyProps } from "../utils"; | ||
| import { INSTANCE_LOADING_STATE } from "../types/PayPalProviderEnums"; | ||
|
|
||
| import type { | ||
| BasePaymentSessionReturn, | ||
|
|
@@ -33,7 +34,7 @@ export function usePayLaterOneTimePaymentSession({ | |
| orderId, | ||
| ...callbacks | ||
| }: PayLaterOneTimePaymentSessionProps): BasePaymentSessionReturn { | ||
| const { sdkInstance } = usePayPal(); | ||
| const { sdkInstance, loadingStatus } = usePayPal(); | ||
| const isMountedRef = useIsMountedRef(); | ||
| const sessionRef = useRef<OneTimePaymentSession | null>(null); // handle cleanup | ||
| const proxyCallbacks = useProxyProps(callbacks); | ||
|
|
@@ -46,10 +47,12 @@ export function usePayLaterOneTimePaymentSession({ | |
|
|
||
| // Separate error reporting effect to avoid infinite loops with proxyCallbacks | ||
| useEffect(() => { | ||
| if (!sdkInstance) { | ||
| if (sdkInstance) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change prevents an error when there's no
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sweet change, will make sure to add this as well to the new CardFieldsProvider component that checks the |
||
| setError(null); | ||
| } else if (loadingStatus !== INSTANCE_LOADING_STATE.PENDING) { | ||
| setError(new Error("no sdk instance available")); | ||
| } | ||
| }, [sdkInstance, setError]); | ||
| }, [sdkInstance, setError, loadingStatus]); | ||
|
|
||
| useEffect(() => { | ||
| if (!sdkInstance) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import { usePayPal } from "./usePayPal"; | |
| import { useIsMountedRef } from "./useIsMounted"; | ||
| import { useError } from "./useError"; | ||
| import { useProxyProps } from "../utils"; | ||
| import { INSTANCE_LOADING_STATE } from "../types/PayPalProviderEnums"; | ||
| import { | ||
| OneTimePaymentSession, | ||
| PayPalPresentationModeOptions, | ||
|
|
@@ -31,7 +32,7 @@ export function usePayPalOneTimePaymentSession({ | |
| orderId, | ||
| ...callbacks | ||
| }: UsePayPalOneTimePaymentSessionProps): BasePaymentSessionReturn { | ||
| const { sdkInstance } = usePayPal(); | ||
| const { sdkInstance, loadingStatus } = usePayPal(); | ||
| const isMountedRef = useIsMountedRef(); | ||
| const sessionRef = useRef<OneTimePaymentSession | null>(null); | ||
| const proxyCallbacks = useProxyProps(callbacks); | ||
|
|
@@ -48,10 +49,12 @@ export function usePayPalOneTimePaymentSession({ | |
|
|
||
| // Separate error reporting effect to avoid infinite loops with proxyCallbacks | ||
| useEffect(() => { | ||
| if (!sdkInstance) { | ||
| if (sdkInstance) { | ||
| setError(null); | ||
| } else if (loadingStatus !== INSTANCE_LOADING_STATE.PENDING) { | ||
| setError(new Error("no sdk instance available")); | ||
| } | ||
| }, [sdkInstance, setError]); | ||
| }, [sdkInstance, setError, loadingStatus]); | ||
|
Comment on lines
50
to
+57
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How this conditions are set up would make the error state persist
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If there's no
Yes this is what we want, I believe; unless, is there a non-error state I'm missing when there's no
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the case I was thinking when making my suggestion:
Based on your response it seems current implementation achieves the desired experience.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah right, yes, I think we want the |
||
|
|
||
| useEffect(() => { | ||
| if (!sdkInstance) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most merchants will be using
paypal-paymentsso it's a safe default.