fix: prevent internal props rendering to DOM#7984
Conversation
🦋 Changeset detectedLatest commit: 8c79aa0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
09ab19c to
78c1fc6
Compare
1915503 to
d302051
Compare
78c1fc6 to
422d492
Compare
d302051 to
8b05a66
Compare
422d492 to
e8e6005
Compare
8b05a66 to
d097ea6
Compare
There was a problem hiding this comment.
Pull request overview
This PR aims to eliminate React DOM warnings in tests caused by invalid implementsClassName fixtures, and to prevent internal/custom props from being forwarded onto DOM elements (notably in CircleBadge and deprecated ActionList.Group).
Changes:
- Updated multiple
implementsClassNameusages in tests to render components with required props/children (e.g.aria-label,name, required slot children). - Adjusted deprecated
ActionList.Groupto stop forwardinggroupIdonto the rendered<div>. - Adjusted
CircleBadgeto avoid forwardinginlineas a DOM attribute, and added a patch changeset.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/SegmentedControl/SegmentedControl.test.tsx | Wraps SegmentedControl in a fixture that includes required aria-label for className testing. |
| packages/react/src/RadioGroup/RadioGroup.test.tsx | Updates implementsClassName fixture to include required RadioGroup.Label child. |
| packages/react/src/Radio/Radio.test.tsx | Updates implementsClassName fixture to include required name prop. |
| packages/react/src/FormControl/tests/FormControl.test.tsx | Updates implementsClassName fixtures to include required FormControl.Label and an input child. |
| packages/react/src/Details/tests/Details.test.tsx | Updates implementsClassName fixture to include required Details.Summary child. |
| packages/react/src/deprecated/ActionList/Group.tsx | Stops forwarding groupId to the DOM by destructuring it out. |
| packages/react/src/DataTable/tests/Table.test.tsx | Updates implementsClassName fixture for Table.Cell to render within a required table structure. |
| packages/react/src/CircleBadge/CircleBadge.tsx | Stops forwarding inline to the DOM by destructuring it out. |
| packages/react/src/CheckboxGroup/CheckboxGroup.test.tsx | Updates implementsClassName fixture to include required CheckboxGroup.Label child. |
| packages/react/src/tests/CheckboxOrRadioGroup.test.tsx | Updates implementsClassName fixture to include required CheckboxOrRadioGroup.Label child. |
| .changeset/fix-dom-prop-warnings.md | Adds a patch changeset documenting the DOM-prop leak prevention. |
Copilot's findings
- Files reviewed: 11/11 changed files
- Comments generated: 2
| const CircleBadge = <As extends React.ElementType>({as: Component = 'div', inline, ...props}: CircleBadgeProps<As>) => ( | ||
| <Component | ||
| {...props} | ||
| data-component="CircleBadge" | ||
| className={clsx(styles.CircleBadge, props.className)} |
| * Collects related `Items` in an `ActionList`. | ||
| */ | ||
| export function Group({header, items, ...props}: GroupProps): JSX.Element { | ||
| export function Group({header, items, groupId: _groupId, ...props}: GroupProps): JSX.Element { |
e8e6005 to
24c767b
Compare
d097ea6 to
884842d
Compare
|
@copilot apply code review suggestions |
Co-authored-by: joshblack <3901764+joshblack@users.noreply.github.com>
Co-authored-by: joshblack <3901764+joshblack@users.noreply.github.com>
Fixes invalid
classNamefixtures that emitted React DOM warnings during tests. Also fixes internal prop leaks for CircleBadge and deprecated ActionList.Group so those props are not rendered on DOM elements.Changelog
New
Changed
inline,size, andvariantto DOM elements.groupIdandshowItemDividersto DOM elements.Removed
Rollout strategy
Testing & Reviewing
Validated with build, tests, type-check, lint, CSS lint, and format checks. Includes a patch changeset. Added targeted regression assertions for CircleBadge and deprecated ActionList.Group to verify internal props are not rendered as DOM attributes.
Merge checklist