fix(react-breadcrumb): make BreadcrumbButtonBaseProps distribute over the ARIA button union - #36663
Open
Ray Knight (ArrayKnight) wants to merge 2 commits into
Conversation
… the ARIA button union
`BreadcrumbButtonBaseProps` was declared with a plain `Omit`:
export type BreadcrumbButtonBaseProps = Omit<BreadcrumbButtonProps, 'size'>;
`BreadcrumbButtonProps` includes `ComponentProps<ButtonSlots>`, whose `root`
slot is `ARIAButtonSlotProps<'a'>` -- a union over `{ as?: 'button' } & button
attrs` and `{ as: 'a' } & anchor attrs`. Plain `Omit` is `Pick<T, Exclude<keyof
T, K>>`, and `keyof` a union keeps only the keys common to every member, so the
omit collapses the union and every anchor-only prop (`href`, `target`, `rel`,
...) disappears from the derived type. `@fluentui/react-button` avoids exactly
this on the same shape by using `DistributiveOmit`
(Button.types.ts:72,84); this makes `react-breadcrumb` consistent with it.
Healed -- all three now type-check against the base surface where none did
before:
<BreadcrumbButton href="#a"> (the spelling react-breadcrumb's
own Default story uses)
<BreadcrumbButton as="a" href="#a">
<BreadcrumbButton as="a" href target rel>
Not changed, and honestly not a regression: a props object literal whose only
property is `href` is still rejected. That is TypeScript weak-type detection --
the `{ as?: 'button' }` union member has no required properties and shares no
property with `{ href }`, so the source only matches the `a` member, which then
demands an explicit `as: 'a'`. It fires identically on the Griffel
`BreadcrumbButtonProps`, and adding any shared property (`children`, which every
real JSX usage has) satisfies it on both. Runtime behaviour is unchanged: this
file declares types only and emits nothing.
etc/react-breadcrumb.api.md regenerated by the build.
Verified: react-breadcrumb type-check + lint pass; react-headless-components-preview
and react-components type-check pass. react-breadcrumb:test is 2 failed / 105
passed both with and without this change (pre-existing @fluentui/react-icons
snapshot drift -- SVG path data and the `fui-Icon` class -- unrelated to it).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013wmpBCYJpDJCLXcScCWz1i
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Aj9uA3rCVgosnh2zNn8qkc
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.
BreadcrumbButtonBasePropsis declared as a plainOmit<BreadcrumbButtonProps, 'size'>.BreadcrumbButtonPropsincludesComponentProps<ButtonSlots>, whose root isARIAButtonSlotProps— a distributive union overas: 'button' | 'a'. A plainOmitcollapses that union into a single object type keyed on the intersection of its members, so the anchor arm'shrefdisappears and the anchor spelling ofBreadcrumbButton(the one the component's own Default story uses) no longer type-checks against the base props type.The fix switches to
DistributiveOmit, matching what@fluentui/react-buttonalready does for the same reason. Type-level only — no runtime change — and the widened type is strictly a superset of the current one, so no existing consumer code stops compiling. Theetc/react-breadcrumb.api.mdreport is regenerated to match.Fixes #36645.
Extracted from #36656 per maintainer request — each in-tree fix from that PR as an isolated change.