feat: normalise component displayName conventions#7871
Open
adierkens wants to merge 2 commits into
Open
Conversation
- Renames sub-component displayName strings to the canonical 'Parent.Slot' convention (BannerPrimaryAction → Banner.PrimaryAction, TimelineItem → Timeline.Item, ParentLink → PageHeader.ParentLink, HorizontalDivider → PageLayout.HorizontalDivider, etc.). - Adds displayName to compound sub-components where the runtime function name doesn't match the canonical name (e.g. Visual → Blankslate.Visual, SegmentedControlButton → SegmentedControl.Button, Panel → SelectPanel, FormControlCaption → FormControl.Caption). - Adds a contributor skill at .github/skills/display-name/SKILL.md documenting when displayName is required vs redundant, the canonical naming convention, and how it interacts with the slot system. - Skips displayName additions for top-level components whose function or variable name already matches the canonical name — modern React infers it from Function.name and the variable assignment. No runtime behaviour changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🦋 Changeset detectedLatest commit: 5ba1782 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 |
github-actions
Bot
added
the
integration-tests: recommended
Contributor
|
adierkens
added
the
Canary Release
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Normalizes React component displayName usage across @primer/react, focusing on making compound sub-components consistently appear as Parent.Slot in DevTools/stacks, and adds contributor documentation describing when displayName is required vs redundant.
Changes:
- Adds/renames
displayNamefor many compound sub-components to the canonicalParent.Slotdot-notation. - Adds a new contributor skill documenting
displayNameconventions and links it from Copilot instructions. - Includes a patch changeset describing the normalization work.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/TopicTag/TopicTagGroup.tsx | Sets TopicTagGroup.displayName to TopicTag.Group. |
| packages/react/src/TooltipV2/Tooltip.tsx | Minor formatting change near slot marker. |
| packages/react/src/Tooltip/Tooltip.tsx | Minor formatting change near deprecated slot marker. |
| packages/react/src/Timeline/Timeline.tsx | Renames displayName strings to Timeline.* dot notation. |
| packages/react/src/Stack/Stack.tsx | Adds StackItem.displayName = 'Stack.Item'. |
| packages/react/src/SelectPanel/SelectPanelMessage.tsx | Adds SelectPanelMessage.displayName = 'SelectPanel.Message'. |
| packages/react/src/SelectPanel/SelectPanel.tsx | Sets root Panel.displayName = 'SelectPanel' for the exported SelectPanel. |
| packages/react/src/SegmentedControl/SegmentedControlIconButton.tsx | Adds displayName aligned with slot marker. |
| packages/react/src/SegmentedControl/SegmentedControlButton.tsx | Adds displayName aligned with slot marker. |
| packages/react/src/PageLayout/PageLayout.tsx | Renames divider displayName strings to PageLayout.* dot notation. |
| packages/react/src/PageHeader/PageHeader.tsx | Renames slot-ish subcomponent displayName strings to PageHeader.*. |
| packages/react/src/Pagehead/Pagehead.tsx | Formatting-only change (blank line). |
| packages/react/src/FormControl/FormControlLeadingVisual.tsx | Adds displayName aligned with slot marker. |
| packages/react/src/FormControl/FormControlLabel.tsx | Adds displayName aligned with slot marker. |
| packages/react/src/FormControl/FormControlCaption.tsx | Adds displayName aligned with slot marker. |
| packages/react/src/FilteredActionList/FilteredActionListLoaders.tsx | Adds FilteredActionListBodyLoader.displayName. |
| packages/react/src/FilteredActionList/FilteredActionListInput.tsx | Adds FilteredActionListInput.displayName. |
| packages/react/src/Details/Details.tsx | Renames Summary.displayName to Details.Summary. |
| packages/react/src/DataTable/Table.tsx | Adds many displayName assignments for table subcomponents. |
| packages/react/src/DataTable/Pagination.tsx | Adds Pagination.displayName. |
| packages/react/src/DataTable/ErrorDialog.tsx | Adds ErrorDialog.displayName. |
| packages/react/src/Blankslate/Blankslate.tsx | Adds Blankslate.* subcomponent displayNames. |
| packages/react/src/Banner/Banner.tsx | Renames banner action displayNames to Banner.*. |
| packages/react/src/ActionList/TrailingAction.tsx | Adds displayName aligned with slot marker. |
| packages/react/src/ActionList/Divider.tsx | Adds displayName aligned with slot marker. |
| packages/react/src/ActionList/Description.tsx | Adds displayName aligned with slot marker. |
| packages/react/src/ActionBar/ActionBar.tsx | Adds displayNames for ActionBar compound exports. |
| .github/skills/display-name/SKILL.md | New contributor skill documenting displayName conventions. |
| .github/copilot-instructions.md | Links the new display-name skill for future reviews/authoring. |
| .changeset/displayname-consistency.md | Patch changeset describing the displayName normalization. |
Copilot's findings
Comments suppressed due to low confidence (3)
packages/react/src/TooltipV2/Tooltip.tsx:403
Tooltipis aforwardRefwrapper around an anonymous arrow function and also has a__SLOT__marker, but it never setsTooltip.displayName. In DevTools this will typically show asForwardRef/Tooltipinconsistently, and it also breaks the “displayName matches slot name” convention documented in this PR. Consider settingTooltip.displayName = 'Tooltip'(and keeping it aligned withSymbol('Tooltip')).
)
},
)
Tooltip.__SLOT__ = Symbol('Tooltip')
packages/react/src/DataTable/Pagination.tsx:390
Paginationis exposed asTable.Paginationinpackages/react/src/DataTable/index.ts, but itsdisplayNameis set toDataTable.Pagination. This makes DevTools/error stacks inconsistent with the public API. Consider renaming toTable.Pagination(or aligning the compound export to matchDataTable.*).
Pagination.displayName = 'DataTable.Pagination'
packages/react/src/DataTable/ErrorDialog.tsx:42
ErrorDialogis exposed asTable.ErrorDialoginpackages/react/src/DataTable/index.ts, but itsdisplayNameis set toDataTable.ErrorDialog. This makes DevTools/error stacks inconsistent with the public API. Consider renaming toTable.ErrorDialog(or aligning the compound export to matchDataTable.*).
}
ErrorDialog.displayName = 'DataTable.ErrorDialog'
- Files reviewed: 30/30 changed files
- Comments generated: 3
Comment on lines
+412
to
+424
| Table.displayName = 'DataTable.Table' | ||
| TableHead.displayName = 'DataTable.Head' | ||
| TableBody.displayName = 'DataTable.Body' | ||
| TableHeader.displayName = 'DataTable.Header' | ||
| TableRow.displayName = 'DataTable.Row' | ||
| TableCell.displayName = 'DataTable.Cell' | ||
| TableCellPlaceholder.displayName = 'DataTable.CellPlaceholder' | ||
| TableContainer.displayName = 'DataTable.Container' | ||
| TableTitle.displayName = 'DataTable.Title' | ||
| TableSubtitle.displayName = 'DataTable.Subtitle' | ||
| TableDivider.displayName = 'DataTable.Divider' | ||
| TableActions.displayName = 'DataTable.Actions' | ||
| TableSkeleton.displayName = 'DataTable.Skeleton' |
|
|
||
| Tooltip.directions = ['n', 'ne', 'e', 'se', 's', 'sw', 'w', 'nw'] | ||
|
|
||
| Tooltip.__SLOT__ = Symbol('DEPRECATED_Tooltip') |
Comment on lines
+42
to
+48
| ``` | ||
|
|
||
| ### 3. Sub-components in the slot system | ||
|
|
||
| The slot system's dev-mode `displayName`-mismatch warning compares `child.type.displayName` against the slot component's `displayName`. Wrappers built with `asSlot` inherit the marker; setting an explicit `displayName` on each slot sub-component keeps the warning useful even when the wrapper's `Function.name` is generic. | ||
|
|
||
| ## When you can skip `displayName` |
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.
Closes #
Normalises component
displayNamestrings across@primer/reactand addsdisplayNamewhere it's actually needed — i.e. compound sub-components whose runtime function name doesn't match the canonical name users see, andforwardRefwrappers with anonymous inner arrow functions. Top-level components whose function/variable name already matches the canonical name are intentionally left alone — modern React infersdisplayNamefromFunction.nameand the variable assignment, so addingX.displayName = 'X'is noise.Companion to the slot consistency work in #7869.
Changelog
New
.github/skills/display-name/SKILL.md— a contributor skill documenting whendisplayNameis required, when it's redundant, the canonicalParent.Slotnaming convention, the required match withSymbol(...)descriptions on slot components, and a quick decision tree. Referenced from.github/copilot-instructions.md.displayNameon compound sub-components where the runtime function name doesn't match the canonical name users see:Visual/Heading/Description/PrimaryAction/SecondaryAction→Blankslate.*ActionBarIconButton/ActionBarGroup/ActionBarMenu/VerticalDivider→ActionBar.*Description/Divider/TrailingAction→ActionList.*ErrorDialog/Pagination/Table/TableHead/TableBody/TableHeader/TableRow/TableCell/TableCellPlaceholder/TableContainer/TableTitle/TableSubtitle/TableDivider/TableActions/TableSkeleton→DataTable.*FilteredActionListInput/FilteredActionListBodyLoader→FilteredActionList.*FormControlCaption/FormControlLabel/FormControlLeadingVisual→FormControl.*SegmentedControlButton/SegmentedControlIconButton→SegmentedControl.Button/.IconButtonPanel→SelectPanel,SelectPanelMessage→SelectPanel.MessageStackItem→Stack.ItemTopicTagGroup→TopicTag.GroupChanged
displayNamestrings from camelCase-without-separator to the canonicalParent.Slotdot notation:BannerPrimaryAction→Banner.PrimaryActionBannerSecondaryAction→Banner.SecondaryActionSummary(inDetails) →Details.SummaryTimelineItem→Timeline.Item,TimelineBody→Timeline.Body,TimelineBreak→Timeline.BreakParentLink(inPageHeader) →PageHeader.ParentLinkTitleArea(inPageHeader) →PageHeader.TitleAreaHorizontalDivider(inPageLayout) →PageLayout.HorizontalDividerVerticalDivider(inPageLayout) →PageLayout.VerticalDividerRemoved
What was evaluated but intentionally not changed
Top-level components whose function or variable name already matches the canonical name we want users to see (e.g.
export function Pagehead,export const VisuallyHidden = ...,const Label = forwardRef(function Label(...))) were intentionally left without an explicitdisplayName. Modern React + bundler inference handles these cases. The skill documents this decision and the cases where you do still want to setdisplayNamedefensively (anonymousforwardRefwrappers, minified production builds whereFunction.nameis stripped).Rollout strategy
No runtime behaviour changes. The renames (e.g.
TimelineItem→Timeline.Item) change what DevTools and error stacks display; if any downstream consumer is grepping React's element tree by string they'd need to update, but that's a non-public-API contract.Testing & Reviewing
npx tsc -p packages/react/tsconfig.json --noEmitclean.npx eslinton all 30 changed files clean.main).Worth a close review:
.github/skills/display-name/SKILL.md— convention statements. Let me know if anything contradicts your intent, particularly the "skip displayName for top-level matching names" recommendation.Merge checklist