feat(Heading): remove support for sx prop

.changeset/weak-papers-drive.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': major
+---
+
+Remove support for `sx` from the `Heading` component

packages/react/src/Header/Header.dev.module.css

@@ -11,3 +11,7 @@
 .HeaderDevLink {
   color: var(--color-prettylights-syntax-carriage-return-text);
 }
+
+.Icon {
+  margin-right: var(--base-size-8);
+}

packages/react/src/Heading/Heading.docs.json

@@ -6,11 +6,6 @@
   "stories": [],
   "importPath": "@primer/react",
   "props": [
-    {
-      "name": "sx",
-      "type": "SystemStyleObject",
-      "deprecated": true
-    },
     {
       "name": "as",
       "type": "React.ElementType",

packages/react/src/Heading/Heading.features.stories.tsx

@@ -5,17 +5,6 @@ export default {
   title: 'Components/Heading/Features',
 }
 
-export const TestSx: StoryFn<typeof Heading> = () => (
-  <Heading
-    sx={{
-      fontSize: 2,
-      fontWeight: 'normal',
-    }}
-  >
-    Heading with sx override
-  </Heading>
-)
-
 export const Small: StoryFn<typeof Heading> = () => <Heading variant="small">Small heading</Heading>
 
 export const Medium: StoryFn<typeof Heading> = () => <Heading variant="medium">Medium heading</Heading>

packages/react/src/Heading/Heading.tsx

@@ -1,16 +1,16 @@
 import {clsx} from 'clsx'
 import React, {forwardRef, useEffect} from 'react'
 import {useRefObjectAsForwardedRef} from '../hooks'
-import type {SxProp} from '../sx'
 import type {ComponentProps} from '../utils/types'
 import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
 import classes from './Heading.module.css'
-import Box from '../Box'
+
+type HeadingLevels = 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'
 
 type StyledHeadingProps = {
-  as?: 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'
+  as?: HeadingLevels
   variant?: 'large' | 'medium' | 'small'
-} & SxProp
+}
 
 const Heading = forwardRef(({as: Component = 'h2', className, variant, ...props}, forwardedRef) => {
   const innerRef = React.useRef<HTMLHeadingElement>(null)
@@ -33,20 +33,8 @@ const Heading = forwardRef(({as: Component = 'h2', className, variant, ...props}
     }, [innerRef])
   }
 
-  if (props.sx) {
-    return (
-      <Box
-        as={Component}
-        className={clsx(className, classes.Heading)}
-        data-variant={variant}
-        {...props}
-        // @ts-ignore temporary disable as we migrate to css modules, until we remove PolymorphicForwardRefComponent
-        ref={innerRef}
-      />
-    )
-  }
   return <Component className={clsx(className, classes.Heading)} data-variant={variant} {...props} ref={innerRef} />
-}) as PolymorphicForwardRefComponent<'h2', StyledHeadingProps>
+}) as PolymorphicForwardRefComponent<HeadingLevels, StyledHeadingProps>
 
 Heading.displayName = 'Heading'
 

packages/react/src/Heading/__tests__/Heading.test.tsx

@@ -1,30 +1,6 @@
 import {describe, expect, it, vi} from 'vitest'
 import {Heading} from '../..'
 import {render, screen} from '@testing-library/react'
-import ThemeProvider from '../../ThemeProvider'
-
-const theme = {
-  breakpoints: ['400px', '640px', '960px', '1280px'],
-  colors: {
-    green: ['#010', '#020', '#030', '#040', '#050', '#060'],
-  },
-  fontSizes: ['12px', '14px', '16px', '20px', '24px', '32px', '40px', '48px'],
-  fonts: {
-    normal: 'Helvetica,sans-serif',
-    mono: 'Consolas,monospace',
-  },
-  lineHeights: {
-    normal: 1.5,
-    condensed: 1.25,
-    condensedUltra: 1,
-  },
-  fontWeights: {
-    light: '300',
-    normal: '400',
-    semibold: '500',
-    bold: '600',
-  },
-}
 
 describe('Heading', () => {
   it('should support `className` on the outermost element', () => {
@@ -38,89 +14,6 @@ describe('Heading', () => {
     expect(heading.tagName).toBe('H2')
   })
 
-  it('respects fontWeight', () => {
-    const {container} = render(
-      <ThemeProvider theme={theme}>
-        <Heading sx={{fontWeight: 'bold'}} />
-      </ThemeProvider>,
-    )
-    const heading = container.firstChild as HTMLElement
-    expect(heading).toHaveStyle(`font-weight: ${theme.fontWeights.bold}`)
-
-    const {container: container2} = render(
-      <ThemeProvider theme={theme}>
-        <Heading sx={{fontWeight: 'normal'}} />
-      </ThemeProvider>,
-    )
-    const heading2 = container2.firstChild as HTMLElement
-    expect(heading2).toHaveStyle(`font-weight: ${theme.fontWeights.normal}`)
-
-    const {container: container3} = render(
-      <ThemeProvider theme={theme}>
-        <Heading sx={{fontWeight: 'semibold'}} />
-      </ThemeProvider>,
-    )
-    const heading3 = container3.firstChild as HTMLElement
-    expect(heading3).toHaveStyle(`font-weight: ${theme.fontWeights.semibold}`)
-
-    const {container: container4} = render(
-      <ThemeProvider theme={theme}>
-        <Heading sx={{fontWeight: 'light'}} />
-      </ThemeProvider>,
-    )
-    const heading4 = container4.firstChild as HTMLElement
-    expect(heading4).toHaveStyle(`font-weight: ${theme.fontWeights.light}`)
-  })
-
-  it('respects lineHeight', () => {
-    const {container} = render(
-      <ThemeProvider theme={theme}>
-        <Heading sx={{lineHeight: 'normal'}} />
-      </ThemeProvider>,
-    )
-    const heading = container.firstChild as HTMLElement
-    ///These sx tests should go away right?
-    expect(heading).toHaveStyle(`line-height: 48px`)
-
-    const {container: container2} = render(
-      <ThemeProvider theme={theme}>
-        <Heading sx={{lineHeight: 'condensed'}} />
-      </ThemeProvider>,
-    )
-    const heading2 = container2.firstChild as HTMLElement
-    expect(heading2).toHaveStyle(`line-height: 40px`)
-
-    const {container: container3} = render(
-      <ThemeProvider theme={theme}>
-        <Heading sx={{lineHeight: 'condensedUltra'}} />
-      </ThemeProvider>,
-    )
-    const heading3 = container3.firstChild as HTMLElement
-    expect(heading3).toHaveStyle(`line-height: 32px`)
-  })
-
-  it('respects fontFamily="mono"', () => {
-    const {container} = render(
-      <ThemeProvider theme={theme}>
-        <Heading sx={{fontFamily: 'mono'}} />
-      </ThemeProvider>,
-    )
-    const heading = container.firstChild as HTMLElement
-    expect(heading).toHaveStyle(`font-family: ${theme.fonts.mono}`)
-  })
-
-  it('renders fontSize', () => {
-    for (const fontSize of theme.fontSizes) {
-      const {container} = render(
-        <ThemeProvider theme={theme}>
-          <Heading sx={{fontSize}} />
-        </ThemeProvider>,
-      )
-      const heading = container.firstChild as HTMLElement
-      expect(heading).toHaveStyle(`font-size: ${fontSize}`)
-    }
-  })
-
   it('logs a warning when trying to render invalid "as" prop', () => {
     const consoleSpy = vi.spyOn(globalThis.console, 'warn').mockImplementation(() => {})
 
@@ -130,15 +23,6 @@ describe('Heading', () => {
 
     consoleSpy.mockRestore()
   })
-  it('respects the "fontStyle" prop', () => {
-    const {container} = render(
-      <ThemeProvider theme={theme}>
-        <Heading sx={{fontStyle: 'italic'}} />
-      </ThemeProvider>,
-    )
-    const heading = container.firstChild as HTMLElement
-    expect(heading).toHaveStyle('font-style: italic')
-  })
 
   // How can we test for generated class names?
   it.skip('should only include css modules class', () => {
@@ -148,18 +32,4 @@ describe('Heading', () => {
     // for this component
     expect(screen.getByText('test')).not.toHaveClass(/^Heading__StyledHeading/)
   })
-
-  it('should support overrides with sx if provided', () => {
-    render(
-      <Heading
-        sx={{
-          fontWeight: '900',
-        }}
-      >
-        test
-      </Heading>,
-    )
-
-    expect(screen.getByText('test')).toHaveStyle('font-weight: 900')
-  })
 })

packages/styled-react/src/components/Heading.tsx

@@ -0,0 +1,17 @@
+import {Heading as PrimerHeading} from '@primer/react'
+import {type HeadingProps as PrimerHeadingProps} from '@primer/react'
+import type {ForwardRefComponent} from '../polymorphic'
+import {sx, type SxProp} from '../sx'
+import styled from 'styled-components'
+
+type HeadingLevels = 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'
+
+type HeadingProps = PrimerHeadingProps & SxProp
+
+const Heading: ForwardRefComponent<HeadingLevels, HeadingProps> = styled(PrimerHeading).withConfig({
+  shouldForwardProp: prop => (prop as keyof HeadingProps) !== 'sx',
+})<HeadingProps>`
+  ${sx}
+`
+
+export {Heading, type HeadingProps}

packages/styled-react/src/index.tsx

@@ -7,7 +7,6 @@ export {Box, type BoxProps} from './components/Box'
 export {Button} from '@primer/react'
 export {CheckboxGroup} from '@primer/react'
 export {Details} from '@primer/react'
-export {Heading} from '@primer/react'
 export {IconButton} from '@primer/react'
 export {Label} from '@primer/react'
 export {Overlay} from '@primer/react'
@@ -36,6 +35,7 @@ export {Dialog, type DialogProps} from './components/Dialog'
 export {Flash} from './components/Flash'
 export {FormControl, type FormControlProps} from './components/FormControl'
 export {Header, type HeaderProps} from './components/Header'
+export {Heading} from './components/Heading'
 export {Link, type LinkProps} from './components/Link'
 export {LinkButton, type LinkButtonProps} from './components/LinkButton'
 export {NavList, type NavListProps} from './components/NavList'