Skip to content

Commit cdeb74a

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 91f08d3 commit cdeb74a

File tree

5 files changed

+132
-109
lines changed

5 files changed

+132
-109
lines changed

src/components/Elements.tsx

Lines changed: 32 additions & 71 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.
@@ -101,74 +87,49 @@ interface PrivateElementsProps {
10187
*/
10288
export const Elements: FunctionComponent<ElementsProps> = ({
10389
stripe: rawStripeProp,
104-
options,
90+
options: optionsProp,
10591
children,
10692
}: 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) {
93+
const [inputs, setInputs] = React.useState({ rawStripe: rawStripeProp, options: optionsProp })
94+
const { rawStripe, options } = inputs
95+
React.useEffect(() => {
96+
const hasRawStripeChanged = rawStripe !== rawStripeProp
97+
const hasOptionsChanged = !isEqual(options, optionsProp)
98+
const canUpdate = rawStripe === null
99+
100+
if (hasRawStripeChanged && !canUpdate) {
121101
console.warn(
122102
'Unsupported prop change on Elements: You cannot change the `stripe` prop after setting it.'
123103
);
124104
}
125-
if (!isEqual(options, prevOptions)) {
105+
106+
if (hasOptionsChanged && !canUpdate) {
126107
console.warn(
127108
'Unsupported prop change on Elements: You cannot change the `options` prop after setting the `stripe` prop.'
128109
);
129110
}
130-
}
131111

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-
}
112+
if (hasRawStripeChanged && canUpdate) setInputs({ rawStripe: rawStripeProp, options: optionsProp })
113+
}, [rawStripe, options, rawStripeProp, optionsProp])
140114

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-
}
115+
const maybeStripe = usePromiseResolver(rawStripe)
116+
const stripe = validateStripe(maybeStripe)
117+
const [ctx, setContext] = React.useState<ElementsContextValue>(() => createElementsContext(null));
156118

157-
React.useEffect(() => {
158-
return (): void => {
159-
isMounted.current = false;
160-
};
161-
}, []);
162119

163120
React.useEffect(() => {
164-
const anyStripe: any = ctx.stripe;
121+
const anyStripe: any = stripe;
165122

166123
if (!anyStripe || !anyStripe._registerWrapper) {
167124
return;
168125
}
169126

170127
anyStripe._registerWrapper({name: 'react-stripe-js', version: _VERSION});
171-
}, [ctx.stripe]);
128+
}, [stripe]);
129+
130+
React.useEffect(() => {
131+
if (stripe) setContext(createElementsContext(stripe, options))
132+
}, [stripe, options])
172133

173134
return (
174135
<ElementsContext.Provider value={ctx}>{children}</ElementsContext.Provider>

src/utils/usePrevious.test.tsx

Lines changed: 0 additions & 27 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: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import {FunctionComponent} from 'react';
2+
import React from 'react';
3+
import { act } from 'react-dom/test-utils';
4+
import {mount, ReactWrapper} from 'enzyme';
5+
import {usePromiseResolver} from './usePromiseResolver';
6+
import { mockStripe } from '../../test/mocks';
7+
8+
const TestComponentInner: FunctionComponent<{value: any}> = () => null
9+
const TestComponent: FunctionComponent<{promiseLike: any}> = ({promiseLike}) => {
10+
const value = usePromiseResolver(promiseLike);
11+
return <TestComponentInner value={value} />
12+
};
13+
14+
const getHookValue = (wrapper: ReactWrapper) => wrapper.find(TestComponentInner).prop('value')
15+
16+
const createController = (): [Promise<unknown>, (value?: unknown) => Promise<void>, (reason?: any) => Promise<void>] => {
17+
let resolveFn: (value?: unknown) => Promise<void> = () => Promise.resolve()
18+
let rejectFn: (reason?: any) => Promise<void> = () => Promise.resolve()
19+
20+
const promise = new Promise((resolve, reject) => {
21+
const createVoidPromise = () => promise.then(() => undefined, () => undefined)
22+
23+
resolveFn = (value) => {
24+
resolve(value)
25+
return createVoidPromise()
26+
}
27+
28+
rejectFn = (reason) => {
29+
reject(reason)
30+
return createVoidPromise()
31+
}
32+
})
33+
34+
return [promise, resolveFn, rejectFn]
35+
}
36+
37+
describe('usePromiseResolver', () => {
38+
let stripe: ReturnType<typeof mockStripe>;
39+
40+
beforeEach(() => {
41+
stripe = mockStripe();
42+
})
43+
44+
it('returns value on mount when not promise given', () => {
45+
const wrapper = mount(<TestComponent promiseLike={stripe} />);
46+
expect(wrapper.find(TestComponentInner).prop('value')).toBe(stripe)
47+
});
48+
49+
it('returns null on mount when promise given', () => {
50+
const [promise] = createController()
51+
const wrapper = mount(<TestComponent promiseLike={promise} />);
52+
expect(getHookValue(wrapper)).toBeNull()
53+
});
54+
55+
it('returns value when given promise resolved', () => {
56+
const [promise, resolve] = createController()
57+
const wrapper = mount(<TestComponent promiseLike={promise} />);
58+
59+
return Promise.resolve(act(() => resolve(stripe))).then(() => {
60+
wrapper.update()
61+
expect(getHookValue(wrapper)).toBe(stripe)
62+
})
63+
});
64+
65+
it('returns null when given promise rejected', () => {
66+
const [promise,, reject] = createController()
67+
const wrapper = mount(<TestComponent promiseLike={promise} />);
68+
69+
return Promise.resolve(act(() => reject(new Error('Something went wrong')))).then(() => {
70+
wrapper.update()
71+
expect(getHookValue(wrapper)).toBeNull()
72+
})
73+
});
74+
});

src/utils/usePromiseResolver.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import React from 'react';
2+
import {isPromise} from '../utils/guards';
3+
4+
export const usePromiseResolver = <T>(mayBePromise: T | PromiseLike<T>): T | null => {
5+
const [resolved, setResolved] = React.useState(() => {
6+
return isPromise(mayBePromise) ? null : mayBePromise
7+
})
8+
9+
React.useEffect(() => {
10+
if (!isPromise(mayBePromise)) return setResolved(mayBePromise)
11+
12+
let isMounted = true
13+
14+
setResolved(null)
15+
mayBePromise.then(resolvedValue => {
16+
if (isMounted) setResolved(resolvedValue)
17+
}, () => undefined)
18+
19+
return () => {
20+
isMounted = false
21+
}
22+
23+
}, [mayBePromise])
24+
25+
return resolved
26+
};

0 commit comments

Comments
 (0)