Skip to content

Stop the Drop-in reloading on every order update - #807

Open
acasazza wants to merge 11 commits into
v5.0.0from
fix/adyen-dropin-reload-partial-authorization
Open

Stop the Drop-in reloading on every order update#807
acasazza wants to merge 11 commits into
v5.0.0from
fix/adyen-dropin-reload-partial-authorization

Conversation

@acasazza

Copy link
Copy Markdown
Member

The bug

An Adyen gift card covering only part of the order left the Drop-in reloading repeatedly ("as if the Adyen component kept reloading"), and a Place Order click could take the page — and the Playwright run — down with:

unhandledRejection: Error: No active payment method.
  at handleSubmit (AdyenPayment.tsx)
  at ref.current.onsubmit
  at handleClick (PlaceOrderButton.tsx)

Root cause

Both parents implement their loader by replacing the subtree rather than overlaying it:

// PaymentMethod.tsx
const content = !loading ? <>{components}</> : getLoaderComponent(loader)
// PaymentGateway.tsx
if (loading) return loaderComponent

To React those are different trees in the same position, so every flip of loading unmounts every gateway below. For a stateless gateway that costs nothing. The Adyen Drop-in, though, owns imperative state — the shopper's selected method and typed-in card details — so it was destroyed and fully re-initialized (fresh AdyenCheckout(), new Dropin().mount(), translations and analytics again) on each flip.

Guarding the individual flips cannot close this: payment_source.payment_response.status and order.payment_status are populated by two different API calls (the gift-card balance check refetches the order; the authorization flips the payment status), so there is a window where a flip looks legitimate.

Dropin.submit() then threw because mount() had reset activePaymentMethod while the patched ref.current.onsubmit survived, leaving <PlaceOrderButton> believing it could submit.

The fix

File Change
PaymentGateway.tsx the adyen_payments branch is no longer swapped out for the loader; isPartiallyAuthorized guards on the loader flips and on recreating the payment source
PaymentMethod.tsx once the methods have rendered they are never swapped back out for the loader
AdyenPayment.tsx one intentional refresh via Core.update(); checkoutRef + latch; remove() on unmount; try/catch around submit(); submit wiring disarmed on refresh

The intentional refresh is kept — refreshing once when the order becomes partially authorized is correct, since the shopper now owes less — but it happens once:

checkoutRef.current?.update({ amount: remainingAmount }, { shouldReinitializeCheckout: true })

Verified against the shipped adyen-web@6.41.0: with true, Core does setOptions(amount)initialize()update() on each mounted component, and BaseElement.update() is state = {} plus unmount().mount(this._node) — a real refresh in place, with the payment method list consistent with what is left to pay. It replaces dropinRef.current.mount("#adyen-dropin"), which re-rendered the Drop-in with the old amount: same lost selection, none of the benefit.

The remaining amount is not gift_card_amount_cents

That field is "the sum of all the gift_cards applied to the order" — Commerce Layer gift_card resources. An Adyen gift card authorized through _authorization_amount_cents is a payment-source authorization and never creates one, so the field stays 0 and total_amount_with_taxes_cents - gift_card_amount_cents silently evaluates to the full total. Adyen's own payment_response.order.remainingAmount is preferred, falling back to total - currentBalance. Reported as { currency, value } and only when the currency is known: triggerAmountUpdate() gates on isAmountValid, which rejects an empty currency with nothing but a console.warn.

Behaviour changes to validate

  • showLoader now means "while first fetching the payment methods", matching its documented description. It no longer re-enters the loading state after the first render.
  • Place Order goes back to disabled right after the gift card is applied, until the shopper picks a method again. This is the logical consequence of a refresh resetting activePaymentMethod, and it prevents the crash at the source rather than only reporting it.

Both touch PaymentMethod/PaymentGateway, which every gateway shares, so the other payments-*.spec.ts suites are worth a run.

Testing

specs/payment_source/AdyenPayment.spec.tsx — 13 tests, mocking @adyen/adyen-web/auto and driving the real onSubmit handler the component installs. The important one renders the real chainPaymentMethodPaymentSourcePaymentGatewayAdyenGatewayAdyenPayment — and pushes through the order updates a partial authorization actually produces, in order, asserting one mount() and no remove().

Each fix was confirmed load-bearing by reverting the source line and watching the test fail:

Reverted Failure
PaymentMethod latch expected 1 times, but got 2 times
PaymentGateway adyen branch expected "remove" to not be called, but was called 1 times
refresh latch expected 1 times, but got 4 times
whole AdyenPayment change reproduces Unhandled Rejection: Error: No active payment method.
  • vitest run — 75 files, 794 tests green
  • biome lint ./src --max-diagnostics=300 — 97 warnings, 0 errors, identical to the baseline on v5.0.0
  • tsc --noEmit — no errors in the changed files (35 pre-existing elsewhere)
  • husky pre-commit hook green (workspace build + lint + suite)
  • payments-adyen-givex.spec.ts passes locally against a linked build

Still open

The loader-replaces-subtree pattern remains for the other gateways, and docs/adr/0001-payment-source-effect-invariants.md should probably gain an invariant for it — happy to add that here or in a follow-up.

🤖 Generated with Claude Code

@acasazza acasazza self-assigned this Jul 31, 2026
@acasazza acasazza added the bug Something isn't working label Jul 31, 2026
@acasazza
acasazza requested review from Copilot, gciotola and malessani and removed request for Copilot July 31, 2026 19:54
@malessani malessani changed the title fix(adyen): stop the Drop-in reloading on every order update Stop the Drop-in reloading on every order update Aug 3, 2026
An Adyen gift card that covers only part of the order left the Drop-in
reloading repeatedly, and a Place Order click could crash the page with
`unhandledRejection: Error: No active payment method.`

Both `<PaymentMethod>` and `<PaymentGateway>` implement their loader by
*replacing* the subtree (`content = !loading ? ... : loader` and
`if (loading) return loaderComponent`), so any flip of `loading` unmounts
every gateway below. For a stateless gateway that costs nothing; the Adyen
Drop-in owns imperative state — the selected method and typed-in details —
so it was destroyed and fully re-initialized each time.

Guarding the individual flips cannot close this: `payment_response.status`
and `payment_status` are populated by two different API calls, so there is
a window where a flip looks legitimate. The gateway is therefore kept
mounted for `adyen_payments`, and the payment methods are never swapped
back out for the loader once rendered. `showLoader` now means "while first
fetching the payment methods", as its documentation says.

The intentional refresh is kept, but happens once: `Core.update({ amount },
{ shouldReinitializeCheckout: true })` with the remaining amount. This
replaces `dropinRef.current.mount("#adyen-dropin")`, which re-rendered the
Drop-in with the *old* amount — losing the selection for no benefit.

The remaining amount is deliberately not derived from
`gift_card_amount_cents`: that field sums the Commerce Layer `gift_card`
resources, and an Adyen gift card authorized through
`_authorization_amount_cents` never creates one, so it stays 0 and the
subtraction would hand back the full total. Adyen's own `remainingAmount`
is preferred, falling back to `total - authorized balance`.

Also:
- `Dropin.remove()` on unmount, clearing `dropinRef`/`checkoutRef`, so a
  remounted component initializes a fresh instance instead of staying wired
  to a destroyed one. Kept in its own mount-scoped effect: the main effect
  re-runs on `status` changes and must not tear the Drop-in down.
- `Dropin.submit()` wrapped in try/catch, routing the failure to
  `setPaymentMethodErrors` instead of an unhandled rejection, and the submit
  wiring disarmed on refresh so `<PlaceOrderButton>` cannot submit an empty
  Drop-in.
- Recreating the payment source is skipped while the order is partially
  authorized: `mismatched_amounts` is true by design in that window, and
  "healing" it would discard the authorization just obtained.

Covered by 13 tests in specs/payment_source/AdyenPayment.spec.tsx,
including one that drives the real
PaymentMethod -> PaymentSource -> PaymentGateway -> AdyenGateway -> AdyenPayment
chain through the order updates a partial authorization produces.
@acasazza
acasazza force-pushed the fix/adyen-dropin-reload-partial-authorization branch from 06c3157 to d05e9e3 Compare August 11, 2026 15:41
@pkg-pr-new

pkg-pr-new Bot commented Aug 11, 2026

Copy link
Copy Markdown
npm i https://pkg.pr.new/commercelayer/commercelayer-react-components/@commercelayer/core-components@807
npm i https://pkg.pr.new/commercelayer/commercelayer-react-components/@commercelayer/react-components@807
npm i https://pkg.pr.new/commercelayer/commercelayer-react-components/@commercelayer/react-hooks-components@807

commit: a7f94d6

Alessandro Casazza added 10 commits August 11, 2026 17:57
Pin brace-expansion, ip-address, js-yaml, nanoid, postcss and undici to
patched ranges via pnpm-workspace overrides, and add the corresponding
minimumReleaseAgeExclude entries so the freshly published patches are not
held back by the release-age gate.

Also bumps iframe-resizer to ^4.4.5 and widens the declared vitest floor to
^4.1.5; vitest still resolves to 4.1.10 in the lockfile.

Lockfile regenerated under pnpm 11, which preserves the workspace overrides.
npm@latest resolved to npm 12, which requires
^22.22.2 || ^24.15.0 || >=26.0.0, so the "Update npm" step failed
with EBADENGINE on Node 20 (EOL since April 2026).

The npm upgrade step is needed for npmjs trusted publishing (OIDC),
so raise the runtime instead of dropping it.
The API clears `shipment.shipping_method` server-side whenever the order
totals change, because shipping method availability depends on them. The
shipments SWR cache is keyed on (accessToken, orderId) alone, with
revalidateOnFocus/Reconnect off, so it never refetched on its own.

Applying a coupon therefore left the cached shipment carrying a shipping
method the order no longer had: `<ShippingMethodRadioButton>` stayed
checked on it, and re-clicking an already-checked radio fires no change
event, so the user was stuck with a disabled save button and no way to
re-select.

Refetch whenever `order.updated_at` moves on, and stamp the revision our
own `setShippingMethod` produces so it does not cause a redundant round
trip (measured: one shipments fetch per selection, unchanged).

Adds six regression tests, each paired with a positive control so a
missing revalidation effect cannot pass them vacuously. Verified against
a mutation matrix: deleting the effect fails all six, and removing the
setShippingMethod stamp or the null-revision guard each fails exactly its
designated test.
Takes every file the coupon/shipping-method bug ran through to 100%
statements, branches and functions, then works outward through the
modules that need no mocking harness.

Shipment chain (100/100/100): Shipment, ShipmentField, Shipments,
ShipmentsContainer, ShipmentsCount, and all five shipping_methods
components. Shipment.tsx had no tests at all, despite owning the
`currentShippingMethodId` derivation that carried the stale selection.

Also to 100%: 22 pure utils (currencies, getErrors, promisify,
filterChildren, customMessages, compareObjAttribute, formCleaner,
sortPaymentMethods, getAmount, the jwt helpers, …), three reducers
(BillingAddress, ShippingAddress, InStockSubscription), the seven
order-amount wrappers plus BaseOrderPrice/BaseField, and
GenericFieldComponent with its Customer/Parcel/ParcelLineItem fields.

One source change: Shipment.tsx destructured
`shipment?.available_shipping_methods || []` behind a guard that had
already proven the array present, so the fallback was unreachable and no
test could cover it. Hoisting the normalisation makes both branches real
without changing behaviour.

Package: 800 -> 1015 tests, 56.11% -> 61.67% statements. 164 of 260
files are now fully covered (statements, branches and functions); 56
remain at 0%.

What is deliberately left: components/payment_source and
payment_gateways (634 statements) wrap third-party SDKs and DOM globals,
and components/orders needs a full provider harness. Those want a mocking
harness designed first, not specs forced to green.
<BraintreePayment> threw as soon as it mounted, leaving checkout stuck on
the skeleton loader for any order whose market offers Braintree:

  Calling `require` for "braintree-web/dist/browser/client.js" in an
  environment that doesn't expose the `require` function.

The component loaded braintree-web with CommonJS `require`. rolldown
resolves those specifiers but cannot turn `require` into a browser
import, so it emitted `__require(...)` against a shim that throws by
design. The previous bundler tolerated the bare require, so this surfaced
with the tsup -> tsdown migration.

Load the three subpaths through dynamic `import()` instead, normalising
the namespace shape (braintree-web is CJS, so `default` holds
module.exports under some bundlers and members are hoisted under others)
and bailing out if the component unmounts mid-load. `__require("braintree`
now appears zero times in both dist bundles.

That crash was also masking an infinite render loop, which appeared as
~500 "Maximum update depth exceeded" errors in five seconds once the
component could mount. Two causes, both in the same effect:

  - `handleSubmitForm` is rebuilt every render and was a dependency, so
    the effect re-ran every render and its cleanup's setState calls
    triggered the next one.
  - `loadBraintree` was both a dependency and reset by that cleanup, so
    the effect tore down and re-created the Braintree client in a cycle.

Fixed with the ref pattern used elsewhere in this codebase: a ref holds
the latest submit closure, and a separate ref guards initialisation, so
`loadBraintree` state only drives rendering.

Verified against a Braintree order in the EU market: page renders, hosted
field iframes mount, zero console errors on a full reload. Not verified:
an actual payment - no card was submitted, so the 3-D Secure and submit
paths are unexercised, and this file has no test coverage.
Brings the workspace up to date ahead of the save-to-address-book fix,
which needs rapid-form v5.

  rapid-form              4.2.0  -> 5.0.0
  @babel/core             7.29.7 -> 8.0.1
  @babel/preset-env       7.29.7 -> 8.0.2
  @commercelayer/js-auth  7.4.2  -> 8.0.0
  iframe-resizer          4.4.5  -> 5.5.9
  jsdom                   29.1.1 -> 30.0.1
  lerna                   9.0.7  -> 10.0.0
  @types/node             25.9.5 -> 26.2.0
  plus biome, swr and @stripe/react-stripe-js patches

Two packages are deliberately held back.

TypeScript stays at 6.0.3: the TS 7 migration already has its own branch
(chore/dependency-upgrades), and folding it in here would mix unrelated
fallout into this diff.

@tanstack/react-table stays at 8.21.3. v9 is an API migration rather than
a version bump - useReactTable -> useTable, getCoreRowModel ->
createCoreRowModel, getPaginationRowModel -> createPaginatedRowModel,
changed ColumnDef generics - and it produced 16 type errors in
OrderList.tsx and OrderListRow.tsx. Note the build did NOT fail on those:
rolldown only transpiles, so this would have shipped as a runtime crash
rather than a build error. components/orders has no test coverage and
OrderList is not reachable from the checkout flow, so the migration is
unverifiable from here and belongs in its own change.

rapid-form v5 widens a tracked field's `value` to `string | string[]`,
which broke four call sites in AddressStateSelector. Adds
`singleFormValue()` to collapse the union back to a string.

Typecheck holds at its measured baseline of 21 pre-existing src errors
(the 43 figure in my notes was stale); lint holds at 97 warnings.
Ticking "Save this address in your account" did nothing at all. The
checkbox state was never written anywhere, so the preference was silently
discarded and the box came back unticked on reopening the Customer step -
the unticked box was reporting the truth. Proven by instrumenting
localStorage.setItem: zero writes across tick, save and reopen, even when
a required field was also edited to force a form sync.

The only code path that recorded it iterated rapid-form's tracked values
looking for `field.type === "checkbox"`. rapid-form only tracks
required/validated fields, and this checkbox renders with
`required={false}`, so it never appeared and the branch was unreachable.

rapid-form v5 adds `trackUnvalidatedFields`, but that alone is not
enough. v5 reports every tracked field as `{ name, value }` only - no
`type`, no `checked` - and encodes a checkbox as the string
`String(el.checked)`, i.e. "true"/"false". So:

  - enable trackUnvalidatedFields so the checkbox is reported at all
  - identify it by its known field name, since a v5 checkbox is
    indistinguishable from a text field by shape; the DOM element and the
    legacy `type` remain as secondary checks for any other checkbox
  - read checked state from the live element first, then `field.checked`,
    then the "true"/"false" string
  - keep every checkbox out of `addressValues`. This one is new with v5:
    "false" is a truthy string, so without the guard it would be PATCHed
    onto the address as a bogus `save_to_customer_book` attribute

Also fixes the restore path, which called setAttribute("checked", "true").
That only seeds `defaultChecked` and leaves a live input visually
unticked; assign the property instead.

The two existing tests for this behaviour passed before this change and
failed after, because they mocked the v4 shape ({ value: "on", type:
"checkbox", checked: true }) that v5 never produces - they were pinning a
fiction while the feature was broken in production. Rewritten around the
real v5 shape, plus a case asserting the checkbox stays out of the
address attributes.

Not verified end to end: the test order's access token expired before the
browser round-trip could be re-run, and placing an order (where the
_save_billing_address_to_customer_address_book trigger actually fires) is
irreversible.
Typing a card number rebuilt the Checkout.com Flow component, wiping
whatever had been entered. Self-feeding cycle:

  1. the card becomes valid, so `onChange` calls `setPaymentRef({ ref })`
  2. that context update re-renders the consumer
  3. mfe-checkout's PaymentContainer builds its gateway config as an
     inline object literal, so `options.appearance` gets a fresh identity
     on every render
  4. `options?.appearance` and `order?.payment_source` were both in the
     mounting effect's dependency array as object identities, so the
     effect re-ran, called loadFlow() again and remounted the Flow

Keep only primitives in the dependency array - loaded, payment_source id,
accessToken, language_code - and read the config object and the context
setters through refs assigned on each render. Adds a mountedForRef guard
so the same payment source cannot mount two Flows even if the effect is
re-entered.

Biome's useExhaustiveDependencies wants the payment_source object back;
that object identity is the bug, so it is suppressed with the reasoning
inline.

Same root cause as the Braintree render loop (a155484) and the Adyen
drop-in reload this branch is named for: a volatile identity in a
gateway's mounting effect. Confirmed fixed in the browser by the reporter.
Not covered by tests - components/payment_source is at 0%.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants