Skip to content

Commit 732b846

Browse files
committed
Fix Elements initialization in React Strict/Concurrent mode
Refs cannot be mutated and used to update state in the same time in rendering phase. As this is side-effect, it can produce various bugs in concurrent mode. In StrictMode Elements doesn't do transition from null to valid stripe instance. As in StrictMode React renders twice, `final` ref becomes `true`, but `ctx` state isn't changed. References: https://reactjs.org/docs/strict-mode.html#detecting-unexpected-side-effects facebook/react#18003 facebook/react#18545
1 parent 39e8c36 commit 732b846

File tree

6 files changed

+172
-116
lines changed

6 files changed

+172
-116
lines changed

src/components/Elements.test.tsx

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,30 @@ describe('Elements', () => {
152152
expect(result.current).toBe(mockElements);
153153
});
154154

155+
test('allows a transition from null to a valid Stripe object in StrictMode', async () => {
156+
let stripeProp: any = null;
157+
const spy = jest.fn();
158+
let TestComponent = () => (
159+
<React.StrictMode>
160+
<Elements stripe={stripeProp}>
161+
<ElementsConsumer>
162+
{({stripe, elements}) => {
163+
spy({stripe, elements});
164+
return null;
165+
}}
166+
</ElementsConsumer>
167+
</Elements>
168+
</React.StrictMode>
169+
);
170+
171+
const {rerender} = render(<TestComponent />);
172+
expect(spy).toBeCalledWith({stripe: null, elements: null});
173+
174+
stripeProp = mockStripe;
175+
rerender(<TestComponent />);
176+
expect(spy).toBeCalledWith({stripe: mockStripe, elements: mockElements});
177+
});
178+
155179
test('works with a Promise resolving to a valid Stripe object', async () => {
156180
const wrapper = ({children}: any) => (
157181
<Elements stripe={mockStripePromise}>{children}</Elements>
@@ -225,9 +249,7 @@ describe('Elements', () => {
225249
expect(mockStripe.elements).toHaveBeenCalledWith({foo: 'foo'});
226250
});
227251

228-
// TODO(christopher): support Strict Mode first
229-
// eslint-disable-next-line jest/no-disabled-tests
230-
test.skip('does not allow updates to options after the Stripe Promise is set in StrictMode', async () => {
252+
test('does not allow updates to options after the Stripe Promise is set in StrictMode', async () => {
231253
// Silence console output so test output is less noisy
232254
consoleWarn.mockImplementation(() => {});
233255

src/components/Elements.tsx

Lines changed: 36 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ import React from 'react';
66
import PropTypes from 'prop-types';
77

88
import {isEqual} from '../utils/isEqual';
9-
import {usePrevious} from '../utils/usePrevious';
10-
import {isStripe, isPromise} from '../utils/guards';
9+
import {usePromiseResolver} from '../utils/usePromiseResolver';
10+
import {isStripe} from '../utils/guards';
1111

1212
const INVALID_STRIPE_ERROR =
1313
'Invalid prop `stripe` supplied to `Elements`. We recommend using the `loadStripe` utility from `@stripe/stripe-js`. See https://stripe.com/docs/stripe-js/react#elements-props-stripe for details.';
@@ -23,28 +23,6 @@ const validateStripe = (maybeStripe: unknown): null | stripeJs.Stripe => {
2323
throw new Error(INVALID_STRIPE_ERROR);
2424
};
2525

26-
type ParsedStripeProp =
27-
| {tag: 'empty'}
28-
| {tag: 'sync'; stripe: stripeJs.Stripe}
29-
| {tag: 'async'; stripePromise: Promise<stripeJs.Stripe | null>};
30-
31-
const parseStripeProp = (raw: unknown): ParsedStripeProp => {
32-
if (isPromise(raw)) {
33-
return {
34-
tag: 'async',
35-
stripePromise: Promise.resolve(raw).then(validateStripe),
36-
};
37-
}
38-
39-
const stripe = validateStripe(raw);
40-
41-
if (stripe === null) {
42-
return {tag: 'empty'};
43-
}
44-
45-
return {tag: 'sync', stripe};
46-
};
47-
4826
interface ElementsContextValue {
4927
elements: stripeJs.StripeElements | null;
5028
stripe: stripeJs.Stripe | null;
@@ -66,6 +44,14 @@ export const parseElementsContext = (
6644
return ctx;
6745
};
6846

47+
const createElementsContext = (stripe: stripeJs.Stripe | null, options?: stripeJs.StripeElementsOptions) => {
48+
const elements = stripe ? stripe.elements(options) : null
49+
return {
50+
stripe,
51+
elements
52+
}
53+
}
54+
6955
interface ElementsProps {
7056
/**
7157
* A [Stripe object](https://stripe.com/docs/js/initializing) or a `Promise` resolving to a `Stripe` object.
@@ -99,66 +85,44 @@ interface PrivateElementsProps {
9985
*
10086
* @docs https://stripe.com/docs/stripe-js/react#elements-provider
10187
*/
102-
export const Elements: FunctionComponent<ElementsProps> = ({
103-
stripe: rawStripeProp,
104-
options,
105-
children,
106-
}: PrivateElementsProps) => {
107-
const final = React.useRef(false);
108-
const isMounted = React.useRef(true);
109-
const parsed = React.useMemo(() => parseStripeProp(rawStripeProp), [
110-
rawStripeProp,
111-
]);
112-
const [ctx, setContext] = React.useState<ElementsContextValue>(() => ({
113-
stripe: null,
114-
elements: null,
115-
}));
116-
117-
const prevStripe = usePrevious(rawStripeProp);
118-
const prevOptions = usePrevious(options);
119-
if (prevStripe !== null) {
120-
if (prevStripe !== rawStripeProp) {
88+
export const Elements: FunctionComponent<ElementsProps> = (props: PrivateElementsProps) => {
89+
const { children } = props
90+
91+
if (props.stripe === undefined) throw new Error(INVALID_STRIPE_ERROR);
92+
93+
const [inputs, setInputs] = React.useState({ rawStripe: props.stripe, options: props.options })
94+
React.useEffect(() => {
95+
const { rawStripe, options } = inputs
96+
const { stripe: nextRawStripe, options: nextOptions } = props
97+
98+
const canUpdate = rawStripe === null
99+
const hasRawStripeChanged = rawStripe !== nextRawStripe
100+
const hasOptionsChanged = !isEqual(options, nextOptions)
101+
102+
if (hasRawStripeChanged && !canUpdate) {
121103
console.warn(
122104
'Unsupported prop change on Elements: You cannot change the `stripe` prop after setting it.'
123105
);
124106
}
125-
if (!isEqual(options, prevOptions)) {
107+
108+
if (hasOptionsChanged && !canUpdate) {
126109
console.warn(
127110
'Unsupported prop change on Elements: You cannot change the `options` prop after setting the `stripe` prop.'
128111
);
129112
}
130-
}
131113

132-
if (!final.current) {
133-
if (parsed.tag === 'sync') {
134-
final.current = true;
135-
setContext({
136-
stripe: parsed.stripe,
137-
elements: parsed.stripe.elements(options),
138-
});
139-
}
114+
const nextInputs = { rawStripe: nextRawStripe, options: nextOptions }
115+
if (hasRawStripeChanged && canUpdate) setInputs(nextInputs)
116+
}, [inputs, props])
140117

141-
if (parsed.tag === 'async') {
142-
final.current = true;
143-
parsed.stripePromise.then((stripe) => {
144-
if (stripe && isMounted.current) {
145-
// Only update Elements context if the component is still mounted
146-
// and stripe is not null. We allow stripe to be null to make
147-
// handling SSR easier.
148-
setContext({
149-
stripe,
150-
elements: stripe.elements(options),
151-
});
152-
}
153-
});
154-
}
155-
}
118+
const [maybeStripe = null] = usePromiseResolver(inputs.rawStripe)
119+
const resolvedStripe = validateStripe(maybeStripe)
120+
const [ctx, setContext] = React.useState(() => createElementsContext(resolvedStripe, inputs.options));
156121

122+
const shouldInitialize = resolvedStripe !== null && ctx.stripe === null
157123
React.useEffect(() => {
158-
return (): void => {
159-
isMounted.current = false;
160-
};
161-
}, []);
124+
if (shouldInitialize) setContext(createElementsContext(resolvedStripe, inputs.options))
125+
}, [shouldInitialize, resolvedStripe, inputs.options])
162126

163127
React.useEffect(() => {
164128
const anyStripe: any = ctx.stripe;

src/utils/usePrevious.test.tsx

Lines changed: 0 additions & 30 deletions
This file was deleted.

src/utils/usePrevious.ts

Lines changed: 0 additions & 11 deletions
This file was deleted.

src/utils/usePromiseResolver.test.tsx

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import {renderHook, act} from '@testing-library/react-hooks';
2+
import {usePromiseResolver} from './usePromiseResolver';
3+
import { mockStripe } from '../../test/mocks';
4+
5+
const createImperivePromise = (): [Promise<unknown>, (value?: unknown) => Promise<void>, (reason?: any) => Promise<void>] => {
6+
let resolveFn: (value?: unknown) => Promise<void> = () => Promise.resolve()
7+
let rejectFn: (reason?: any) => Promise<void> = () => Promise.resolve()
8+
9+
const promise = new Promise((resolve, reject) => {
10+
const createVoidPromise = () => promise.then(() => undefined, () => undefined)
11+
12+
resolveFn = (value) => {
13+
resolve(value)
14+
return createVoidPromise()
15+
}
16+
17+
rejectFn = (reason) => {
18+
reject(reason)
19+
return createVoidPromise()
20+
}
21+
})
22+
23+
return [promise, resolveFn, rejectFn]
24+
}
25+
26+
describe('usePromiseResolver', () => {
27+
let stripe: ReturnType<typeof mockStripe>;
28+
29+
beforeEach(() => {
30+
stripe = mockStripe();
31+
})
32+
33+
it('returns resolved on mount when not promise given', () => {
34+
const {result} = renderHook(() => usePromiseResolver(stripe));
35+
expect(result.current).toEqual([stripe, undefined, 'resolved'])
36+
});
37+
38+
it('returns pending on mount when promise given', () => {
39+
const [promise] = createImperivePromise()
40+
const {result} = renderHook(() => usePromiseResolver(promise));
41+
expect(result.current).toEqual([undefined, undefined, 'pending'])
42+
});
43+
44+
it('returns resolved when given promise resolved', async () => {
45+
const [promise, resolve] = createImperivePromise()
46+
const {result} = renderHook(() => usePromiseResolver(promise));
47+
48+
await act(() => resolve(stripe))
49+
expect(result.current).toEqual([stripe, undefined, 'resolved'])
50+
});
51+
52+
it('returns rejected when given promise rejected', async () => {
53+
const [promise,, reject] = createImperivePromise()
54+
const {result} = renderHook(() => usePromiseResolver(promise));
55+
56+
const error = new Error('Something went wrong')
57+
await act(() => reject(error))
58+
expect(result.current).toEqual([undefined, error, 'rejected'])
59+
});
60+
});

src/utils/usePromiseResolver.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import React from 'react';
2+
import {isPromise} from '../utils/guards';
3+
4+
type PromisePending = [undefined, undefined, 'pending'];
5+
type PromiseResolved<T> = [T, undefined, 'resolved'];
6+
type PromiseRejected = [undefined, any, 'rejected'];
7+
type PromiseState<T> = PromisePending | PromiseResolved<T> | PromiseRejected;
8+
9+
const createPending = (): PromisePending => [undefined, undefined, 'pending'];
10+
11+
const createResolved = <T>(value: T): PromiseResolved<T> => [
12+
value,
13+
undefined,
14+
'resolved',
15+
];
16+
17+
const createRejected = (reason: any): PromiseRejected => [
18+
undefined,
19+
reason,
20+
'rejected',
21+
];
22+
23+
export const usePromiseResolver = <T>(
24+
mayBePromise: T | PromiseLike<T>
25+
): PromiseState<T> => {
26+
const [state, setState] = React.useState<PromiseState<T>>(() =>
27+
isPromise(mayBePromise) ? createPending() : createResolved(mayBePromise)
28+
);
29+
30+
React.useEffect(() => {
31+
if (!isPromise(mayBePromise)) return setState(createResolved(mayBePromise));
32+
33+
let isMounted = true;
34+
35+
setState(createPending());
36+
mayBePromise
37+
.then(
38+
(resolved) => createResolved(resolved),
39+
(error) => createRejected(error)
40+
)
41+
.then((nextState) => {
42+
if (isMounted) setState(nextState);
43+
});
44+
45+
return () => {
46+
isMounted = false;
47+
};
48+
}, [mayBePromise]);
49+
50+
return state;
51+
};

0 commit comments

Comments
 (0)