-
Notifications
You must be signed in to change notification settings - Fork 123
v6 React error handling #721
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 2 commits
b323c0a
8dfccfe
619570b
4d3af30
5a06cbe
6e8f172
50e4a5c
693da58
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 |
|---|---|---|
|
|
@@ -8,7 +8,7 @@ import { usePayPal } from "../hooks/usePayPal"; | |
| import { INSTANCE_LOADING_STATE } from "../types/PayPalProviderEnums"; | ||
|
|
||
| import type { CreateInstanceOptions, PayPalV6Namespace } from "../types"; | ||
| import type { PayPalContextState } from "../components/PayPalProvider"; | ||
| import type { PayPalState } from "../context/PayPalProviderContext"; | ||
|
|
||
| // Test constants | ||
| export const TEST_CLIENT_TOKEN = "test-client-token"; | ||
|
|
@@ -74,27 +74,24 @@ function createMockPayPalNamespace(): PayPalV6Namespace { | |
| } | ||
|
|
||
| // State assertion helpers | ||
| function expectPendingState(state: Partial<PayPalContextState>): void { | ||
| function expectPendingState(state: Partial<PayPalState>): void { | ||
| expect(state.loadingStatus).toBe(INSTANCE_LOADING_STATE.PENDING); | ||
| } | ||
|
|
||
| function expectResolvedState(state: Partial<PayPalContextState>): void { | ||
| function expectResolvedState(state: Partial<PayPalState>): void { | ||
| expect(state.loadingStatus).toBe(INSTANCE_LOADING_STATE.RESOLVED); | ||
| expect(state.sdkInstance).toBeTruthy(); | ||
| expect(state.error).toBe(null); | ||
| } | ||
|
|
||
| function expectRejectedState( | ||
| state: Partial<PayPalContextState>, | ||
| error?: Error, | ||
| ): void { | ||
| function expectRejectedState(state: Partial<PayPalState>, error?: Error): void { | ||
| expect(state.loadingStatus).toBe(INSTANCE_LOADING_STATE.REJECTED); | ||
| expect(state.sdkInstance).toBe(null); | ||
| if (error) { | ||
| expect(state.error).toEqual(error); | ||
| } | ||
| } | ||
| function expectReloadingState(state: Partial<PayPalContextState>): void { | ||
| function expectReloadingState(state: Partial<PayPalState>): void { | ||
| // When props change, only loadingStatus is reset to PENDING | ||
| // Old instance and eligibility remain until new ones are loaded | ||
| expect(state.loadingStatus).toBe(INSTANCE_LOADING_STATE.PENDING); | ||
|
|
@@ -141,17 +138,12 @@ describe("PayPalProvider", () => { | |
| }); | ||
|
|
||
| test('should set loadingStatus to "rejected" when SDK fails to load', async () => { | ||
| await withConsoleSpy("error", async () => { | ||
| (loadCoreSdkScript as jest.Mock).mockRejectedValue( | ||
| new Error(TEST_ERROR_MESSAGE), | ||
| ); | ||
| const mockError = new Error(TEST_ERROR_MESSAGE); | ||
| (loadCoreSdkScript as jest.Mock).mockRejectedValue(mockError); | ||
|
|
||
| const { state } = renderProvider(); | ||
| const { state } = renderProvider(); | ||
|
|
||
| await waitFor(() => | ||
| expectRejectedState(state, new Error(TEST_ERROR_MESSAGE)), | ||
| ); | ||
| }); | ||
| await waitFor(() => expectRejectedState(state, mockError)); | ||
| }); | ||
|
|
||
| test.each<[string, "sandbox" | "production", string, string]>([ | ||
|
|
@@ -273,31 +265,21 @@ describe("PayPalProvider", () => { | |
| }); | ||
|
|
||
| test("should handle eligibility loading failure gracefully", async () => { | ||
| await withConsoleSpy("warn", async (spy) => { | ||
| const mockInstance = { | ||
| ...createMockSdkInstance(), | ||
| findEligibleMethods: jest | ||
| .fn() | ||
| .mockRejectedValue(new Error("Eligibility failed")), | ||
| }; | ||
|
|
||
| (loadCoreSdkScript as jest.Mock).mockResolvedValue({ | ||
| createInstance: jest.fn().mockResolvedValue(mockInstance), | ||
| }); | ||
| const mockError = new Error("Eligibility failed"); | ||
| const mockInstance = { | ||
| ...createMockSdkInstance(), | ||
| findEligibleMethods: jest.fn().mockRejectedValue(mockError), | ||
| }; | ||
|
|
||
| const { state } = renderProvider(); | ||
| (loadCoreSdkScript as jest.Mock).mockResolvedValue({ | ||
| createInstance: jest.fn().mockResolvedValue(mockInstance), | ||
| }); | ||
|
|
||
| await waitFor(() => expectResolvedState(state)); | ||
| const { state } = renderProvider(); | ||
|
|
||
| await waitFor(() => | ||
| expect(spy).toHaveBeenCalledWith( | ||
| "Failed to get eligible payment methods:", | ||
| expect.any(Error), | ||
| ), | ||
| ); | ||
| await waitFor(() => expectRejectedState(state, mockError)); | ||
|
|
||
| expect(state.eligiblePaymentMethods).toBe(null); | ||
| }); | ||
| expect(state.eligiblePaymentMethods).toBe(null); | ||
| }); | ||
| }); | ||
|
|
||
|
|
@@ -563,25 +545,12 @@ describe("Auto-memoization", () => { | |
| }); | ||
| }); | ||
|
|
||
| describe("usePayPal", () => { | ||
|
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. I moved this to its own test suite for |
||
| test("should throw an error when used without PayPalProvider", () => { | ||
| withConsoleSpy("error", () => { | ||
| const { TestComponent } = setupTestComponent(); | ||
|
|
||
| expect(() => render(<TestComponent />)).toThrow( | ||
| "usePayPal must be used within a PayPalProvider", | ||
| ); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| function setupTestComponent() { | ||
| const state: PayPalContextState = { | ||
| const state: PayPalState = { | ||
| loadingStatus: INSTANCE_LOADING_STATE.PENDING, | ||
| sdkInstance: null, | ||
| eligiblePaymentMethods: null, | ||
| error: null, | ||
| dispatch: jest.fn(), | ||
| }; | ||
|
|
||
| function TestComponent({ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,25 +10,22 @@ import { | |
| INSTANCE_LOADING_STATE, | ||
| INSTANCE_DISPATCH_ACTION, | ||
| } from "../types/PayPalProviderEnums"; | ||
| import { useCompareMemoize } from "../utils"; | ||
| import { toError, useCompareMemoize } from "../utils"; | ||
|
|
||
| import type { | ||
| CreateInstanceOptions, | ||
| Components, | ||
| CreateInstanceOptions, | ||
| EligiblePaymentMethodsOutput, | ||
| LoadCoreSdkScriptOptions, | ||
| PayPalV6Namespace, | ||
| SdkInstance, | ||
| } from "../types"; | ||
| import type { InstanceAction } from "../context/PayPalProviderContext"; | ||
|
|
||
| export interface PayPalContextState { | ||
| sdkInstance: SdkInstance<readonly [Components, ...Components[]]> | null; | ||
| eligiblePaymentMethods: EligiblePaymentMethodsOutput | null; | ||
| error: Error | null; | ||
| dispatch: React.Dispatch<InstanceAction>; | ||
|
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.
|
||
| loadingStatus: INSTANCE_LOADING_STATE; | ||
| } | ||
| import type { | ||
| InstanceAction, | ||
| PayPalState, | ||
| } from "../context/PayPalProviderContext"; | ||
| import type { usePayPal } from "../hooks/usePayPal"; | ||
|
|
||
| type PayPalProviderProps = CreateInstanceOptions< | ||
| readonly [Components, ...Components[]] | ||
|
|
@@ -37,6 +34,10 @@ type PayPalProviderProps = CreateInstanceOptions< | |
| children: React.ReactNode; | ||
| }; | ||
|
|
||
| /** | ||
| * {@link PayPalProvider} creates the SDK script, component scripts, runs eligibility, then | ||
| * provides these in {@link PayPalContext} to child components via the {@link usePayPal} hook. | ||
| */ | ||
| export const PayPalProvider: React.FC<PayPalProviderProps> = ({ | ||
| clientMetadataId, | ||
| clientToken, | ||
|
|
@@ -78,13 +79,9 @@ export const PayPalProvider: React.FC<PayPalProviderProps> = ({ | |
| } | ||
| } catch (error) { | ||
| if (isSubscribed) { | ||
| const errorInstance = | ||
| error instanceof Error | ||
| ? error | ||
| : new Error(String(error)); | ||
| dispatch({ | ||
| type: INSTANCE_DISPATCH_ACTION.SET_ERROR, | ||
| value: errorInstance, | ||
| value: toError(error), | ||
| }); | ||
| } | ||
| } | ||
|
|
@@ -135,13 +132,9 @@ export const PayPalProvider: React.FC<PayPalProviderProps> = ({ | |
| }); | ||
| } catch (error) { | ||
| if (isSubscribed) { | ||
| const errorInstance = | ||
| error instanceof Error | ||
| ? error | ||
| : new Error(String(error)); | ||
| dispatch({ | ||
| type: INSTANCE_DISPATCH_ACTION.SET_ERROR, | ||
| value: errorInstance, | ||
| value: toError(error), | ||
| }); | ||
| } | ||
| } | ||
|
|
@@ -191,10 +184,10 @@ export const PayPalProvider: React.FC<PayPalProviderProps> = ({ | |
| }); | ||
| } catch (error) { | ||
| if (isSubscribed) { | ||
| console.warn( | ||
| "Failed to get eligible payment methods:", | ||
| error, | ||
| ); | ||
| dispatch({ | ||
| type: INSTANCE_DISPATCH_ACTION.SET_ERROR, | ||
| value: toError(error), | ||
| }); | ||
| } | ||
| } | ||
| }; | ||
|
|
@@ -206,20 +199,18 @@ export const PayPalProvider: React.FC<PayPalProviderProps> = ({ | |
| }; | ||
| }, [state.sdkInstance, state.loadingStatus]); | ||
|
|
||
| const contextValue = useMemo( | ||
| const contextValue: PayPalState = useMemo( | ||
| () => ({ | ||
| sdkInstance: state.sdkInstance, | ||
| eligiblePaymentMethods: state.eligiblePaymentMethods, | ||
| error: state.error, | ||
| dispatch, | ||
| loadingStatus: state.loadingStatus, | ||
| }), | ||
| [ | ||
| state.sdkInstance, | ||
| state.eligiblePaymentMethods, | ||
| state.error, | ||
| state.loadingStatus, | ||
| dispatch, | ||
| ], | ||
| ); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| import { renderHook } from "@testing-library/react-hooks"; | ||
|
|
||
| import { useIsMountedRef } from "./useIsMounted"; | ||
|
|
||
| describe("useIsMountedRef", () => { | ||
| it("should return true if the component is still mounted", () => { | ||
| const { result } = renderHook(() => useIsMountedRef()); | ||
|
|
||
| expect(result.current.current).toBe(true); | ||
| }); | ||
|
|
||
| it("should return false if the component has unmounted", () => { | ||
| const { result, unmount } = renderHook(() => useIsMountedRef()); | ||
|
|
||
| unmount(); | ||
|
|
||
| expect(result.current.current).toBe(false); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| import { useEffect, useRef } from "react"; | ||
|
|
||
| import type React from "react"; | ||
|
|
||
| /** | ||
| * Return a {@link React.MutableRefObject} a stable ref that's `true` if the component is mounted, `false` otherwise. | ||
| * | ||
| * The return must, unfortunately be included in dependency arrays. See the issue here: [\[eslint-plugin-react-hooks\] allow configuring custom hooks as "static" #16873](https://github.com/facebook/react/issues/16873). | ||
| */ | ||
| export function useIsMountedRef(): React.MutableRefObject<boolean> { | ||
| const isMounted = useRef(false); | ||
|
|
||
| useEffect(() => { | ||
| isMounted.current = true; | ||
|
|
||
| return () => { | ||
| isMounted.current = false; | ||
| }; | ||
| }, []); | ||
|
|
||
| return isMounted; | ||
| } |
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.
This allows importing types only for use in
jsdocwithout the linter reporting an unused variable error.