feat: add prebuilt PrimerCardForm and payment outcome flow (ACC-6495)#349
feat: add prebuilt PrimerCardForm and payment outcome flow (ACC-6495)#349OnurVar wants to merge 1 commit intoov/feat/ACC-6494from
Conversation
Generated by 🚫 Danger Swift against fe457d7 |
There was a problem hiding this comment.
3 issues found.
About Unblocked
Unblocked has been set up to automatically review your team's pull requests to identify genuine bugs and issues.
📖 Documentation — Learn more in our docs.
💬 Ask questions — Mention @unblocked to request a review or summary, or ask follow-up questions about your code.
👍 Give feedback — React to comments with 👍 or 👎 to help us improve.
⚙️ Customize — Adjust settings in your preferences.
| } else if (id.includes('cardholder') || id.includes('name')) { | ||
| fieldErrors.cardholderName = description; |
There was a problem hiding this comment.
The check id.includes('name') will match any error whose errorId contains the substring "name" — e.g. "payment_method_name_invalid", "username_required", etc. — and assign it to the cardholderName field, even if it's unrelated.
Consider tightening to match specifically on cardholder-related IDs:
} else if (id.includes('cardholder') || id.includes('card_holder') || id.includes('cardholder_name')) {
fieldErrors.cardholderName = description;
}| useEffect(() => { | ||
| if (!state.isReady || !state.activeMethod) { | ||
| return; | ||
| } | ||
|
|
||
| const method = state.activeMethod; | ||
| let cancelled = false; | ||
| const m = new PrimerHeadlessUniversalCheckoutRawDataManager(); | ||
| managerRef.current = m; | ||
|
|
||
| const callbacks = { | ||
| onValidation: (isValid: boolean, errors: PrimerError[] | undefined) => { | ||
| const parsed = parseValidationErrors(errors); | ||
| setState((prev) => ({ | ||
| ...prev, | ||
| cardFormState: { ...prev.cardFormState, isValid, errors: parsed }, | ||
| })); | ||
| }, | ||
| onBinDataChange: (binData: PrimerBinData) => { | ||
| setState((prev) => ({ | ||
| ...prev, | ||
| cardFormState: { ...prev.cardFormState, binData }, | ||
| })); | ||
| }, | ||
| onMetadataChange: (metadata: unknown) => { | ||
| setState((prev) => ({ | ||
| ...prev, | ||
| cardFormState: { ...prev.cardFormState, metadata }, | ||
| })); | ||
| }, | ||
| }; | ||
| lastManagerCallbacksRef.current = callbacks; | ||
|
|
||
| (async () => { | ||
| try { | ||
| await m.configure({ paymentMethodType: method, ...callbacks }); | ||
| if (cancelled) return; | ||
| const requiredFields = await m.getRequiredInputElementTypes(); | ||
| if (cancelled) return; | ||
| setState((prev) => ({ | ||
| ...prev, | ||
| cardFormState: { ...prev.cardFormState, requiredFields }, | ||
| })); | ||
| } catch (err) { | ||
| console.error(`${LOG} manager configure failed ${fmt(err)}`); | ||
| } | ||
| })(); | ||
|
|
||
| return () => { | ||
| cancelled = true; | ||
| m.cleanUp().catch((err) => console.warn(`${LOG} manager cleanUp failed ${fmt(err)}`)); | ||
| m.removeAllListeners(); | ||
| if (managerRef.current === m) { | ||
| managerRef.current = null; | ||
| } | ||
| lastManagerCallbacksRef.current = null; | ||
| setState((prev) => ({ ...prev, cardFormState: initialCardFormState })); | ||
| }; | ||
| }, [state.activeMethod, state.isReady]); |
There was a problem hiding this comment.
The raw-data manager lifecycle effect lists [state.activeMethod, state.isReady] as dependencies. Any setState call that touches other InternalState fields (e.g. paymentOutcome, cardFormState) won't change these booleans/strings, so this is safe in most paths.
However, the retry() callback at R392 calls setState((prev) => ({ ...prev, paymentOutcome: null })). If this setState were ever batched with a state update that also flips isReady or activeMethod (e.g. on session expiry during a retry), the effect cleanup would destroy the manager (m.cleanUp()) while retry() is mid-flight — m.submit() at R393 would then operate on a cleaned-up manager.
This is unlikely in normal flows but could produce a hard-to-diagnose crash on session timeout during retry. Consider adding a guard in the cleanup that skips cleanUp() while a retry is in progress (e.g. via an isRetryingRef).
| const onRetry = () => { | ||
| // Navigate to processing first so the user sees an immediate spinner; | ||
| // the outcome transitioner replaces with success/error when retry resolves. | ||
| replace(CheckoutRoute.processing); | ||
| retry().catch((err) => console.warn(`${LOG} retry error ${fmt(err)}`)); |
There was a problem hiding this comment.
retry() can throw on the m.configure() or m.setRawData() steps (R394-R396 in the provider). Those failures are JS-side errors that never reach the native onError callback, so paymentOutcome is never set to error and the user is stuck on the processing spinner indefinitely.
The .catch() here only logs a warning. Consider navigating back to the error screen (or re-setting paymentOutcome) in the catch handler:
retry().catch((err) => {
console.warn(`${LOG} retry error ${fmt(err)}`);
replace(CheckoutRoute.error, { error: err });
});
Generated by 🚫 Danger Kotlin against fe457d7 |
|
Appetize Android link: https://appetize.io/app/oq65lou2vntonwbnx4mmk6mjeq |
|
Appetize iOS link: https://appetize.io/app/pas7jbw3n54kuft6a4hzzk43b4 |
3ab7101 to
f199509
Compare
ebb678c to
1864c32
Compare
f199509 to
adff445
Compare
1864c32 to
27ab968
Compare
| const SUCCESS_AUTO_DISMISS_MS = 5000; | ||
|
|
||
| function CheckoutLoadingScreen() { | ||
| return <LoadingScreen title="Loading your secure checkout" subtitle="This won't take long" />; |
There was a problem hiding this comment.
I commented on this is earlier, if this is fresher then my other comment can be ignored
adff445 to
3b27d04
Compare
27ab968 to
fe457d7
Compare
3b27d04 to
0793920
Compare
fe457d7 to
b008d32
Compare
0793920 to
cc715c6
Compare
b008d32 to
64939c8
Compare
cc715c6 to
5d922f9
Compare
64939c8 to
3faa0d0
Compare
5d922f9 to
a540c9c
Compare
3faa0d0 to
d9b5077
Compare
a540c9c to
b3db7c1
Compare
d9b5077 to
76a5d85
Compare
b3db7c1 to
0a72258
Compare
76a5d85 to
b968673
Compare
0a72258 to
32541fc
Compare
b968673 to
5aaaff2
Compare
32541fc to
401455f
Compare
60142f0 to
161509c
Compare
b161b04 to
e6dd532
Compare
ab90f37 to
62691a0
Compare
62691a0 to
526ecf5
Compare
Summary
<PrimerCardForm />— drop-in card form composinguseCardForm()+ the four card inputs with an internal focus chain (card → expiry → CVV → name → submit) and submit button. ExposesonSubmitStart,autoFocus,style,testID.paymentOutcomethrough<CheckoutFlow />: success auto-dismisses the sheet after 5s, error lands on the retry screen. HonorsisSuccessScreenEnabled/isErrorScreenEnabledto skip screens when disabled.activeMethod(set by the method-selection screen rather than the view lifecycle). The manager survives the card-form view mount/unmount so retry and post-submit screens keep the same native manager alive.PrimerCheckoutProvider: reconfigure → re-set raw data → submit. The reconfigure is a workaround for ACC-6920 (iOSRawDataManager.swift:237nullifies its delegate on successful tokenization) — aTODO(iOS native fix)marks it.setHeight/resetHeightwith a token-based height-request stack. Each request returns a per-token release so a late cleanup from a prior screen can't clobber a newer screen's active request.NavigationContaineralways render the current screen inside a stableAnimated.Viewwrapper; the outgoing screen is a sibling during transitions. Component instances survive transition start/end — no remount churn, no duplicateuseEffectfires.CardFormScreen(header + scroll +KeyboardAvoidingView) as the drop-in wrapper; standalone merchants render<PrimerCardForm />directly.RawDataManager.configureListeners()idempotent so reconfigure-on-retry doesn't stack duplicate subscriptions or trigger "no listeners registered" warnings.src/Components/internal/debug.tswith a single-linefmt()for console logs.Out of scope (intentional)
RawDataManagerdelegate fix — tracked separately; retry workaround stays until the native change lands.Jira
ACC-6495
Test plan
yarn typecheck— cleanyarn lint— cleanyarn test— 91/91 passing4242 4242 4242 4242) → Pay → processing → success → auto-dismiss after 5s4000 0000 0000 0002) → Pay → processing → error → Retry re-submitsStacked on
Depends on #347 (ACC-6494 input-chain additions). Merge #347 first, then rebase this onto
main.