diff --git a/.changeset/displayname-consistency.md b/.changeset/displayname-consistency.md new file mode 100644 index 00000000000..179c286c620 --- /dev/null +++ b/.changeset/displayname-consistency.md @@ -0,0 +1,11 @@ +--- +'@primer/react': patch +--- + +Normalise component `displayName` strings to the canonical `Parent.Slot` convention, and add `displayName` to compound sub-components and `forwardRef`-wrapped components that were missing it. + +- Renames sub-component `displayName` strings that were using camelCase without a separator to the canonical `Parent.Slot` convention: `BannerPrimaryAction` → `Banner.PrimaryAction`, `BannerSecondaryAction` → `Banner.SecondaryAction`, `Summary` → `Details.Summary`, `TimelineItem|Body|Break` → `Timeline.Item|Body|Break`, `ParentLink|TitleArea` → `PageHeader.ParentLink|TitleArea`, `HorizontalDivider|VerticalDivider` → `PageLayout.HorizontalDivider|VerticalDivider`. +- Adds `displayName` to compound sub-components where the runtime function name doesn't match the canonical name users see (e.g. `Visual` → `Blankslate.Visual`, `SegmentedControlButton` → `SegmentedControl.Button`, `Panel` → `SelectPanel`, `FormControlCaption` → `FormControl.Caption`, etc.) and to `forwardRef`-wrapped components with anonymous inner arrow functions where DevTools would otherwise show `ForwardRef`. +- 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. + +No runtime behaviour changes. \ No newline at end of file diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index d58f438ffe1..6f96735cdfd 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -136,6 +136,10 @@ npm run lint:fix # Auto-fix linting issues When working on UI components, always use the `primer-storybook` MCP tools to access Storybook's component and documentation knowledge before answering or taking any action. Reference the `.github/skills/storybook/SKILL.md` file for detailed instructions on using the Storybook MCP effectively and accurately. +## `displayName` + +When authoring or reviewing a component and deciding whether to set `displayName`, reference the `.github/skills/display-name/SKILL.md` file. It covers when `displayName` is needed (compound sub-components, anonymous `forwardRef` wrappers, slot sub-components) vs when modern React's `Function.name` inference makes it redundant, plus the canonical `Parent.Slot` naming convention. + ## Known Issues and Workarounds **Timing Expectations:** diff --git a/.github/skills/display-name/SKILL.md b/.github/skills/display-name/SKILL.md new file mode 100644 index 00000000000..1722c20e98f --- /dev/null +++ b/.github/skills/display-name/SKILL.md @@ -0,0 +1,128 @@ +--- +name: display-name +description: "Use when: authoring or reviewing a component in Primer React, deciding whether to set `displayName`, and choosing the right string for it. Covers the criteria (when it's needed vs redundant), the canonical naming convention, and interaction with the slot system." +--- + +# `displayName` in Primer React + +`Component.displayName` controls how the component appears in React DevTools, error stacks, and the slot system's dev-mode warnings. This skill documents when to set it, when not to, and how to name it. + +## TL;DR + +- **Set `displayName` whenever the runtime function name doesn't match the canonical name you want users to see** — i.e. compound sub-components (`Banner.PrimaryAction`) or `forwardRef` wrappers around anonymous arrow functions. +- **Skip `displayName` when the function/variable name already matches the canonical name.** Modern React + bundlers infer it from `Function.name` and assigning to a `const`. Adding `X.displayName = 'X'` is noise. +- **Always use `Parent.Slot` dot notation** for sub-components, never camelCase concatenation (`TimelineItem` ❌, `Timeline.Item` ✅). + +## When to set `displayName` + +Set it in these cases: + +### 1. Compound sub-components where the function name doesn't match the canonical name + +```tsx +// The function/variable name is the bare slot name, but DevTools should +// show the dot-notation compound name. +function Visual(...) { ... } +Visual.displayName = 'Blankslate.Visual' + +const SegmentedControlButton = forwardRef(...) +SegmentedControlButton.displayName = 'SegmentedControl.Button' + +const Panel = (...) => ... +Panel.displayName = 'SelectPanel' // exported as `SelectPanel`, not `Panel` +``` + +### 2. `forwardRef` (or `memo`) wrapping an anonymous arrow function + +```tsx +// ❌ Without displayName, DevTools shows "ForwardRef" +// (variable-name inference works in React 18+ but is unreliable under minification). +const Octicon = React.forwardRef((props, ref) => ...) +Octicon.displayName = 'Octicon' +``` + +### 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` + +Skip it in these cases — adding it is pure noise: + +### 1. Named function components whose name matches the canonical name + +```tsx +// ✅ `Function.name === 'Pagehead'` already; DevTools picks it up. +export function Pagehead({...}) { ... } +// No displayName needed. + +// Same for arrow functions assigned to a const: +export const VisuallyHidden = ({...}) => { ... } +// `VisuallyHidden.name === 'VisuallyHidden'` via variable-name inference. +``` + +### 2. `forwardRef` with a named inner function whose name matches + +```tsx +// ✅ React 18+ uses the inner function name. +const Label = React.forwardRef(function Label(props, ref) { ... }) +// No displayName needed. +``` + +### 3. `forwardRef` wrapping a `const` whose name matches (in React 18+) + +```tsx +// In React 18 + modern bundlers, DevTools infers "Select" from the const assignment. +const Select = React.forwardRef((props, ref) => ...) +// Skip displayName for the root, BUT set it on Select.Option / Select.OptGroup if +// those wrap something whose name differs. +``` + +> Caveat: under aggressive minification (terser `keep_fnames: false`), `Function.name` becomes a short hash. If you ship a minified bundle and want DevTools to remain accurate in production builds, you can set `displayName` defensively. Primer's published build does not currently minify function names, so we treat `displayName` as optional in the cases above. + +## Naming convention + +| Component shape | Convention | Example | +| --------------------------- | ----------------------- | ------------------------------------------ | +| Top-level (root) component | `'ComponentName'` | `'Dialog'`, `'ActionList'` | +| Sub-component of a compound | `'Parent.Slot'` | `'Dialog.Header'`, `'PageLayout.Pane'` | +| Nested sub-component | `'Parent.Slot.SubSlot'` | `'ActionList.GroupHeading.TrailingAction'` | + +### Don't + +- `'TimelineItem'` — use `'Timeline.Item'`. +- `'BannerPrimaryAction'` — use `'Banner.PrimaryAction'`. +- `'ParentLink'` (PageHeader) — use `'PageHeader.ParentLink'`. +- `'DEPRECATED_Tooltip'` or lifecycle annotations — handle deprecation via JSDoc/changesets, not the symbol/displayName. +- `'My-Component'` (kebab-case) or `'my.component'` (lowercase) — always PascalCase tokens separated by dots. + +### Match the `Symbol(...)` description + +If the component has a `__SLOT__` marker, the `Symbol(...)` description **must match** the `displayName`. The slot system's `displayName`-mismatch warning depends on this contract: + +```tsx +Header.displayName = 'Dialog.Header' +Header.__SLOT__ = Symbol('Dialog.Header') +``` + +See the `slots` skill for more on the slot system. + +## Checklist for a new compound component + +1. Top-level component: usually skip `displayName` — let the function name carry through. +2. Each sub-component (e.g. `Foo.Bar`, `Foo.Baz`): + - Set `Sub.displayName = 'Foo.Bar'` after the declaration. + - If it's a slot sub-component, also set `Sub.__SLOT__ = Symbol('Foo.Bar')` with the matching description. +3. If you use `forwardRef`/`memo` with an anonymous arrow inner function, either: + - Promote to a named inner: `forwardRef(function Foo(props, ref) { ... })`, or + - Set `Foo.displayName = 'Foo'` explicitly. + +## Quick decision tree + +``` +Is the function/variable name already the same as the canonical name? +├── Yes → Is it forwardRef(arrow)? +│ ├── Yes (anonymous inner) → Set displayName +│ └── No (named or const) → Skip; DevTools infers +└── No → Always set displayName (and match any __SLOT__ Symbol) +``` diff --git a/packages/react/src/ActionBar/ActionBar.tsx b/packages/react/src/ActionBar/ActionBar.tsx index 8fd85484606..dfcc6d57501 100644 --- a/packages/react/src/ActionBar/ActionBar.tsx +++ b/packages/react/src/ActionBar/ActionBar.tsx @@ -473,3 +473,7 @@ export const VerticalDivider = () => { /> ) } +ActionBarIconButton.displayName = 'ActionBar.IconButton' +ActionBarGroup.displayName = 'ActionBar.Group' +ActionBarMenu.displayName = 'ActionBar.Menu' +VerticalDivider.displayName = 'ActionBar.VerticalDivider' diff --git a/packages/react/src/ActionList/Description.tsx b/packages/react/src/ActionList/Description.tsx index 07b0e4c3d5a..50c4e9ba5d4 100644 --- a/packages/react/src/ActionList/Description.tsx +++ b/packages/react/src/ActionList/Description.tsx @@ -82,4 +82,5 @@ export const Description: FCWithSlotMarker +TrailingAction.displayName = 'ActionList.TrailingAction' TrailingAction.__SLOT__ = Symbol('ActionList.TrailingAction') diff --git a/packages/react/src/Banner/Banner.tsx b/packages/react/src/Banner/Banner.tsx index 8837134b86d..49429f688f0 100644 --- a/packages/react/src/Banner/Banner.tsx +++ b/packages/react/src/Banner/Banner.tsx @@ -276,7 +276,7 @@ const BannerPrimaryAction = forwardRef(({children, className, ...rest}, forwarde ) }) as PolymorphicForwardRefComponent<'button', BannerPrimaryActionProps> -BannerPrimaryAction.displayName = 'BannerPrimaryAction' +BannerPrimaryAction.displayName = 'Banner.PrimaryAction' export type BannerSecondaryActionProps = Omit @@ -294,6 +294,6 @@ const BannerSecondaryAction = forwardRef(({children, className, ...rest}, forwar ) }) as PolymorphicForwardRefComponent<'button', BannerSecondaryActionProps> -BannerSecondaryAction.displayName = 'BannerSecondaryAction' +BannerSecondaryAction.displayName = 'Banner.SecondaryAction' export {BannerPrimaryAction, BannerSecondaryAction} diff --git a/packages/react/src/Blankslate/Blankslate.tsx b/packages/react/src/Blankslate/Blankslate.tsx index fa785e9991b..3ae7d91c2e6 100644 --- a/packages/react/src/Blankslate/Blankslate.tsx +++ b/packages/react/src/Blankslate/Blankslate.tsx @@ -145,3 +145,8 @@ export type { BlankslatePrimaryActionProps, BlankslateSecondaryActionProps, } +Visual.displayName = 'Blankslate.Visual' +Heading.displayName = 'Blankslate.Heading' +Description.displayName = 'Blankslate.Description' +PrimaryAction.displayName = 'Blankslate.PrimaryAction' +SecondaryAction.displayName = 'Blankslate.SecondaryAction' diff --git a/packages/react/src/DataTable/ErrorDialog.tsx b/packages/react/src/DataTable/ErrorDialog.tsx index c7dec38fb11..04900bb7e87 100644 --- a/packages/react/src/DataTable/ErrorDialog.tsx +++ b/packages/react/src/DataTable/ErrorDialog.tsx @@ -37,3 +37,5 @@ export function ErrorDialog({title = 'Error', children, onRetry, onDismiss}: Tab ) } + +ErrorDialog.displayName = 'DataTable.ErrorDialog' diff --git a/packages/react/src/DataTable/Pagination.tsx b/packages/react/src/DataTable/Pagination.tsx index c8463b0b991..2b3a7f9d02e 100644 --- a/packages/react/src/DataTable/Pagination.tsx +++ b/packages/react/src/DataTable/Pagination.tsx @@ -385,3 +385,5 @@ function usePagination(config: PaginationConfig): PaginationResult { selectNextPage, } } + +Pagination.displayName = 'DataTable.Pagination' diff --git a/packages/react/src/DataTable/Table.tsx b/packages/react/src/DataTable/Table.tsx index deb39f018df..98afa809ac4 100644 --- a/packages/react/src/DataTable/Table.tsx +++ b/packages/react/src/DataTable/Table.tsx @@ -408,3 +408,17 @@ export { TableCellPlaceholder, TableSkeleton, } + +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' diff --git a/packages/react/src/Details/Details.tsx b/packages/react/src/Details/Details.tsx index f578cb222b9..b75ee861256 100644 --- a/packages/react/src/Details/Details.tsx +++ b/packages/react/src/Details/Details.tsx @@ -57,7 +57,7 @@ function Summary({as, children, ...props}: Summary ) } -Summary.displayName = 'Summary' +Summary.displayName = 'Details.Summary' export {Summary} diff --git a/packages/react/src/FilteredActionList/FilteredActionListInput.tsx b/packages/react/src/FilteredActionList/FilteredActionListInput.tsx index 987f4d8ffed..170b50da261 100644 --- a/packages/react/src/FilteredActionList/FilteredActionListInput.tsx +++ b/packages/react/src/FilteredActionList/FilteredActionListInput.tsx @@ -57,3 +57,5 @@ export function FilteredActionListInput({ ) } + +FilteredActionListInput.displayName = 'FilteredActionList.Input' diff --git a/packages/react/src/FilteredActionList/FilteredActionListLoaders.tsx b/packages/react/src/FilteredActionList/FilteredActionListLoaders.tsx index b28567e4072..104e752e369 100644 --- a/packages/react/src/FilteredActionList/FilteredActionListLoaders.tsx +++ b/packages/react/src/FilteredActionList/FilteredActionListLoaders.tsx @@ -66,3 +66,5 @@ function LoadingSkeleton({rows = 10, ...props}: {rows: number}): JSX.Element { ) } + +FilteredActionListBodyLoader.displayName = 'FilteredActionList.BodyLoader' diff --git a/packages/react/src/FormControl/FormControlCaption.tsx b/packages/react/src/FormControl/FormControlCaption.tsx index c8c30eba9ef..8c288960232 100644 --- a/packages/react/src/FormControl/FormControlCaption.tsx +++ b/packages/react/src/FormControl/FormControlCaption.tsx @@ -26,6 +26,7 @@ function FormControlCaption({id, children, className, style}: FormControlCaption ) } +FormControlCaption.displayName = 'FormControl.Caption' FormControlCaption.__SLOT__ = Symbol('FormControl.Caption') export {FormControlCaption} diff --git a/packages/react/src/FormControl/FormControlLabel.tsx b/packages/react/src/FormControl/FormControlLabel.tsx index e0a6e829cb8..37b76464868 100644 --- a/packages/react/src/FormControl/FormControlLabel.tsx +++ b/packages/react/src/FormControl/FormControlLabel.tsx @@ -57,6 +57,7 @@ const FormControlLabel: FCWithSlotMarker< ) } +FormControlLabel.displayName = 'FormControl.Label' FormControlLabel.__SLOT__ = Symbol('FormControl.Label') export default FormControlLabel diff --git a/packages/react/src/FormControl/FormControlLeadingVisual.tsx b/packages/react/src/FormControl/FormControlLeadingVisual.tsx index c038c1705be..5996e043138 100644 --- a/packages/react/src/FormControl/FormControlLeadingVisual.tsx +++ b/packages/react/src/FormControl/FormControlLeadingVisual.tsx @@ -21,6 +21,7 @@ const FormControlLeadingVisual: FCWithSlotMarker( ) }, ) as PolymorphicForwardRefComponent<'a', ParentLinkProps> -ParentLink.displayName = 'ParentLink' +ParentLink.displayName = 'PageHeader.ParentLink' // ContextBar // Generic slot for any component above the title region. Use it for custom breadcrumbs and other navigation elements instead of ParentLink. @@ -220,7 +220,7 @@ const TitleArea = React.forwardRef -TitleArea.displayName = 'TitleArea' +TitleArea.displayName = 'PageHeader.TitleArea' // PageHeader.LeadingAction and PageHeader.TrailingAction should only be visible on regular viewports. // So they come as hidden on narrow viewports by default and their visibility can be managed by their `hidden` prop. diff --git a/packages/react/src/PageLayout/PageLayout.tsx b/packages/react/src/PageLayout/PageLayout.tsx index bd1ff29733a..79e031d06c5 100644 --- a/packages/react/src/PageLayout/PageLayout.tsx +++ b/packages/react/src/PageLayout/PageLayout.tsx @@ -170,7 +170,7 @@ const HorizontalDivider = memo>( }, ) -HorizontalDivider.displayName = 'HorizontalDivider' +HorizontalDivider.displayName = 'PageLayout.HorizontalDivider' type VerticalDividerProps = DividerProps & { draggable?: boolean @@ -191,7 +191,7 @@ const VerticalDivider = memo>( }, ) -VerticalDivider.displayName = 'VerticalDivider' +VerticalDivider.displayName = 'PageLayout.VerticalDivider' type SidebarDividerProps = { position: 'start' | 'end' diff --git a/packages/react/src/Pagehead/Pagehead.tsx b/packages/react/src/Pagehead/Pagehead.tsx index c0bead6c382..0c901b007be 100644 --- a/packages/react/src/Pagehead/Pagehead.tsx +++ b/packages/react/src/Pagehead/Pagehead.tsx @@ -12,4 +12,5 @@ const Pagehead = ({as: BaseComponent = 'div', className, ...rest}: PageheadProps export type PageheadProps = React.ComponentPropsWithoutRef<'div'> & { as?: React.ElementType } + export default Pagehead diff --git a/packages/react/src/SegmentedControl/SegmentedControlButton.tsx b/packages/react/src/SegmentedControl/SegmentedControlButton.tsx index 075bc8f3e6e..58bc053aea4 100644 --- a/packages/react/src/SegmentedControl/SegmentedControlButton.tsx +++ b/packages/react/src/SegmentedControl/SegmentedControlButton.tsx @@ -72,4 +72,5 @@ const SegmentedControlButton: FCWithSlotMarker = props => { ) } +Panel.displayName = 'SelectPanel' + export const SelectPanel = Object.assign(Panel, { __SLOT__: Symbol('SelectPanel'), SecondaryActionButton: SecondaryButton, diff --git a/packages/react/src/SelectPanel/SelectPanelMessage.tsx b/packages/react/src/SelectPanel/SelectPanelMessage.tsx index 458c42883be..00d11c096fd 100644 --- a/packages/react/src/SelectPanel/SelectPanelMessage.tsx +++ b/packages/react/src/SelectPanel/SelectPanelMessage.tsx @@ -59,3 +59,5 @@ export const SelectPanelMessage: React.FC = ({ ) } + +SelectPanelMessage.displayName = 'SelectPanel.Message' diff --git a/packages/react/src/Stack/Stack.tsx b/packages/react/src/Stack/Stack.tsx index 9a96c6e71f6..564506b518a 100644 --- a/packages/react/src/Stack/Stack.tsx +++ b/packages/react/src/Stack/Stack.tsx @@ -155,3 +155,4 @@ const StackItem = forwardRef(({as: Component = 'div', children, grow, shrink, cl export {Stack, StackItem} export type {StackProps, StackItemProps} +StackItem.displayName = 'Stack.Item' diff --git a/packages/react/src/Timeline/Timeline.tsx b/packages/react/src/Timeline/Timeline.tsx index 8abec1e5d99..4c20407a402 100644 --- a/packages/react/src/Timeline/Timeline.tsx +++ b/packages/react/src/Timeline/Timeline.tsx @@ -48,7 +48,7 @@ const TimelineItem = React.forwardRef( }, ) -TimelineItem.displayName = 'TimelineItem' +TimelineItem.displayName = 'Timeline.Item' export type TimelineBadgeVariant = | 'accent' @@ -87,7 +87,7 @@ const TimelineBody = React.forwardRef(({class return
}) -TimelineBody.displayName = 'TimelineBody' +TimelineBody.displayName = 'Timeline.Body' export type TimelineBreakProps = { /** Class name for custom styling */ @@ -98,7 +98,7 @@ const TimelineBreak = React.forwardRef(({cla return
}) -TimelineBreak.displayName = 'TimelineBreak' +TimelineBreak.displayName = 'Timeline.Break' export default Object.assign(Timeline, { Item: TimelineItem, diff --git a/packages/react/src/Tooltip/Tooltip.tsx b/packages/react/src/Tooltip/Tooltip.tsx index 8122db14ae5..adcbeae0666 100644 --- a/packages/react/src/Tooltip/Tooltip.tsx +++ b/packages/react/src/Tooltip/Tooltip.tsx @@ -55,7 +55,6 @@ const Tooltip = React.forwardRef(function Tooltip( Tooltip.alignments = ['left', 'right'] Tooltip.directions = ['n', 'ne', 'e', 'se', 's', 'sw', 'w', 'nw'] - Tooltip.__SLOT__ = Symbol('DEPRECATED_Tooltip') export default Tooltip diff --git a/packages/react/src/TooltipV2/Tooltip.tsx b/packages/react/src/TooltipV2/Tooltip.tsx index ad347d927b4..ec7f4bcbfd4 100644 --- a/packages/react/src/TooltipV2/Tooltip.tsx +++ b/packages/react/src/TooltipV2/Tooltip.tsx @@ -399,5 +399,4 @@ export const Tooltip: ForwardRefExoticComponent< ) }, ) - Tooltip.__SLOT__ = Symbol('Tooltip') diff --git a/packages/react/src/TopicTag/TopicTagGroup.tsx b/packages/react/src/TopicTag/TopicTagGroup.tsx index 03cb88f8697..1cf861ad3b6 100644 --- a/packages/react/src/TopicTag/TopicTagGroup.tsx +++ b/packages/react/src/TopicTag/TopicTagGroup.tsx @@ -14,3 +14,5 @@ function TopicTagGroup({children, className, ...rest}: TopicTagGroupProps) { export {TopicTagGroup} export type {TopicTagGroupProps} + +TopicTagGroup.displayName = 'TopicTag.Group'