Skip to content

Conversation

francinelucca
Copy link
Member

@francinelucca francinelucca commented Oct 10, 2025

Extends useSlot functionality to support comparing by __SLOT__ prop, not just component type. This allows consumers to create wrapper for subcomponents while still maintaining support with @primer/react components that use slots

Changelog

New

  • tests for useSlots and FormControl to account for new slot functionality

Changed

  • update useSlots config and functionality to support comparing by both component types and slot names
  • updated components and subcomponents that use useSlots or child.type === comparisons to account for slot names

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

Testing & Reviewing

Merge checklist

@francinelucca francinelucca requested a review from a team as a code owner October 10, 2025 00:32
Copy link

changeset-bot bot commented Oct 10, 2025

🦋 Changeset detected

Latest commit: f50d6b3

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

This PR includes changesets to release 2 packages
Name Type
@primer/react Minor
@primer/styled-react Major

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

@github-actions github-actions bot added the staff Author is a staff member label Oct 10, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR extends the useSlots functionality to support comparing components by their __SLOT__ property in addition to their component type. This enables consumers to create wrapper components around subcomponents while maintaining compatibility with @primer/react components that use slots.

Key Changes:

  • Updated useSlots to support slot name identification via __SLOT__ property
  • Added __SLOT__ properties to components in both packages to enable slot-based matching
  • Modified components using useSlots to support the new slot configuration format

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/react/src/utils/get-slot-name.ts New utility function to extract slot names from React elements
packages/react/src/hooks/useSlots.ts Core logic update to support slot-based matching and new configuration format
packages/react/src/hooks/tests/useSlots.test.tsx Comprehensive tests for new slot functionality
packages/react/src/FormControl/FormControl.tsx Updated to use new slot configuration and support slot-based component identification
packages/react/src/FormControl/tests/FormControl.test.tsx Tests for slot identification with wrapped components
packages/react/src/FormControl/FormControlLabel.tsx Added __SLOT__ property for slot identification
packages/styled-react/src/components/*.tsx Added __SLOT__ properties to multiple components for compatibility
Multiple component files Updated useSlots configurations and component type checks to support slot names

@github-actions github-actions bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Oct 10, 2025
Copy link
Contributor

👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks!

@github-actions github-actions bot requested a deployment to storybook-preview-6976 October 10, 2025 00:36 Abandoned
Comment on lines 116 to 117
// @ts-ignore - TS doesn't know about the __SLOT__ property
ActionListItem.__SLOT__ = Symbol('ActionList.Item')
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open to suggestions about this @ts-ignore here
CC: @adierkens

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't want to push to your branch, but this should fix the TS types: 60799a3

@github-actions github-actions bot requested a deployment to storybook-preview-6976 October 10, 2025 00:40 Abandoned
@github-actions github-actions bot temporarily deployed to storybook-preview-6976 October 10, 2025 00:50 Inactive
@github-actions github-actions bot requested a deployment to storybook-preview-6976 October 10, 2025 01:38 Abandoned
@github-actions github-actions bot requested a deployment to storybook-preview-6976 October 10, 2025 01:44 Abandoned
@github-actions github-actions bot temporarily deployed to storybook-preview-6976 October 10, 2025 02:05 Inactive
Copy link
Contributor

👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks!

@github-actions github-actions bot requested a deployment to storybook-preview-6976 October 10, 2025 21:15 Abandoned
@github-actions github-actions bot added integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm and removed integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Oct 10, 2025
Copy link
Contributor

👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks!

@github-actions github-actions bot requested a deployment to storybook-preview-6976 October 10, 2025 21:18 Abandoned
@github-actions github-actions bot added integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm and removed integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Oct 10, 2025
Copy link
Contributor

👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks!

1 similar comment
Copy link
Contributor

👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks!

@github-actions github-actions bot requested a deployment to storybook-preview-6976 October 10, 2025 21:23 Abandoned
@github-actions github-actions bot added integration-tests: failing Changes in this PR cause breaking changes in gh/gh and removed integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm integration-tests: passing Changes in this PR do NOT cause breaking changes in gh/gh labels Oct 10, 2025
@primer-integration
Copy link

🟢 ci completed with status success.

@github-actions github-actions bot added integration-tests: passing Changes in this PR do NOT cause breaking changes in gh/gh and removed integration-tests: failing Changes in this PR cause breaking changes in gh/gh labels Oct 10, 2025
@francinelucca francinelucca added this pull request to the merge queue Oct 10, 2025
Merged via the queue into main with commit 2ec5bf2 Oct 10, 2025
42 of 43 checks passed
@francinelucca francinelucca deleted the chore/implement-slot-names branch October 10, 2025 22:46
@primer primer bot mentioned this pull request Oct 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration-tests: passing Changes in this PR do NOT cause breaking changes in gh/gh staff Author is a staff member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants