Accumulate state.include instead of replacing it - #805
Merged
Conversation
`state.include` is the union of the resources every mounted component
needs, but `setIncludesResource` fell through to `baseReducer`, whose
plain `{ ...state, ...payload }` spread replaced the list wholesale.
`addResourceToInclude` is a free function with no store access, so it can
only union against the `resourcesIncluded` it is handed — and most of the
26 call sites don't pass it. Components that dispatch more than once from
a single effect pass all read the same stale snapshot, so the last
dispatch dropped the others' resources. In `PlaceOrderContainer` that
lost five `shipments.*` resources plus `billing_address`, putting only
`["line_items.item", "shipping_address"]` on the wire.
This matters more than a missing field: the API cannot distinguish "I did
not request this relationship" from "this relationship is unset", so a
dropped `shipments.shipping_method` makes a selected shipping method look
unselected rather than failing the request.
Handle the action in `orderReducer` and union there, so every call site is
correct whether or not it passes `resourcesIncluded`. Two behaviours are
preserved deliberately: an explicitly empty list still resets, because the
effect teardown in `useOrderState` relies on it; and `include` is left
untouched when there is nothing to merge, since turning `undefined` into
`[]` would flip the `include?.length === 0` checks in that same hook.
gciotola
approved these changes
Jul 29, 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
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.
state.includeis the union of the resources every mounted component needs, but it was being replaced rather than accumulated — so components silently lost each other's includes.Why this is worse than a missing field
Commerce Layer only embeds relationships you explicitly request, and the response cannot distinguish the two cases:
A dropped include therefore does not fail the request — it produces wrong application state. Losing
shipments.shipping_methodmakes a selected shipping method look unselected.Mechanism
setIncludesResourcefell through tobaseReducer, whose{ ...state, ...payload }spread replacesincludewholesale.addResourceToIncludeis a free function with no store access, so it can only union against theresourcesIncludedit is handed — and most of the 26 call sites don't pass it. Components that dispatch more than once from a single effect pass all read the same pre-update snapshot, so the last dispatch wins and the others' resources are gone.PlaceOrderContainerdoes exactly this — three dispatches in one pass, only the third passingresourcesIncluded:resourcesIncluded?shipments.*incl.shipments.shipping_methodbilling_addressshipping_addressThe effect then re-runs, still sees
shipmentsabsent, and repeats the same self-defeating dance.Confirmed on the wire
OrderReducer.includeRequest.spec.tsbuilds state through that exact dispatch sequence and asserts whatgetApiOrderpasses tosdk.orders.retrieve. Against the old reducer:The request was
include: ['line_items.item', 'shipping_address']—shipments.shipping_methodandbilling_addressgenuinely absent. Not inferred from reading the code.Fix
Handle the action in
orderReducerand union there, so all 26 call sites are correct whether or not they passresourcesIncluded. Fixing it here rather than in each caller is why this belongs in react-components instead of being worked around downstream.Two behaviours are preserved deliberately:
useOrderStatedispatchesinclude: []to clear the list, and a naive union would silently make that a no-op.includeis left untouched when there is nothing to merge — callers that only reportincludeLoadedomit it, and turningundefinedinto[]would flip theinclude?.length === 0checks in that same hook and change its effect dependencies.Tests
Both new specs were verified to fail against the old reducer before being accepted (3 of 6, and 1 of 2), so they are regression tests rather than tautologies.
Notes
v5.0.0;mainis the v4 line.pnpm typecheckreports 35 errors on this branch. They are byte-identical to the pre-existing baseline and unrelated to this change; clearing them is part of the separate dependency-upgrade work.🤖 Generated with Claude Code