Conversation
acasazza
added
the
breaking-change
✅ Deploy Preview for commercelayer-react-components ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Implement SWR into hooks
… into the core package
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%.
Replace the legacy `iframe-resizer` package and its `@types/iframe-resizer` stub with `@iframe-resizer/parent`, which ships its own types and exports the resizer as a default export. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…laced Two ways a partial gift card payment ended up authorized against a payment source the order no longer points at, both leaving the shopper on an order that shows no gift card amount at all. Adyen's session, including the `order_data` that expires after a minute, is baked into the Drop-in at creation. When the API rejects a call because it expired, the reducer destroys the payment source and a fresh one takes its place, but the instance on screen kept talking to the dead session: `checkout` is set once at initialization and never cleared, which latches the init branch shut for the rest of the component's life. It is now rebuilt — gated on an explicit expiry signal rather than on the payment source id, because <PaymentGateway> also creates a source with a new id whenever the amount is mismatched, and rebuilding there is the reload loop d05e9e3 fixed. The Drop-in also installs its `onSubmit` once, so it closed over the payment source from the render that built it. <PaymentGateway> recreates the source whenever the order carries more than one payment method, and when that landed between the build and the shopper's click the submit redeemed the gift card at Adyen against an orphan: the balance check returned 5000 on the old source while the order already pointed at the new one, so `_authorize` came back with `gift_card_amount_cents: 0`. The id now comes from a latest-value ref, captured once per submit rather than per call — the expiry path deliberately reuses the id the reducer has just destroyed. An expired session was also reported to the shopper as "The gift card has no balance. Please use a different one.", because a failed request and an empty card both arrived as `undefined?.balance ?? 0`. The two are now distinguished. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rtial-authorization Stop the Drop-in reloading on every order update
Registra i 42 commit rimasti solo su main (fix v4.29.x, security remediation, CI pnpm 10) come mergiati, mantenendo integralmente l'albero della v5.0.0: il codice e' stato riscritto da zero e nessuna porzione della v4 deve rientrare nella v5.
packages/docs was the pre-v5 documentation, kept around only as a sample of the old structure; its stories target the v4 API and will be rewritten in packages/document. It also dragged in a three-headed Storybook dependency set (7.6.24, 8.6.18, 9.0.8 alongside 10.5.3), which was the only remaining consumer of lodash in the workspace: @storybook/addons/api/client-api@7 pulled manager-api and preview-api@7.6.24, and @storybook/addon-interactions@8 pulled @storybook/test -> @testing-library/jest-dom@6.5.0. lodash is now gone from the lockfile entirely. - drop the dead docs:dev / docs:build scripts (document:dev / document:build already exist) - point the gh-pages workflow at packages/document/storybook-static - lockfile regenerated with pnpm 10, -2370 lines
Removing packages/docs took away the last consumers of lodash (the Storybook 7.6.24 manager-api/preview-api chain and @testing-library/jest-dom@6.5.0), so the '>=4.17.24' override no longer resolves anything: regenerating the lockfile changes exactly one line, the override mirror itself, with no package version moving.
Merge main into v5.0.0 and drop the legacy docs package
acasazza
marked this pull request as ready for review
August 21, 2026 09:05
The old packages/docs was the v4 documentation and was deleted in 213b4d3; this is the v5 storybook, which had been living under the placeholder name 'document' only to avoid colliding with it. Now that the collision is gone, it takes the name that describes what it is. Renamed the directory, the package name, the root docs:dev / docs:build scripts, the gh-pages artifact path and the storybook MCP server filter in .mcp.json.
…hain The Netlify deploy has failed on every build since June, and deleting packages/docs in 213b4d3 made it unrecoverable: whatever the dashboard build command referenced is gone. Nobody on the team has Netlify credentials, so the fix has to come from the repo — which netlify.toml can do, since it takes precedence over the dashboard. Two likely causes are addressed at once: - pnpm. Overrides live in pnpm-workspace.yaml, which only pnpm 10+ reads; older versions reject the lockfile with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH. Netlify has no PNPM_VERSION variable — the only lever is the root packageManager field via Corepack, which was missing entirely, so Netlify was running whatever pnpm its build image ships. Pinned to 11.22.0, and the inline 'version: 10' is dropped from the three workflows so there is a single source of truth (having both has already produced 'Multiple versions of pnpm specified' on this repo). - build target. command and publish now point at the renamed docs package. base is pinned to '/' to neutralise any stale base directory still set in the dashboard, which would abort the build before this file is read. Node is aligned to 24.x everywhere — it was 20.x in gh-pages and 22.x in the other two — matching the version the suite is developed and tested on locally.
Reverts the trigger removal from e84de92, restoring the original block verbatim, comments included. GitHub Pages is configured with build_type 'workflow' and is already serving commercelayer.github.io/commercelayer-react-components, so the deploy job has a valid target. Note the trigger watches 'main' only, as it originally did: it will stay dormant until v5 lands there. No VITE_BASE_URL is set. packages/docs/.storybook/main.ts reads one, but with it unset Vite emits relative asset paths (./assets, ./sb-manager), which resolve correctly both under the Pages repository subpath and at the Netlify domain root. Verified by inspecting index.html and iframe.html from both builds.
Fix the Netlify build from the repo and rename document to docs
The Netlify deploy failed with 'Rolldown failed to resolve import @commercelayer/core-components from packages/react-components/src/hooks/useCommerceLayer.ts'. The storybook's Vite config aliases @commercelayer/react-components straight to its source, so it looked like no prior build was needed. But that source imports @commercelayer/core-components, a workspace package whose entry point is ./dist/index.js — and nothing aliases it. On a clean checkout dist/ does not exist, so resolution fails. It passed locally only because dist/ was left over from earlier builds; deleting the three dist/ directories reproduces the Netlify error exactly, and pnpm build makes it pass again. Not a publishing problem: the dependency is declared as workspace:*, so pnpm links the local package and never consults the npm registry. The gh-pages workflow had the same latent bug — it also ran docs:build alone and would have failed identically on a clean runner.
…e-deps Build the libraries before the storybook
gciotola
approved these changes
Aug 21, 2026
This was referenced Aug 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The v5 release branch. The library was rewritten from scratch rather than evolved, so this PR replaces the v4 codebase wholesale — 733 files, roughly 48k lines each way. All three packages are at
5.0.0-beta.1.What changes for consumers
Containers give way to standalone components. The v4 pattern of wrapping everything in a
*Containerthat owned a reducer is gone. Each domain now exposes a component you can use directly, with its state in a hook and its logic in a framework-free core function. This was done domain by domain: prices (#752), skus and sku lists (#758, #720, #731), availability (#761, #745), orders (#766, #764), line items (#776, #777), gift cards (#779), customers and addresses (#784, #786), shipments (#789), payment methods (#796), in-stock subscriptions (#792). The old containers are deprecated, not silently removed.Three packages instead of one.
@commercelayer/react-componentskeeps the React surface;@commercelayer/core-componentsholds the framework-agnostic logic;@commercelayer/react-hooks-componentsholds the hooks. Renamed to those names in #802, and published on pkg-pr-new per commit (#794).Data fetching moved to SWR (#701), with
interceptorsnow accepted by the SDK hooks and theCommerceLayercomponent (#750), andstate.includeaccumulated rather than overwritten (#805).SDK v8 (#806). Also:
jwt-decodedropped (#627), lodash dropped (#743).Toolchain
Build is tsdown with the React Compiler applied and a guard that fails the build if compiler output is missing. Lint and format are Biome, tests are Vitest, docs are Storybook 10 in
packages/docs. pnpm is pinned to11.22.0throughpackageManager, Node to24.x; the Netlify build lives innetlify.tomlrather than the dashboard.Relationship with
mainmain(v4.29.x) has been merged in via #814 using-s ours: the merge records the 42 commits that only existed onmain— the v4 fixes, the #775/#798 security remediations, the pnpm 10 CI bump — while keeping the v5 tree untouched, because their v5 equivalents already exist and no v4 code should re-enter. That merge is what took this PR fromCONFLICTINGto a clean fast-forward.State
All checks green, including the Netlify deploy preview, which had been failing on every build since June and is now serving the storybook again (67 stories). Blocked only on review.
Merge this with a merge commit, not squash or rebase — #814 carries
origin/mainas its second parent, and flattening the history would drop it.