From 8e4a4bda425a0802fd761f121b21dd61f557220a Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Fri, 29 May 2026 14:42:46 +0000 Subject: [PATCH 1/4] perf(PageHeader): replace 18 :has() selectors with hoisted data attributes --- .changeset/perf-pageheader-has-selectors.md | 5 ++ .../src/PageHeader/PageHeader.module.css | 40 ++++++++-------- packages/react/src/PageHeader/PageHeader.tsx | 48 ++++++++++++++----- 3 files changed, 63 insertions(+), 30 deletions(-) create mode 100644 .changeset/perf-pageheader-has-selectors.md diff --git a/.changeset/perf-pageheader-has-selectors.md b/.changeset/perf-pageheader-has-selectors.md new file mode 100644 index 00000000000..5bd74c90ed2 --- /dev/null +++ b/.changeset/perf-pageheader-has-selectors.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +PageHeader: hoist title-size and navigation-visibility state from descendants onto the root and replace 18 `:has()` selectors with plain attribute selectors. Same rendered output; the engine no longer re-evaluates subtree-scoped `:has()` selectors on every DOM mutation inside a `PageHeader`. diff --git a/packages/react/src/PageHeader/PageHeader.module.css b/packages/react/src/PageHeader/PageHeader.module.css index 992f1618d12..de5c4630eec 100644 --- a/packages/react/src/PageHeader/PageHeader.module.css +++ b/packages/react/src/PageHeader/PageHeader.module.css @@ -33,9 +33,12 @@ line-height is calculated with calc(height/font-size) and the below numbers are from @primer/primitives. --custom-font-size, --custom-line-height, --custom-font-weight are custom properties that can be used to override the below values. We don't want these values to be overridden but still want to allow consumers to override them if needed. + + Size + nav-visibility data attributes are hoisted to the root by the React + component to keep selectors as plain attribute matches (cheap) rather than + relying on `:has()` (per-descendant invalidation). */ - /* stylelint-disable selector-pseudo-class-disallowed-list -- :has() scoped to CSS Module, audited (github/github-ui#17224) */ - &:has([data-component='TitleArea'][data-size-variant='large']) { + &[data-title-size-variant='large'] { font-size: var(--custom-font-size, var(--text-title-size-large, 2rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); /* calc(48/32) */ @@ -43,7 +46,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); } - &:has([data-component='TitleArea'][data-size-variant='medium']) { + &[data-title-size-variant='medium'] { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-semibold, 600)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); /* calc(32/20) */ @@ -51,7 +54,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); } - &:has([data-component='TitleArea'][data-size-variant='subtitle']) { + &[data-title-size-variant='subtitle'] { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); /* calc(32/20) */ @@ -61,7 +64,7 @@ /* Responsive size variants */ @media (--viewportRange-narrow) { - &:has([data-component='TitleArea'][data-size-variant-narrow='large']) { + &[data-title-size-variant-narrow='large'] { font-size: var(--custom-font-size, var(--text-title-size-large, 2rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); @@ -69,7 +72,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); } - &:has([data-component='TitleArea'][data-size-variant-narrow='medium']) { + &[data-title-size-variant-narrow='medium'] { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-semibold, 600)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); @@ -77,7 +80,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); } - &:has([data-component='TitleArea'][data-size-variant-narrow='subtitle']) { + &[data-title-size-variant-narrow='subtitle'] { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); @@ -87,7 +90,7 @@ } @media (--viewportRange-regular) { - &:has([data-component='TitleArea'][data-size-variant-regular='large']) { + &[data-title-size-variant-regular='large'] { font-size: var(--custom-font-size, var(--text-title-size-large, 2rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); @@ -95,7 +98,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); } - &:has([data-component='TitleArea'][data-size-variant-regular='medium']) { + &[data-title-size-variant-regular='medium'] { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-semibold, 600)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); @@ -103,7 +106,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); } - &:has([data-component='TitleArea'][data-size-variant-regular='subtitle']) { + &[data-title-size-variant-regular='subtitle'] { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); @@ -113,7 +116,7 @@ } @media (--viewportRange-wide) { - &:has([data-component='TitleArea'][data-size-variant-wide='large']) { + &[data-title-size-variant-wide='large'] { font-size: var(--custom-font-size, var(--text-title-size-large, 2rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); @@ -121,7 +124,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); } - &:has([data-component='TitleArea'][data-size-variant-wide='medium']) { + &[data-title-size-variant-wide='medium'] { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-semibold, 600)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); @@ -129,7 +132,7 @@ --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); } - &:has([data-component='TitleArea'][data-size-variant-wide='subtitle']) { + &[data-title-size-variant-wide='subtitle'] { font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); @@ -138,33 +141,32 @@ } } - &[data-has-border='true']:has([data-component='PH_Navigation'][data-hidden-all]), - &[data-has-border='true']:not(:has([data-component='PH_Navigation'])) { + &[data-has-border='true'][data-nav-hidden-all], + &[data-has-border='true']:not([data-has-nav]) { border-block-end: var(--borderWidth-thin) solid var(--borderColor-default); padding-block-end: var(--base-size-8); } @media (--viewportRange-narrow) { - &[data-has-border='true']:has([data-component='PH_Navigation'][data-hidden-narrow]) { + &[data-has-border='true'][data-nav-hidden-narrow] { border-block-end: var(--borderWidth-thin) solid var(--borderColor-default); padding-block-end: var(--base-size-8); } } @media (--viewportRange-regular) { - &[data-has-border='true']:has([data-component='PH_Navigation'][data-hidden-regular]) { + &[data-has-border='true'][data-nav-hidden-regular] { border-block-end: var(--borderWidth-thin) solid var(--borderColor-default); padding-block-end: var(--base-size-8); } } @media (--viewportRange-wide) { - &[data-has-border='true']:has([data-component='PH_Navigation'][data-hidden-wide]) { + &[data-has-border='true'][data-nav-hidden-wide] { border-block-end: var(--borderWidth-thin) solid var(--borderColor-default); padding-block-end: var(--base-size-8); } } - /* stylelint-enable selector-pseudo-class-disallowed-list */ & [data-component='PH_LeadingAction'], & [data-component='PH_TrailingAction'], diff --git a/packages/react/src/PageHeader/PageHeader.tsx b/packages/react/src/PageHeader/PageHeader.tsx index 1acaadf06f3..1b15274e290 100644 --- a/packages/react/src/PageHeader/PageHeader.tsx +++ b/packages/react/src/PageHeader/PageHeader.tsx @@ -51,6 +51,21 @@ const Root = React.forwardRef { const rootRef = useProvidedRefOrCreate(forwardedRef as React.RefObject) + // Hoist title size + navigation visibility off direct children onto the + // root so styling can use plain attribute selectors instead of `:has()`. + let titleVariant: TitleAreaProps['variant'] = 'medium' + let hasNavigation = false + let navigationHidden: NavigationProps['hidden'] | undefined + for (const child of React.Children.toArray(children)) { + if (!React.isValidElement(child)) continue + if (child.type === TitleArea) { + titleVariant = (child.props as TitleAreaProps).variant ?? 'medium' + } else if (child.type === Navigation) { + hasNavigation = true + navigationHidden = (child.props as NavigationProps).hidden ?? false + } + } + const isInteractive = (element: HTMLElement) => { return ( ['a', 'button'].some(selector => element.matches(selector)) || @@ -108,6 +123,9 @@ const Root = React.forwardRef @@ -374,12 +392,15 @@ const Navigation: React.FC> = ({ // Based on getBreakpointDeclarations, this function will return the // correct data attribute for the given hidden value for CSS modules. -function getHiddenDataAttributes(isHidden: boolean | ResponsiveValue): { - 'data-hidden-all'?: boolean - 'data-hidden-narrow'?: boolean - 'data-hidden-regular'?: boolean - 'data-hidden-wide'?: boolean -} { +function getHiddenDataAttributes( + isHidden: boolean | ResponsiveValue, + prefix: 'hidden' | 'nav-hidden' = 'hidden', +): Record { + const all = `data-${prefix}-all` + const narrow = `data-${prefix}-narrow` + const regular = `data-${prefix}-regular` + const wide = `data-${prefix}-wide` + if (isResponsiveValue(isHidden)) { const responsiveValue = isHidden @@ -387,28 +408,28 @@ function getHiddenDataAttributes(isHidden: boolean | ResponsiveValue): const narrowMediaQuery = 'narrow' in responsiveValue ? { - 'data-hidden-narrow': responsiveValue.narrow || undefined, + [narrow]: responsiveValue.narrow || undefined, } : {} const regularMediaQuery = 'regular' in responsiveValue ? { - 'data-hidden-regular': responsiveValue.regular || undefined, + [regular]: responsiveValue.regular || undefined, } : {} const wideMediaQuery = 'wide' in responsiveValue ? { - 'data-hidden-wide': responsiveValue.wide || undefined, + [wide]: responsiveValue.wide || undefined, } : {} // check if all values are the same - this is not a recommended practice but we still should check for it if (areAllValuesTheSame(responsiveValue)) { // if all the values are the same, we can just use one of the value to determine the CSS property's value - return {'data-hidden-all': responsiveValue.narrow || undefined} + return {[all]: responsiveValue.narrow || undefined} // check if regular and wide have the same value, if so we can just return the narrow and regular media queries } else if (haveRegularAndWideSameValue(responsiveValue)) { return { @@ -424,10 +445,15 @@ function getHiddenDataAttributes(isHidden: boolean | ResponsiveValue): } } else { // If the given value is not a responsive value - return {'data-hidden-all': isHidden || undefined} + return {[all]: isHidden || undefined} } } +function getNavHiddenDataAttributes(isHidden: boolean | ResponsiveValue | undefined) { + if (isHidden === undefined) return undefined + return getHiddenDataAttributes(isHidden, 'nav-hidden') +} + export const PageHeader = Object.assign(Root, { ContextArea, ParentLink, From 79cbb1c36517bab1f2768378127de3959f764926 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Mon, 1 Jun 2026 17:45:30 +0000 Subject: [PATCH 2/4] handle nesting --- .../react/src/PageHeader/PageHeader.test.tsx | 100 ++++++++++++++++++ packages/react/src/PageHeader/PageHeader.tsx | 28 +++-- 2 files changed, 118 insertions(+), 10 deletions(-) diff --git a/packages/react/src/PageHeader/PageHeader.test.tsx b/packages/react/src/PageHeader/PageHeader.test.tsx index a66f648d4bc..6529337696f 100644 --- a/packages/react/src/PageHeader/PageHeader.test.tsx +++ b/packages/react/src/PageHeader/PageHeader.test.tsx @@ -111,4 +111,104 @@ describe('PageHeader', () => { ) expect(container.firstChild).toHaveAttribute('aria-label', 'Custom aria-label') }) + + describe('hoisted root data attributes', () => { + it('hoists the TitleArea variant onto the root as "data-title-size-variant"', () => { + const {container} = render( + + + Title + + , + ) + expect(container.firstChild).toHaveAttribute('data-title-size-variant', 'large') + }) + + it('defaults the hoisted "data-title-size-variant" to "medium" when TitleArea has no variant', () => { + const {container} = render( + + + Title + + , + ) + expect(container.firstChild).toHaveAttribute('data-title-size-variant', 'medium') + }) + + it('hoists the TitleArea variant through fragment wrappers', () => { + const {container} = render( + + <> + + Title + + + , + ) + expect(container.firstChild).toHaveAttribute('data-title-size-variant', 'large') + }) + + it('does not emit "data-title-size-variant" when no TitleArea is rendered', () => { + const {container} = render( + + + Navigation + + , + ) + expect(container.firstChild).not.toHaveAttribute('data-title-size-variant') + }) + + it('emits "data-has-nav" on the root when a Navigation child is rendered', () => { + const {container} = render( + + + Title + + + Navigation + + , + ) + expect(container.firstChild).toHaveAttribute('data-has-nav') + }) + + it('hoists a Navigation child through fragment wrappers', () => { + const {container} = render( + + <> + + Navigation + + + , + ) + expect(container.firstChild).toHaveAttribute('data-has-nav') + }) + + it('does not emit "data-has-nav" on the root when no Navigation child is rendered', () => { + const {container} = render( + + + Title + + , + ) + expect(container.firstChild).not.toHaveAttribute('data-has-nav') + }) + + it('hoists the Navigation hidden state onto the root as "data-nav-hidden-*"', () => { + const {container} = render( + + + Title + + + , + ) + expect(container.firstChild).toHaveAttribute('data-nav-hidden-narrow', 'true') + }) + }) }) diff --git a/packages/react/src/PageHeader/PageHeader.tsx b/packages/react/src/PageHeader/PageHeader.tsx index 1b15274e290..33697810999 100644 --- a/packages/react/src/PageHeader/PageHeader.tsx +++ b/packages/react/src/PageHeader/PageHeader.tsx @@ -51,20 +51,28 @@ const Root = React.forwardRef { const rootRef = useProvidedRefOrCreate(forwardedRef as React.RefObject) - // Hoist title size + navigation visibility off direct children onto the - // root so styling can use plain attribute selectors instead of `:has()`. - let titleVariant: TitleAreaProps['variant'] = 'medium' + // Hoist title size + navigation visibility off children onto the root so + // styling can use plain attribute selectors instead of `:has()`. We descend + // into fragments so this matches the previous DOM-based `:has()` selectors, + // which saw through fragment wrappers. `titleVariant` stays undefined when + // no TitleArea is rendered so the root doesn't emit title sizing in that case. + let titleVariant: TitleAreaProps['variant'] | undefined let hasNavigation = false let navigationHidden: NavigationProps['hidden'] | undefined - for (const child of React.Children.toArray(children)) { - if (!React.isValidElement(child)) continue - if (child.type === TitleArea) { - titleVariant = (child.props as TitleAreaProps).variant ?? 'medium' - } else if (child.type === Navigation) { - hasNavigation = true - navigationHidden = (child.props as NavigationProps).hidden ?? false + const hoistChildState = (nodes: React.ReactNode) => { + for (const child of React.Children.toArray(nodes)) { + if (!React.isValidElement(child)) continue + if (child.type === React.Fragment) { + hoistChildState((child.props as {children?: React.ReactNode}).children) + } else if (child.type === TitleArea) { + titleVariant = (child.props as TitleAreaProps).variant ?? 'medium' + } else if (child.type === Navigation) { + hasNavigation = true + navigationHidden = (child.props as NavigationProps).hidden ?? false + } } } + hoistChildState(children) const isInteractive = (element: HTMLElement) => { return ( From 88cda5e8598e75d3e1321894ea478743496a12fc Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Mon, 1 Jun 2026 13:55:21 -0400 Subject: [PATCH 3/4] Apply suggestion from @mattcosta7 --- packages/react/src/PageHeader/PageHeader.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/PageHeader/PageHeader.tsx b/packages/react/src/PageHeader/PageHeader.tsx index 33697810999..6983a42f3dd 100644 --- a/packages/react/src/PageHeader/PageHeader.tsx +++ b/packages/react/src/PageHeader/PageHeader.tsx @@ -57,7 +57,7 @@ const Root = React.forwardRef { for (const child of React.Children.toArray(nodes)) { From 6001d9fe6c3465bc57124d47ebfa87920713d441 Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Mon, 1 Jun 2026 20:32:34 +0000 Subject: [PATCH 4/4] refactor(PageHeader): return hoisted child state to fix TS narrowing --- packages/react/src/PageHeader/PageHeader.tsx | 32 ++++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/packages/react/src/PageHeader/PageHeader.tsx b/packages/react/src/PageHeader/PageHeader.tsx index 6983a42f3dd..e67c6db4cac 100644 --- a/packages/react/src/PageHeader/PageHeader.tsx +++ b/packages/react/src/PageHeader/PageHeader.tsx @@ -56,23 +56,37 @@ const Root = React.forwardRef { + // + // We return the hoisted state (rather than mutating closure variables) so + // TypeScript's control-flow analysis keeps the correct types at the usage + // sites below — a value mutated inside a closure stays narrowed to its + // initializer (e.g. `hasNavigation` would otherwise be typed as `false`). + type HoistedChildState = { + titleVariant?: TitleAreaProps['variant'] + hasNavigation: boolean + navigationHidden?: NavigationProps['hidden'] + } + const hoistChildState = (nodes: React.ReactNode): HoistedChildState => { + const result: HoistedChildState = {hasNavigation: false} for (const child of React.Children.toArray(nodes)) { if (!React.isValidElement(child)) continue if (child.type === React.Fragment) { - hoistChildState((child.props as {children?: React.ReactNode}).children) + const nested = hoistChildState((child.props as {children?: React.ReactNode}).children) + if (nested.titleVariant !== undefined) result.titleVariant = nested.titleVariant + if (nested.hasNavigation) { + result.hasNavigation = true + result.navigationHidden = nested.navigationHidden + } } else if (child.type === TitleArea) { - titleVariant = (child.props as TitleAreaProps).variant ?? 'medium' + result.titleVariant = (child.props as TitleAreaProps).variant ?? 'medium' } else if (child.type === Navigation) { - hasNavigation = true - navigationHidden = (child.props as NavigationProps).hidden ?? false + result.hasNavigation = true + result.navigationHidden = (child.props as NavigationProps).hidden ?? false } } + return result } - hoistChildState(children) + const {titleVariant, hasNavigation, navigationHidden} = hoistChildState(children) const isInteractive = (element: HTMLElement) => { return (