Skip to content

fix(ep-commerce): provide FormProvider from EP Product Provider - #409

Open
mzaintariq wants to merge 1 commit into
masterfrom
fix/ep-product-provider-form-provider
Open

fix(ep-commerce): provide FormProvider from EP Product Provider#409
mzaintariq wants to merge 1 commit into
masterfrom
fix/ep-product-provider-form-provider

Conversation

@mzaintariq

Copy link
Copy Markdown
Collaborator

What does this MR do?

Mounts a react-hook-form FormProvider inside EPProductProvider, matching commerce Product Box’s contract. Without it, descendants that write selection via form context (SelectedLocationSlug, variants, quantity for ATC) silently no-op when the PDP uses EP Product Provider instead of Product Box.

Changes

  • Wrap provider children in FormProvider from useForm()
  • Reset form state when the product id changes
  • Doc comment clarifying parity with commerce ProductProvider

Design decisions

  • EP Product Provider is the SSR-friendly product context; location picker / ATC already assume form context (“must be inside a Product Box”)
  • Product Box already provides FormProvider; this closes the gap for compositions that only use EP Product Provider
  • Cart drawer qty/remove do not depend on this — they use cart line data, not PDP form state

Testing

  • Manual: PDP with EP Product Provider + location radios — selecting a location updates selection / URL and ATC receives location
  • No change expected for Product Box–based PDPs (nested form providers remain the existing composition)

Reviewer notes

  • Independent of cart drawer designer-error work; no dependency on that PR
  • Follow-up found while testing multilocation ATC → cart; not required for cart-error DataProviders
  • Consider a small unit/integration test later if we want FormProvider presence covered in CI

Match commerce Product Box so location/variant/quantity form state is shared with Add To Cart; without it, EPLocationPicker clicks are no-ops.

@field123 field123 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct premise — there's no FormProvider anywhere in elastic-path today, so the PDP → ATC wiring really is dead. Keep it.

The reset is the problem (inline). Also worth adding the test now, not as a follow-up: the infra exists in registerEPAddToCartButton.test.tsx, and a child that setValues in a mount effect would catch this.

const formMethods = useForm();
useEffect(() => {
formMethods.reset();
}, [dataProduct?.id, formMethods]);

@field123 field123 Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reset() fires on mount, and child effects run before parent effects. EPStockProvider.tsx:232-243 sets SelectedLocationSlug from ?location= with deps [form] (stable ref in RHF 7.46.2), so it runs once — then this wipes it and never re-runs. Deep-linked location is lost and ATC posts with no locationId.

Suggest dropping the effect and mirroring commerce (contexts.tsx:26-32) — fresh form at render, before any child effect:

function ProductFormScope({ children }: { children: React.ReactNode }) {
  const methods = useForm();
  return <FormProvider {...methods}>{children}</FormProvider>;
}

// <ProductFormScope key={dataProduct?.id}>…</ProductFormScope>

<div className={className} data-ep-product-provider="">
{content}
</div>
<FormProvider {...formMethods}>

@field123 field123 Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"No change for Product Box–based PDPs" isn't established — compositions where form writers straddle this boundary (commerce Product Quantity outside, EP ATC inside) now split across two form contexts. Worth checking the real storefront pages before merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants