fix(DST-1717): expose dependencies on the components that own their collection

[sebald]

Description

React Aria collections cache each rendered item against the item object. A render function that reads anything else — a label from state, a lookup by id — keeps rendering the value it first saw. The escape hatch is dependencies, listed like a hook's dependency array, but it only exists on the collection components themselves.

Select, ComboBox, TagGroup, TagField and Autocomplete own their collection internally, so there was no way to reach it: the option or tag went stale and stayed stale, with no error and no warning.

All five now accept dependencies and forward it to every collection they render — the popover copy and the tray/dropdown copy are separate collections and each needs it.

<Select items={people} dependencies={[shift]}>
  {person => (
    <Select.Option id={person.id}>
      {person.name} — {shift}
    </Select.Option>
  )}
</Select>

Closes DST-1717

Split out of #5727

This rode along on the Table expandable-rows branch (DST-1702), where Table.Body gets dependencies from RAC for free — which is what made the gap in these five obvious. It is unrelated to that feature, so it ships on its own: the collection fix doesn't wait on a Chromatic run for the Table, and both diffs stay reviewable. Raised in review at #discussion_r3776037196.

One extra fix the tests forced out

Autocomplete shipped dependencies that you could not actually use. Its children inherited RAC's ChildrenOrFunction<ComboBoxRenderProps> — render props, not an item function — so items plus a render function only type-checked through a cast, and that pattern is the entire reason dependencies exists.

AutocompleteProps now takes an optional item type and declares the render function on children, the same shape TagFieldProps already uses:

export interface AutocompleteProps<T extends object = object>
  extends Omit<RAC.ComboBoxProps<T>, RemovedProps> {
  children?: ReactNode | ((item: T) => ReactNode);
}

AutocompleteBase becomes a generic function so T is inferred from items, mirroring TagFieldBase. Type-level only — no runtime change, and existing uses (static <Autocomplete.Option> children) are untouched. It deliberately does not copy ComboBox's (item: any), which is only any because that interface doesn't omit children before redeclaring it.

Screenshots / Preview

No visual change — dependencies alters when an item re-renders, not how it looks. All WithDependencies stories are snapshot-disabled.

Test Instructions

Each component gets a WithDependencies story plus a .test() that proves the stale render goes away: render options from items plus a function that reads a shift value from state, flip the state, assert the option text followed.

The tray path is now covered too. The first round of tests all drove the popover, which left the second forward per component — the half that only fails on touch — unproven. Select, ComboBox, TagField and Autocomplete each add a WithDependenciesMobile story (viewport: smallScreen) whose test opens the tray and asserts the same thing.

Verified load-bearing by mutation rather than trusted green. Dropping dependencies from Select's tray ListBox:

× Select > With Dependencies Mobile > Re-renders the tray options when a listed dependency changes
✓ Select > With Dependencies        > Re-renders the options when a listed dependency changes

Only the mobile test fails — which is precisely the blind spot this PR was flagged for.

Two things the tray tests needed that the popover ones did not, both worth knowing if you write more:

• the trigger is matched by role, because the tray title repeats the field label and getByLabelText then finds two nodes;
• closing the tray has to be awaited — the underlay swallows clicks until it unmounts, so a click on the control behind it silently does nothing. This one masqueraded as a stale render.

1. pnpm sb → each component's With Dependencies / With Dependencies Mobile story → press Switch shift and re-open the list; the option label should follow.
2. Automated: pnpm typecheck:only; pnpm test:sb --run Select ComboBox TagGroup TagField Autocomplete (125 passing).

Worth a reviewer's eye

TagGroup is the one component that gets no second test, deliberately. Its collapseAt branch does render a second TagList, but canCollapse is collapseAt !== undefined && typeof children !== 'function' — so a render function can never reach that branch, and with static children there is no per-item cache to go stale. Its dependencies forward is therefore inert today; it is kept, with a comment, so the two branches stay in step if collapse ever learns to handle items. Happy to drop it instead if you'd rather not carry unreachable wiring.

[changeset-bot]

🦋 Changeset detected

Latest commit: 2f617c9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

Name	Type
@marigold/components	Minor
@marigold/docs	Minor
@marigold/cli	Major
@marigold/theme-rui	Patch
@marigold/system	Minor
@marigold/icons	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
marigold-docs	Ready	Preview	Aug 21, 2026 11:57am
marigold-storybook	Ready	Preview	Aug 21, 2026 11:57am

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
marigold-production	Ignored		Aug 21, 2026 11:57am

[github-actions]

Accessibility tests executed. Download the report here.

[github-actions]

Coverage Report for Marigold Code Coverage

Status	Category	Percentage	Covered / Total
🔵	Lines	98.78%	2923 / 2959
🔵	Statements	97.79%	3062 / 3131
🔵	Functions	97.92%	803 / 820
🔵	Branches	91.22%	1893 / 2075

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
packages/components/src/Autocomplete/Autocomplete.tsx	84.21%	100%	83.33%	84.21%	83-85
packages/components/src/Autocomplete/MobileAutocomplete.tsx	100%	100%	100%	100%
packages/components/src/ComboBox/ComboBox.tsx	100%	100%	100%	100%
packages/components/src/ComboBox/MobileCombobox.tsx	100%	100%	100%	100%
packages/components/src/Select/Select.tsx	100%	100%	100%	100%
packages/components/src/TagField/TagField.tsx	97.56%	94.44%	100%	100%	238
packages/components/src/TagGroup/TagGroup.tsx	100%	84.31%	100%	100%

Generated in workflow #23597 for commit 2f617c9 by the Vitest Coverage Report Action

[github-actions]

Accessibility tests executed. Download the report here.

[aromko]

PR Review: #5740 - fix(DST-1717): expose dependencies on the components that own their collection

Overview

Field	Value
Author	@sebald
Branch	fix/DST-1717-expose-dependencies → main
Files Changed	13
Additions	+641
Deletions	-15
CI	All green (Typecheck, Lint, Format, Unit Tests 1-4, Storybook Tests 1-4, Size Limit, CodeQL)
Visual Regression	Not started

Linked Ticket

DST-1717: 🐛 Expose dependencies on the components that own their collection

• Status: In Progress
• Scope match: complete. All five components accept dependencies, both render paths are forwarded per component (popover ListBox and tray / MobileAutocomplete / TagFieldDropdown), each gets a WithDependencies story plus a .test(), and the changeset is minor on @marigold/components.
• The ticket calls out the second forward per component as the thing to check in review — that is exactly what the four WithDependenciesMobile tests cover, and the description documents a mutation check (dropping dependencies from Select's tray ListBox fails only the mobile test).

Code Review Checklist

TypeScript Standards

• No new any types — dependencies resolves to RAC's ReadonlyArray<any> through ListBoxProps['dependencies'], which is the ticket's stated typing
• Uses import type — import { ListBox, type ListBoxProps } in all five call sites
• Proper interface definitions — AutocompleteProps<T extends object = object> gains the item render function and mirrors TagFieldProps
• ComboBoxProps keeps its pre-existing (item: any) render function (see Suggestions)

Component Patterns

• disabled/loading/error naming untouched; no isDisabled leakage
• No className or style exposed — RemovedProps still omits both everywhere, and Autocomplete additionally omits children
• useClassNames untouched; no hardcoded styles added
• dependencies is destructured out of props in all five, so it never lands on a DOM node via ...rest
• AutocompleteBase becomes a generic function declaration but keeps ref in props and the Object.assign compound shape, matching TagFieldBase

React Best Practices

• Functional components only
• No conditional hooks; the isSmallScreen branch stays below the hook calls
• No new re-render or memoisation concerns — this is a prop forward; dependencies is the deliberate cache-invalidation escape hatch, not an optimisation

Testing Standards

• Vitest / storybook/test, no Jest APIs
• userEvent, never fireEvent
• Accessible queries throughout (getByRole, getByLabelText, findByRole) — no getByTestId
• Every new story carries tags: ['component-test'], and .test() children inherit it
• chromatic: { disableSnapshot: true } on all new stories, consistent with the "no visual change" claim
• Module-scope userEvent used where the test context provides one (see Suggestions)

Accessibility

• No ARIA, focus or keyboard behaviour changed — the diff is prop plumbing plus stories
• Tray tests scope the trigger by role because the tray title repeats the field label, and await the underlay unmounting before clicking behind it; both are documented inline

Issues Found

Critical (Must Fix)

• packages/components/src/Autocomplete/Autocomplete.stories.tsx:536 — 🔴 Visual Regression Tests — not run. This PR changes UI-affecting files (packages/components/src/**, five *.stories.tsx), and no Chromatic build exists for this branch: the workflow is triggered by issue_comment (so runs are recorded against main, not the head branch), and both runs for this PR on 2026-08-14 05:19 came back skipped, not successful.

  Worth weighing against the diff, though: the five component files change no class, style, cn() or useClassNames line — it is pure prop plumbing — and the repo's norm is VRT at merge (DST-1711/1715/1716 all merged with post-merge runs only). So this is a "run it if you want the existing snapshots to back the no-visual-change claim", not a hard blocker. Trigger with /run-chromatic on this PR.

Warnings (Should Fix)

None.

Suggestions (Nice to Have)

• packages/components/src/ComboBox/ComboBox.tsx:93 — 💡 dependencies exists purely for the items + render-function pattern, but on ComboBox that render function is still children?: ReactNode | ((item: any) => ReactNode) (line 86), so consumers who reach for this new prop get an untyped item. This PR already worked out the fix for exactly this shape on Autocomplete: add 'children' to RemovedProps, make the interface ComboBoxProps<T extends object = object>, and turn ComboBoxBase into a generic function so T is inferred from items. ComboBoxProps also carries a row of pre-existing RAC.ComboBoxProps<any>[...] lookups the same generic would clean up. Clearly out of the stated scope — but the two components now differ in exactly the place the new prop is used, so it's worth either doing here or filing a follow-up.

• packages/components/src/Select/Select.stories.tsx:1057 — 💡 The new tests take { args, canvas, step } from the test context but reach for the module-scope userEvent imported from storybook/test. CLAUDE.md is explicit: "Destructure canvas, userEvent, args, step from the test context — never within(canvasElement), never a module-scope userEvent." The PR is internally split on it: TagGroup.stories.tsx's WithDependencies.test correctly destructures userEvent from the context, while the new tests in Select, ComboBox, TagField and Autocomplete do not. Fair point that the module-scope form is the dominant existing style in those four files, so a full sweep would be a separate change — new tests are just the cheap place to stop adding to it.

• packages/components/src/TagGroup/TagGroup.tsx:40 — 💡 Select, ComboBox, TagField and Autocomplete each got the same six-line JSDoc explaining when dependencies is needed, and the docs pages render those descriptions via <AutoTypeTable>. TagGroup picks the prop up through Pick<TagListProps<object>, … 'dependencies'>, so its props table will show React Aria's terser "Values that should invalidate the item cache when using dynamic collections" instead. Redeclaring it on TagGroupProps with the same block would keep the five docs pages consistent.

Notes on the reviewer questions in the description

• TagGroup's unreachable collapse-branch forward — keeping it is the right call, and the reasoning holds: canCollapse = collapseAt !== undefined && typeof children !== 'function', so a render function can never reach that TagList. Worth a small correction to the framing though: only the collapse-branch forward is inert. The non-collapse TagList (TagGroup.tsx:317) is live, and WithDependencies proves it. The comment parked at the end of TagGroup.stories.tsx says this accurately; the PR body reads as if the whole forward were dead.
• Coverage of the five is complete — the only other internal items={items} collections in packages/components/src are Calendar/YearListBox, Calendar/MonthListBox (fixed internal data, no outside state) and Breadcrumbs (maps children directly, no dynamic collection), so none of them has the same gap.
• The Autocomplete children retype is not a practical break — the render-props form it replaces was never wired to the RAC ComboBox anyway; children goes to the ListBox / MobileAutocomplete. Type-only, and the changeset already spells it out.

Recommendation

Comment

The code is in good shape: correct fix, both collections covered per component, tests that were verified load-bearing by mutation rather than trusted green, and full CI passing. Nothing here blocks merge. Optionally run /run-chromatic to have the existing snapshots back the "no visual change" claim; the three suggestions above are take-or-leave.

---

Generated with Claude Code review-pr skill

[github-actions]

Accessibility tests executed. Download the report here.