Skip to content

fix(core): forward rest props (data-testid) to CheckboxInput's native input#3738

Open
let-sunny wants to merge 1 commit into
facebook:mainfrom
let-sunny:checkbox-input-forward-rest-props
Open

fix(core): forward rest props (data-testid) to CheckboxInput's native input#3738
let-sunny wants to merge 1 commit into
facebook:mainfrom
let-sunny:checkbox-input-forward-rest-props

Conversation

@let-sunny

@let-sunny let-sunny commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

CheckboxInput accepts BaseProps — which explicitly types data-* attributes ("telemetry, testing, integration hooks") — but destructures a closed list of named props with no ...rest capture, so data-testid and every other extra attribute is silently dropped and never reaches the DOM. Button, TextInput, Selector, Slider, and Badge all forward rest props to their root/interactive element; CheckboxInput is the only one of these that doesn't. The API-Conventions wiki states data-testid should "pass through to the primary interactive element".

This is the same defect class as #1415 (Selector not forwarding extra BaseProps attributes), which was fixed in #1444 with exactly this rest-spread pattern — this PR applies that established fix to CheckboxInput.

Change

  • CheckboxInput.tsx: capture ...rest in the destructure and spread it onto the native <input type="checkbox"> before the component's own named attributes, so rest can never clobber checked/disabled/type — mirroring TextInput's existing pattern.
  • No doc.mjs change: docPropReferences.test.ts exempts data-testid/className/style/xstyle/ref as universal props not listed per component (verified against TextInput.doc.mjs).

Verification

  • 3 new co-located tests: data-testid lands on the input; arbitrary data-* lands; rest props cannot override checked/disabled/type.
  • Regression proof: with the source fix reverted, the two forwarding tests fail on pre-fix code (getByTestId finds nothing / attribute null); all 31 tests in the file pass with the fix.
  • pnpm --filter @astryxdesign/core typecheck clean; eslint clean on touched files; repo pre-commit checks (sync/package-boundaries/changesets/demo-media) all pass.
  • Changeset: @astryxdesign/core patch.
  • Note: 4 pre-existing lines in the test file were re-wrapped by the repo's pre-commit prettier hook (formatting only, no logic change).

Repro (pre-fix, published @astryxdesign/core@0.1.3 and current main)

<CheckboxInput label="Accept terms" value={false} data-testid="accept-terms" />
// rendered tree contains no trace of data-testid; the same prop on Button/TextInput
// lands on the <button>/<input> respectively

… input

CheckboxInput used a closed destructuring list with no ...rest capture,
so any prop not explicitly named was silently dropped despite
CheckboxInputProps (via BaseProps) typing data-* attributes as valid.
Every sibling input component (TextInput, Selector, Slider, Button,
Badge) already forwards rest props; CheckboxInput was the sole
outlier (same defect class as facebook#1444, fixed on Selector).

Rest is spread before the input's own named attributes, mirroring
TextInput's pattern, so it cannot override checked/disabled/type or
any other explicitly-set prop.
@vercel

vercel Bot commented Jul 9, 2026

Copy link
Copy Markdown

@let-sunny is attempting to deploy a commit to the Meta Open Source Team on Vercel.

A member of the Team first needs to authorize it.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jul 9, 2026
@let-sunny let-sunny marked this pull request as ready for review July 9, 2026 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant