Skip to content

Commit 0cb1a60

Browse files
authored
Merge pull request #819 from paypal/fix/use-eligible-methods-hook
Fix/use eligible methods hook
2 parents d13a400 + 3d329a4 commit 0cb1a60

File tree

5 files changed

+74
-26
lines changed

5 files changed

+74
-26
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@paypal/react-paypal-js": patch
3+
---
4+
5+
Integrate useEligibleMethods hook into PayLaterOneTimePaymentButton component to automatically fetch and dispatch eligibility to context. Also fixes React Strict Mode compatibility by resetting lastFetchRef on cleanup.

packages/react-paypal-js/src/v6/components/PayLaterOneTimePaymentButton.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { usePayPal } from "../hooks/usePayPal";
55

66
import type { UsePayLaterOneTimePaymentSessionProps } from "../hooks/usePayLaterOneTimePaymentSession";
77

8-
type PayLaterOneTimePaymentButtonProps =
8+
export type PayLaterOneTimePaymentButtonProps =
99
UsePayLaterOneTimePaymentSessionProps & {
1010
autoRedirect?: never;
1111
disabled?: boolean;

packages/react-paypal-js/src/v6/hooks/useEligibleMethods.test.ts

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -110,16 +110,17 @@ describe("useEligibleMethods", () => {
110110
});
111111

112112
describe("isLoading state", () => {
113-
test("should return isLoading=false initially when not fetching", () => {
113+
test("should return isLoading=true when no eligibility data and no error", () => {
114114
const { result } = renderHook(() => useEligibleMethods(), {
115115
wrapper: createWrapper({
116116
loadingStatus: INSTANCE_LOADING_STATE.PENDING,
117+
eligiblePaymentMethods: null,
117118
}),
118119
});
119120

120-
// isLoading reflects isFetching state, not loadingStatus
121-
// Initially false because no fetch has started yet (no sdkInstance)
122-
expect(result.current.isLoading).toBe(false);
121+
// isLoading is true when we don't have eligibility data yet
122+
// This prevents UI flash before effect runs
123+
expect(result.current.isLoading).toBe(true);
123124
});
124125

125126
test("should return isLoading=false when eligibility already in context", () => {
@@ -139,15 +140,23 @@ describe("useEligibleMethods", () => {
139140
expect(result.current.isLoading).toBe(false);
140141
});
141142

142-
test("should return isLoading=false when context has error", () => {
143+
test("should return isLoading=true when context has error but no eligibility data", () => {
144+
// Context error (SDK load failure) doesn't set eligibilityError,
145+
// but we still don't have eligibility data, so isLoading is true
143146
const { result } = renderHook(() => useEligibleMethods(), {
144147
wrapper: createWrapper({
145148
loadingStatus: INSTANCE_LOADING_STATE.REJECTED,
146149
error: new Error("Failed"),
150+
eligiblePaymentMethods: null,
147151
}),
148152
});
149153

150-
expect(result.current.isLoading).toBe(false);
154+
// isLoading is true because we don't have eligibility data
155+
// The context error is returned separately
156+
expect(result.current.isLoading).toBe(true);
157+
expect(result.current.error?.message).toContain(
158+
"PayPal context error",
159+
);
151160
});
152161
});
153162

@@ -185,26 +194,28 @@ describe("useEligibleMethods", () => {
185194
}),
186195
});
187196

188-
// isLoading is false because no fetch has started (no sdkInstance)
197+
// isLoading is true because we don't have eligibility data yet
198+
// This prevents UI flash before the effect has a chance to run
189199
expect(result.current).toEqual({
190200
eligiblePaymentMethods: null,
191-
isLoading: false,
201+
isLoading: true,
192202
error: null,
193203
});
194204
});
195205

196-
test("should return correct shape during error state", () => {
206+
test("should return correct shape during error state with no eligibility", () => {
197207
const { result } = renderHook(() => useEligibleMethods(), {
198208
wrapper: createWrapper({
199209
eligiblePaymentMethods: null,
200210
loadingStatus: INSTANCE_LOADING_STATE.REJECTED,
201211
}),
202212
});
203213

204-
// Error from hook's local state, not context
214+
// isLoading is true because we have no eligibility data and no eligibility error
215+
// (context error is different from eligibility fetch error)
205216
expect(result.current).toEqual({
206217
eligiblePaymentMethods: null,
207-
isLoading: false,
218+
isLoading: true,
208219
error: null,
209220
});
210221
});
@@ -298,7 +309,7 @@ describe("useEligibleMethods", () => {
298309
expect(mockDispatch).not.toHaveBeenCalled();
299310
});
300311

301-
test("should set isLoading=true while fetching and false after", async () => {
312+
test("should set isLoading=true while fetching", async () => {
302313
let resolvePromise: (value: unknown) => void;
303314
const pendingPromise = new Promise((resolve) => {
304315
resolvePromise = resolve;
@@ -315,13 +326,33 @@ describe("useEligibleMethods", () => {
315326
}),
316327
});
317328

318-
// Effect runs synchronously, so isLoading is already true
329+
// isLoading is true because no eligibility data yet
319330
expect(result.current.isLoading).toBe(true);
320331

321-
// After promise resolves, should be false
332+
// After promise resolves, isLoading is still true because
333+
// eligiblePaymentMethods in context hasn't been updated
334+
// (dispatch is mocked). In real usage, context would update.
322335
await act(async () => {
323336
resolvePromise!(mockEligibilityResult);
324337
});
338+
// The fetch itself completes but context isn't updated in this test setup
339+
// so isLoading remains true. See "isLoading state" tests for cases
340+
// where eligibility is in context.
341+
expect(result.current.isLoading).toBe(true);
342+
});
343+
344+
test("should return isLoading=false when eligibility data exists", () => {
345+
const mockSdkInstance = createMockSdkInstance();
346+
347+
const { result } = renderHook(() => useEligibleMethods(), {
348+
wrapper: createWrapper({
349+
sdkInstance: mockSdkInstance,
350+
eligiblePaymentMethods: mockEligibilityResult,
351+
loadingStatus: INSTANCE_LOADING_STATE.RESOLVED,
352+
}),
353+
});
354+
355+
// isLoading is false because eligibility data exists in context
325356
expect(result.current.isLoading).toBe(false);
326357
});
327358

packages/react-paypal-js/src/v6/hooks/useEligibleMethods.ts

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ export function useEligibleMethods(
7070
const [eligibilityError, setError] = useError();
7171
const [isFetching, setIsFetching] = useState(false);
7272

73+
// Use ref to access eligiblePaymentMethods in effect without adding to deps
74+
const eligiblePaymentMethodsRef = useRef(eligiblePaymentMethods);
75+
eligiblePaymentMethodsRef.current = eligiblePaymentMethods;
76+
7377
// Memoize payload to avoid unnecessary re-fetches when object reference changes
7478
const memoizedPayload = useDeepCompareMemoize(payload);
7579

@@ -100,7 +104,10 @@ export function useEligibleMethods(
100104

101105
// If eligibility exists and we haven't fetched anything yet (e.g., server hydration),
102106
// mark as fetched to avoid unnecessary re-fetch with same payload
103-
if (eligiblePaymentMethods && lastFetchRef.current === null) {
107+
if (
108+
eligiblePaymentMethodsRef.current &&
109+
lastFetchRef.current === null
110+
) {
104111
lastFetchRef.current = {
105112
instance: sdkInstance,
106113
payload: memoizedPayload,
@@ -140,26 +147,28 @@ export function useEligibleMethods(
140147

141148
return () => {
142149
isSubscribed = false;
150+
lastFetchRef.current = null; // Reset fetch tracking on unmount or dependency change
143151
};
144-
}, [
145-
sdkInstance,
146-
memoizedPayload,
147-
eligiblePaymentMethods,
148-
dispatch,
149-
setError,
150-
]);
152+
}, [sdkInstance, memoizedPayload, dispatch, setError]);
153+
154+
// isLoading should be true if:
155+
// 1. We're actively fetching, OR
156+
// 2. We don't have eligibility data yet and no error occurred
157+
// This prevents a flash where isLoading=false before the effect runs
158+
const isLoading =
159+
isFetching || (!eligiblePaymentMethods && !eligibilityError);
151160

152161
if (contextError) {
153162
return {
154163
eligiblePaymentMethods,
155-
isLoading: isFetching,
164+
isLoading,
156165
error: new Error(`PayPal context error: ${contextError}`),
157166
};
158167
}
159168

160169
return {
161170
eligiblePaymentMethods,
162-
isLoading: isFetching,
171+
isLoading,
163172
error: eligibilityError,
164173
};
165174
}

packages/react-paypal-js/src/v6/index.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,10 @@ export {
2323
PayPalCardFieldsProvider,
2424
type CardFieldsSessionType,
2525
} from "./components/PayPalCardFieldsProvider";
26-
export { PayLaterOneTimePaymentButton } from "./components/PayLaterOneTimePaymentButton";
26+
export {
27+
PayLaterOneTimePaymentButton,
28+
type PayLaterOneTimePaymentButtonProps,
29+
} from "./components/PayLaterOneTimePaymentButton";
2730
export { PayPalGuestPaymentButton } from "./components/PayPalGuestPaymentButton";
2831
export { PayPalOneTimePaymentButton } from "./components/PayPalOneTimePaymentButton";
2932
export { PayPalProvider } from "./components/PayPalProvider";

0 commit comments

Comments
 (0)