-
Notifications
You must be signed in to change notification settings - Fork 123
CardFieldsProvider and context created #754
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
e97fec2
6a81a74
2ffa138
0139c17
51dd0c5
2dd02be
5c99ce1
623c0b8
d4bd4fc
41229fd
7ec8065
c0c3111
b7d8126
37bd290
58fd9d6
c2a5cfc
3b2e9ba
b6c2f70
bdc987d
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,5 @@ | ||
| --- | ||
| "@paypal/react-paypal-js": patch | ||
| --- | ||
|
|
||
| Created CardFieldsProvider and context for creating and providing Card Fields sessions |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,292 @@ | ||
| /** | ||
| * @jest-environment node | ||
| */ | ||
|
|
||
| import React from "react"; | ||
| import { renderToString } from "react-dom/server"; | ||
|
|
||
| import { usePayPal } from "../hooks/usePayPal"; | ||
| import { useCardFields, useCardFieldsSession } from "../hooks/useCardFields"; | ||
| import { INSTANCE_LOADING_STATE } from "../types"; | ||
| import { isServer } from "../utils"; | ||
| import { CardFieldsProvider } from "./CardFieldsProvider"; | ||
|
|
||
| import type { | ||
| CardFieldsSessionState, | ||
| CardFieldsStatusState, | ||
| } from "../context/CardFieldsProviderContext"; | ||
| import type { CardFieldsSessionType } from "./CardFieldsProvider"; | ||
|
|
||
| jest.mock("../hooks/usePayPal"); | ||
|
|
||
| jest.mock("../utils", () => ({ | ||
| ...jest.requireActual("../utils"), | ||
| isServer: () => true, | ||
|
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. Also, I don't see the provider using |
||
| })); | ||
|
|
||
| const mockUsePayPal = usePayPal as jest.MockedFunction<typeof usePayPal>; | ||
|
|
||
| const oneTimePaymentSessionType: CardFieldsSessionType = "one-time-payment"; | ||
| const savePaymentSessionType: CardFieldsSessionType = "save-payment"; | ||
|
|
||
| // Test utilites | ||
| function expectInitialStatusState(state: CardFieldsStatusState): void { | ||
| expect(state.cardFieldsError).toBe(null); | ||
| } | ||
|
|
||
| function expectInitialSessionState(state: CardFieldsSessionState): void { | ||
| expect(state.cardFieldsSession).toBe(null); | ||
| } | ||
|
|
||
| // Render helpers | ||
| function renderSSRCardFieldsProvider({ | ||
| sessionType, | ||
| children, | ||
| }: { | ||
| sessionType: CardFieldsSessionType; | ||
| children?: React.ReactNode; | ||
| }) { | ||
| const { cardFieldsSessionState, cardFieldsStatusState, TestComponent } = | ||
| setupSSRTestComponent(); | ||
|
|
||
| const html = renderToString( | ||
| <CardFieldsProvider sessionType={sessionType}> | ||
| <TestComponent>{children}</TestComponent> | ||
| </CardFieldsProvider>, | ||
| ); | ||
|
|
||
| return { html, cardFieldsSessionState, cardFieldsStatusState }; | ||
| } | ||
|
|
||
| function setupSSRTestComponent() { | ||
|
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. What makes this "SSR" specifically? |
||
| const cardFieldsStatusState: CardFieldsStatusState = { | ||
| cardFieldsError: null, | ||
| }; | ||
|
|
||
| const cardFieldsSessionState: CardFieldsSessionState = { | ||
| cardFieldsSession: null, | ||
| }; | ||
|
|
||
| function TestComponent({ | ||
| children = null, | ||
| }: { | ||
| children?: React.ReactNode; | ||
| }) { | ||
| const newCardFieldsStatusState = useCardFields(); | ||
| const newCardFieldsSessionState = useCardFieldsSession(); | ||
|
|
||
| Object.assign(cardFieldsStatusState, newCardFieldsStatusState); | ||
| Object.assign(cardFieldsSessionState, newCardFieldsSessionState); | ||
|
|
||
| return <>{children}</>; | ||
| } | ||
|
|
||
| return { cardFieldsStatusState, cardFieldsSessionState, TestComponent }; | ||
| } | ||
|
|
||
| describe("CardFieldsProvider SSR", () => { | ||
| beforeEach(() => { | ||
| mockUsePayPal.mockReturnValue({ | ||
| sdkInstance: null, | ||
| loadingStatus: INSTANCE_LOADING_STATE.PENDING, | ||
| eligiblePaymentMethods: null, | ||
| error: null, | ||
| }); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| describe("Server-Side Rendering", () => { | ||
| test("should verify isServer mock is working", () => { | ||
| expect(isServer()).toBe(true); | ||
| }); | ||
|
Comment on lines
+102
to
+104
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 a strange test, did this come from AI? I don't think this test suite should be checking this. |
||
|
|
||
| test("should not attempt DOM access during server rendering", () => { | ||
| // In Node environment, document should be undefined | ||
| expect(typeof document).toBe("undefined"); | ||
|
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. IMO, if we're meaning to test SSR, we should use and mock |
||
|
|
||
| // Will throw error if any DOM access is attempted during render | ||
| expect(() => | ||
| renderSSRCardFieldsProvider({ | ||
| sessionType: oneTimePaymentSessionType, | ||
| }), | ||
| ).not.toThrow(); | ||
| }); | ||
|
|
||
| test("should work correctly in SSR context", () => { | ||
| const { cardFieldsSessionState, cardFieldsStatusState } = | ||
| renderSSRCardFieldsProvider({ | ||
| sessionType: oneTimePaymentSessionType, | ||
| }); | ||
|
|
||
| expectInitialStatusState(cardFieldsStatusState); | ||
| expectInitialSessionState(cardFieldsSessionState); | ||
| }); | ||
|
|
||
| test("should render without warnings or errors", () => { | ||
| // This sdkInstance state would fail client side | ||
| mockUsePayPal.mockReturnValue({ | ||
| sdkInstance: null, | ||
| loadingStatus: INSTANCE_LOADING_STATE.REJECTED, | ||
| eligiblePaymentMethods: null, | ||
| error: null, | ||
| }); | ||
|
|
||
| const consoleSpy = jest | ||
| .spyOn(console, "error") | ||
| .mockImplementation(); | ||
|
|
||
| const { html } = renderSSRCardFieldsProvider({ | ||
| sessionType: oneTimePaymentSessionType, | ||
| children: <div>SSR Content</div>, | ||
| }); | ||
|
|
||
| expect(consoleSpy).not.toHaveBeenCalled(); | ||
| expect(html).toBeTruthy(); | ||
| expect(typeof html).toBe("string"); | ||
| expect(html).toContain("SSR Content"); | ||
|
|
||
| consoleSpy.mockRestore(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("Hydration Preparation", () => { | ||
| test("should prepare serializable state for client hydration", () => { | ||
| const { cardFieldsSessionState, cardFieldsStatusState } = | ||
| renderSSRCardFieldsProvider({ | ||
| sessionType: oneTimePaymentSessionType, | ||
| }); | ||
|
|
||
| // Server state should be safe for serialization | ||
| const serializedSessionState = JSON.stringify( | ||
| cardFieldsSessionState, | ||
| ); | ||
| const serializedStatusState = JSON.stringify(cardFieldsStatusState); | ||
|
|
||
| expect(() => JSON.parse(serializedSessionState)).not.toThrow(); | ||
| expect(() => JSON.parse(serializedStatusState)).not.toThrow(); | ||
|
|
||
| const parsedSessionState = JSON.parse(serializedSessionState); | ||
| const parsedStatusState = JSON.parse(serializedStatusState); | ||
|
|
||
| expectInitialSessionState(parsedSessionState); | ||
| expectInitialStatusState(parsedStatusState); | ||
| }); | ||
|
|
||
| test("should maintain consistent state with different options", () => { | ||
| const { | ||
| cardFieldsStatusState: cardFieldsStatusState1, | ||
| cardFieldsSessionState: cardFieldsSessionState1, | ||
| } = renderSSRCardFieldsProvider({ | ||
| sessionType: oneTimePaymentSessionType, | ||
| }); | ||
| const { | ||
| cardFieldsStatusState: cardFieldsStatusState2, | ||
| cardFieldsSessionState: cardFieldsSessionState2, | ||
| } = renderSSRCardFieldsProvider({ | ||
| sessionType: savePaymentSessionType, | ||
| }); | ||
|
|
||
| // Both should have consistent initial state regardless of options | ||
| expectInitialStatusState(cardFieldsStatusState1); | ||
| expectInitialSessionState(cardFieldsSessionState1); | ||
|
|
||
| expectInitialStatusState(cardFieldsStatusState2); | ||
| expectInitialSessionState(cardFieldsSessionState2); | ||
| }); | ||
| }); | ||
|
|
||
| describe("Multiple renders", () => { | ||
| test("should maintain state consistency across multiple server renders", () => { | ||
| const { | ||
| html: html1, | ||
| cardFieldsStatusState: cardFieldsStatusState1, | ||
| cardFieldsSessionState: cardFieldsSessionState1, | ||
| } = renderSSRCardFieldsProvider({ | ||
| sessionType: oneTimePaymentSessionType, | ||
| }); | ||
| const { | ||
| html: html2, | ||
| cardFieldsStatusState: cardFieldsStatusState2, | ||
| cardFieldsSessionState: cardFieldsSessionState2, | ||
| } = renderSSRCardFieldsProvider({ | ||
| sessionType: oneTimePaymentSessionType, | ||
| }); | ||
|
|
||
| expectInitialStatusState(cardFieldsStatusState1); | ||
| expectInitialSessionState(cardFieldsSessionState1); | ||
| expectInitialStatusState(cardFieldsStatusState2); | ||
| expectInitialSessionState(cardFieldsSessionState2); | ||
| expect(html1).toBe(html2); | ||
| }); | ||
|
|
||
| test("should generate consistent HTML across renders", () => { | ||
| const { html: html1 } = renderSSRCardFieldsProvider({ | ||
| sessionType: oneTimePaymentSessionType, | ||
| children: <div data-testid="ssr-content">Test Content</div>, | ||
| }); | ||
| const { html: html2 } = renderSSRCardFieldsProvider({ | ||
| sessionType: oneTimePaymentSessionType, | ||
| children: <div data-testid="ssr-content">Test Content</div>, | ||
| }); | ||
|
|
||
| expect(html1).toBe(html2); | ||
| expect(html1).toContain('data-testid="ssr-content"'); | ||
| }); | ||
| }); | ||
|
|
||
| describe("Context isolation", () => { | ||
| const expectedSessionContextKeys = ["cardFieldsSession"] as const; | ||
| const expectedStatusContextKeys = ["cardFieldsError"] as const; | ||
|
|
||
| describe("useCardFields", () => { | ||
| test("should only return status context values", () => { | ||
| const { cardFieldsStatusState } = renderSSRCardFieldsProvider({ | ||
| sessionType: oneTimePaymentSessionType, | ||
| }); | ||
|
|
||
| const receivedStatusKeys = Object.keys(cardFieldsStatusState); | ||
| expect(receivedStatusKeys.sort()).toEqual( | ||
| [...expectedStatusContextKeys].sort(), | ||
| ); | ||
| }); | ||
|
|
||
| test("should not return session context values", () => { | ||
| const { cardFieldsStatusState } = renderSSRCardFieldsProvider({ | ||
| sessionType: oneTimePaymentSessionType, | ||
| }); | ||
|
|
||
| const receivedStatusKeys = Object.keys(cardFieldsStatusState); | ||
| expectedSessionContextKeys.forEach((key) => { | ||
| expect(receivedStatusKeys).not.toContain(key); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| describe("useCardFieldsSession", () => { | ||
| test("should only return session context values", () => { | ||
| const { cardFieldsSessionState } = renderSSRCardFieldsProvider({ | ||
| sessionType: oneTimePaymentSessionType, | ||
| }); | ||
|
|
||
| const receivedSessionKeys = Object.keys(cardFieldsSessionState); | ||
| expect(receivedSessionKeys.sort()).toEqual( | ||
| [...expectedSessionContextKeys].sort(), | ||
| ); | ||
| }); | ||
|
|
||
| test("should not return status context values", () => { | ||
| const { cardFieldsSessionState } = renderSSRCardFieldsProvider({ | ||
| sessionType: oneTimePaymentSessionType, | ||
| }); | ||
|
|
||
| const receivedSessionKeys = Object.keys(cardFieldsSessionState); | ||
| expectedStatusContextKeys.forEach((key) => { | ||
| expect(receivedSessionKeys).not.toContain(key); | ||
| }); | ||
| }); | ||
| }); | ||
| }); | ||
| }); | ||
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.
These tests are weird to me since we don't have SSR logic in the provider. Some of these tests seem incorrect, others are testing functionality I'm not sure we should be testing for SSR specifically (e.g.
Context isolation), and some I don't think we're actually specifically coding for (e.g.should maintain state consistency across multiple server renders).IMO we should add these when we have SSR logic, and we should be more specific / careful about what we're testing.
Uh oh!
There was an error while loading. Please reload this page.
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.
Great callout! This tests were implemented following the tests
PayPalProviderhad for SSR. This subject was brought up with the team. The SSR tests files for Card fields and PayPal providers will be removed in a new PR. SSR tests will be added as needed.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.
Removed here