Conversation
acasazza
commented
Mar 27, 2025
Member
- Create new core package
- Add configuration and getPrices function
- Add getPrices tests, biomejs, and global vitest config
- Add new documentation folder
- Add new getAccessToken function. Resolve Create getAccessToken function #617
- Fix vite types env
❌ Deploy Preview for commercelayer-react-components failed.
|
Create usePrices hook
getPayMethods was defined as an inline async function inside the component body, making it a new reference on every render. Because it was listed as a useEffect dependency, the effect re-ran on every render. While state.paymentMethods was still unset (between async dispatches true and getPaymentMethods was called again — infinite loop. - Remove the getPayMethods wrapper function entirely - Call getPaymentMethods directly inside the effect - Remove getPayMethods from the dependency array
fetchOrder(state.order) was called inside useMemo, which runs during the render phase. When fetchOrder updated external state (e.g. AppProvider), React logged: 'Cannot update a component while rendering a different component'. - Move the fetchOrder call into a useEffect with [fetchOrder, state.order] deps - Remove fetchOrder from useMemo deps (it is no longer referenced inside it)
Introduces usePaymentMethod hook that encapsulates the reducer, includes setup, and payment method fetching currently owned by PaymentMethodsContainer. PaymentMethod now works in two modes: - Standalone (no container parent): calls usePaymentMethod internally and wraps its children with PaymentMethodContext.Provider — no container needed. Pass gateway config via the new config prop on PaymentMethod. - Container mode (existing): detects the parent PaymentMethodsContainer via the new _isProvided marker on PaymentMethodContext and uses it as before. PaymentMethodsContainer is kept for backwards compatibility with a @deprecated JSDoc notice; it will be removed in the next major version. Closes #795 (partial — payment_gateways and payment_source still pending)
PaymentMethodsContainer (backward compat): - renders children, provides _isProvided in context - populates paymentMethods from order, calls addResourceToInclude PaymentMethod standalone mode: - detects standalone via _isProvided absence, provides context to children - populates paymentMethods, calls addResourceToInclude on mount - skips addResourceToInclude when includes already loaded - renders one div per payment method after effects settle PaymentMethod container mode: - reads paymentMethods from parent container context - standalone hook does not call addResourceToInclude when isStandalone=false - _isProvided preserved through the tree Regression: does not trigger 'Maximum update depth exceeded'
…ner and usePaymentMethod - Refactor PaymentMethod.tsx: replace module-level loadingResource variable with useRef to properly scope per component instance - Add comprehensive tests covering all uncovered branches and statements: expressPayments flow, autoSelect paths, clickableContainer, hide/sortBy props, showLoader effects, config prop, getOrder fallback call - Mock @commercelayer/core with importOriginal to preserve existing exports - Add MockPaymentMethodProvider helper for isolated effect testing - PaymentMethod.tsx: 100% statements, 99.21% branches - PaymentMethodsContainer.tsx: 100% statements, 100% branches - usePaymentMethod.ts: 100% statements, 100% branches
- Remove react-doctor.yml CI workflow - Fix pgk-pr-new.yaml: replace 'pnpm build' (no root script) with explicit filter for the three published packages
…rms not accepted The second useEffect in PlaceOrderButton called setNotPermitted(false) unconditionally when paymentMethodErrors/errors cleared, ignoring isPermitted. This meant filling in payment data (which clears prior errors) would enable the button even if the PrivacyAndTermsCheckbox was not checked. Fix: guard the re-enable path with isPermitted so clearing errors only enables the button when all other conditions (privacy/terms accepted, billing/shipping address, etc.) are also satisfied.
…rs clear The previous fix introduced a regression: the error-clearing useEffect called setNotPermitted(false) whenever isPermitted=true, but isPermitted only checks if payment_method.id is set — it does not check whether the payment source (card details) is actually filled. This caused the button to enable as soon as errors cleared, even without card details entered. Fix: introduce a hasBlockingErrors state. The error effect only manages this flag (and loading/forceDisable side-effects). The first payment effect now reacts to hasBlockingErrors changes — so when errors clear, it re-runs its full check including the card.brand and onsubmit conditions before enabling the button.
…tainer to prevent infinite re-render loop
Inline functions inside useMemo get a new reference on every recompute.
Payment forms (StripePaymentForm, BraintreePayment, KlarnaPayment, etc.)
include setPaymentRef in their useEffect deps — when this changes on every
render the effect re-runs, calling setPaymentRef again, which mutates state,
which triggers useMemo to recompute, causing an infinite loop.
Fix: extract setLoading, setPaymentRef and setPaymentMethodErrors as stable
useCallback(() => {}, []) callbacks. dispatch from useReducer is guaranteed
stable so empty deps are correct. This matches the pattern already used in
usePaymentMethod (standalone mode), where setPaymentRefCallback is useCallback.
ExternalPayment.tsx had an explicit biome-ignore to work around this bug;
StripePaymentForm, BraintreePayment, KlarnaPayment and CheckoutComPayment
did not and were all susceptible to the infinite loop in container mode.
The status useEffect had a default case calling setNotPermitted(false) on every mount, running after the payment check effect and unconditionally enabling the button regardless of payment state. Root cause: dep was [status != null] — always true, so the effect only ran once on mount. With status='standby' (initial state) it hit the default case, overrode the payment check, and left the button enabled. For orders without a payment method, isPermitted stayed false and the payment check effect never re-ran to correct this. Fix: remove the default case — the payment check effect is the sole authority for enabling the button. Change dep to [status] so the effect re-runs on actual status changes (e.g. 'standby' -> 'disabled'). Also update the handleClick tests: they previously relied on the buggy status effect to enable the button for clicking. Now they correctly mock getCardDetails to return a brand (matching real user state), add paymentType to contexts that were missing it, and align currentPaymentMethodType in Providers with the paymentType under test.
BaseSelect used an uncontrolled <select> (defaultValue) which only applied on the initial mount. When the value prop changed externally (e.g. from a country to empty), React had no mechanism to update the DOM element and the old selection persisted. Switch BaseSelect to a controlled component: maintain localValue state synced via useEffect([value]), and expose onChange to callers. This ensures the select always reflects the current value prop. Also remove the truthy guard in AddressCountrySelector's useEffect so setValue is called even when value is falsy, resetting the form element to the placeholder in the browser/form scenario.
Calling setValue(name, '') unconditionally on mount would cause rapid-form to process the empty required field immediately, potentially triggering premature validation errors. The controlled BaseSelect already handles the visual reset via localValue state — no manual setValue needed for the placeholder case.
When order.billing_address?.country_code is null (SDK returns null for
unset fields), the destructuring default '' does not apply (it only
applies for undefined). This caused localValue=null which renders
<select value={null}>, potentially showing the first option instead of
the placeholder.
Apply nullish coalescing (value ?? '') on both useState initializer and
useEffect sync so null is treated identically to undefined: both result
in the placeholder being selected.
…ween null/undefined The useEffect approach had two problems: 1. When value prop changed from null to undefined (or vice versa), the effect fired and reset the user's selection to '' — both are the same semantic 'no value' but have different JS identities. 2. The effect ran after the commit phase, causing a one-frame flicker where the wrong option was briefly visible. Switch to the React derived-state pattern: update localValue synchronously during render when safeValue (value ?? '') differs from the last synced value. This ensures: - null/undefined oscillation does NOT reset the user's own selection - value=null -> value='IT' (order pre-fill) correctly shows Italy - value='IT' -> value='' (reset) correctly shows placeholder - User selecting a country while value stays null is preserved
Switch from controlled select (value + useState derived-state) back to an uncontrolled approach (defaultValue) combined with useLayoutEffect to imperatively update the DOM when the external value prop genuinely changes. The controlled approach caused the selected label to not update when the user changed the option while the parent held a stable pre-filled value prop — React would revert the DOM back to localValue on every render cycle before the user's pick could be reflected, or browser-native autofill/form behaviour conflicted with the React-managed value. With the uncontrolled approach: - defaultValue sets the correct initial option (placeholder or pre-fill). - useLayoutEffect fires before paint and sets select.value imperatively only when the external safeValue actually changes, so user-driven changes are never silently reset. - null/undefined are normalised to "" so the placeholder option is always visible when no value is provided by the SDK.
The PaymentGateway effect fires setPaymentSource fire-and-forget; before the create request settles, paymentSource/order.payment_source are still null, so any dependency change re-runs the effect and fires a second create — producing two payment source records (observed on Stripe / single payment method). Add two complementary guards: - an in-flight useRef guard in the PaymentGateway effect so it can't re-enter before the request resolves; - request coalescing in setPaymentSource via a module-level Map keyed by order id + resource + operation path, protecting all gateways. Add reducer coalescing specs and a PaymentGateway integration spec. Document the domain language (CONTEXT.md) and the coalescing rationale (ADR 0001). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tsup builds without type-checking and CI gates only on biome + vitest, so type errors ship silently. Running tsc against the base tsconfig is misleading — its Node module resolution breaks @commercelayer/sdk/bundle subpath imports and it pulls in all specs, inflating the error count. Add tsconfig.typecheck.json mirroring the tsup build (Bundler resolution, node types, scoped to src) and a `typecheck` script, giving an accurate picture of type health for the shipped library. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The pre-commit hook runs `pnpm build`, but the workspace root had no `build` script, so every root commit failed at that step. Add a root `build` script that builds the library packages (core, hooks, react-components) in dependency order, mirroring the existing `build:watch` alias. Hook wording is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The test fixtures in extender.ts read env vars (VITE_SALES_CHANNEL_*, VITE_INTEGRATION_*) that don't exist in the root .env, so authentication always failed and every integration test silently passed via its token-null early return, while still making a doomed network request that could exceed the 5s timeout. - read the VITE_TEST_* env vars that .env actually defines - skip visibly via skipIf when credentials are missing (no network call) - await use() in fixtures - raise testTimeout to 15s since these tests hit the live API (~3s each) - getPrices/getSkus specs: replace SKU code DIGITALPRODUCT (absent from the test org) with TSHIRTMM000000E63E74MXXX and assert an exact match
- remove the legacy examples/ directory - refine standalone payment gateways, methods, and sources in react-components along with their specs and mock handlers - align core, hooks, docs, and document packages (import sorting, story and test config updates)
rapid-form attaches input listeners only to the elements present (and required) when refValidation runs, and its MutationObserver only prunes removed fields. A field mounted in a descendant-only render (e.g. mfe-checkout mounts address fields after its async settings fetch) or a field that becomes required after mount was never wired: user changes updated the DOM but never reached form values, permanently, since nothing re-rendered the form component to re-wire it. This is the intermittent dead billing country select seen in mfe-checkout e2e runs. useFormWiring wraps refValidation with a MutationObserver that re-runs it when named fields are added or a required attribute changes. Re-running is idempotent (rapid-form skips already-wired elements). Applied to useAddressFormFields, CustomerAddressForm and GiftCardOrCouponForm.
The v5 refactor dropped the props spread on the BaseInput branch, so data-testid, disabled, aria-* etc. disappeared whenever the state field rendered as a text input (country without predefined states) while the select branch kept them. Restores main's behavior of forwarding passthrough props on both branches, so they survive the select/input swap.
…es late-mounted fields upstream
… dist tsup silently ignored the babelOptions config, so babel-plugin-react-compiler was never actually applied to any published artifact. tsdown (rolldown) wires it through @rolldown/plugin-babel + reactCompilerPreset with target 19, and a post-build check-compiler guard now fails the build if the compiler output ever goes missing again. - hooks: react peer range tightened to >=19.0.0 (compiled output imports the built-in react/compiler-runtime) - react-components: platform browser keeps the require-interop shim browser-safe; gains check-exports (attw) and a ci script like its siblings - drop prop-types: its validators were dead code only used to derive one internal type, and its untyped import broke the rolldown dts build
build: replace tsup with tsdown and apply React Compiler to published dist
Non-breaking (semver minor/patch) refresh via npm-check-updates across all workspace packages. Notable: @commercelayer/sdk 7.11->7.12.1, @stripe/* , rapid-form 4.0.3->4.0.4, swr, msw 2.14->2.15, react 19.2.6->19.2.7, tsdown, storybook 10.3->10.5, @biomejs/biome 2.4->2.5. pnpm/iframe-resizer held back per .ncurc.cjs. pnpm auto-recorded minimumReleaseAgeExclude entries for the freshest versions (storybook 10.5.3, tsdown 0.22.12, verkit 0.1.0). The biome 2.5 bump newly enforces a11y/useKeyWithClickEvents; suppressed on the deliberate action-only anchor in AddressField (consistent with its existing a11y ignores). Verified: build (all libs + React Compiler guard) and core/hooks/rc test suites (129/99/766 pass).
…e-components Refactor payment methods: standalone components without deprecated containers
…ents Rename two v5 workspace packages, both folder and npm package name: - packages/core -> packages/core-components (@commercelayer/core-components) - packages/hooks -> packages/react-hooks-components (@commercelayer/react-hooks-components) Folders moved with git mv to preserve history. Updated: package.json name fields, workspace:* cross-deps, ~81 imports across react-components (src + specs) and hooks src, root build/build:watch filters, react-components vitest aliases (incl. #sdk/#types and _vitest.config.mts), pkg-pr-new.yaml publish paths, and packages/document doc content. Regenerated pnpm-lock.yaml. Packages are unpublished, so no external consumers are affected. Verified locally: pnpm build (all libs + React Compiler guard) and pnpm test (core 129 / hooks 99 / react-components 766, all pass); frozen-lockfile install consistent. Closes #801
…ckages chore: rename core → core-components and hooks → react-hooks-components
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
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.