Remove Box usage and sx prop from Link and misc other areas (stories, etc)

.changeset/light-schools-wish.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': major
+---
+
+Removes `Box` component usage and `sx` prop from some components

packages/react/src/Checkbox/Checkbox.docs.json

@@ -67,17 +67,7 @@
     {
       "name": "ref",
       "type": "React.RefObject<HTMLInputElement>"
-    },
-    {
-      "name": "as",
-      "type": "React.ElementType",
-      "defaultValue": "\"input\""
-    },
-    {
-      "name": "sx",
-      "type": "SystemStyleObject",
-      "deprecated": true
     }
   ],
   "subcomponents": []
-}
+}

packages/react/src/Checkbox/Checkbox.tsx

@@ -1,13 +1,11 @@
 import {clsx} from 'clsx'
 import {useProvidedRefOrCreate} from '../hooks'
 import React, {useContext, useEffect, type ChangeEventHandler, type InputHTMLAttributes, type ReactElement} from 'react'
-import {type SxProp} from '../sx'
 import useLayoutEffect from '../utils/useIsomorphicLayoutEffect'
 import type {FormValidationStatus} from '../utils/types/FormValidationStatus'
 import {CheckboxGroupContext} from '../CheckboxGroup/CheckboxGroupContext'
 import classes from './Checkbox.module.css'
 import sharedClasses from './shared.module.css'
-import Box from '../Box'
 
 export type CheckboxProps = {
   /**
@@ -35,27 +33,14 @@ export type CheckboxProps = {
    * Used during form submission and to identify which checkbox inputs are selected
    */
   value?: string
-} & Exclude<InputHTMLAttributes<HTMLInputElement>, 'value'> &
-  SxProp
+} & Exclude<InputHTMLAttributes<HTMLInputElement>, 'value'>
 
 /**
  * An accessible, native checkbox component
  */
 const Checkbox = React.forwardRef<HTMLInputElement, CheckboxProps>(
   (
-    {
-      checked,
-      className,
-      defaultChecked,
-      indeterminate,
-      disabled,
-      onChange,
-      sx: sxProp,
-      required,
-      validationStatus,
-      value,
-      ...rest
-    },
+    {checked, className, defaultChecked, indeterminate, disabled, onChange, required, validationStatus, value, ...rest},
     ref,
   ): ReactElement => {
     const checkboxRef = useProvidedRefOrCreate(ref as React.RefObject<HTMLInputElement>)
@@ -99,16 +84,6 @@ const Checkbox = React.forwardRef<HTMLInputElement, CheckboxProps>(
       }
     })
 
-    if (sxProp) {
-      return (
-        <Box
-          as="input"
-          {...inputProps}
-          className={clsx(className, sharedClasses.Input, classes.Checkbox)}
-          sx={sxProp}
-        />
-      )
-    }
     return <input {...inputProps} className={clsx(className, sharedClasses.Input, classes.Checkbox)} />
   },
 )

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,18 +1,24 @@
 import {clsx} from 'clsx'
-import React, {forwardRef, useEffect} from 'react'
+import React, {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'
+import {type PolymorphicProps, fixedForwardRef} from '../utils/modern-polymorphic'
 
-type StyledHeadingProps = {
-  as?: 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'
-  variant?: 'large' | 'medium' | 'small'
-} & SxProp
+type HeadingElement = 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'
 
-const Heading = forwardRef(({as: Component = 'h2', className, variant, ...props}, forwardedRef) => {
+const UnwrappedHeading = <As extends HeadingElement = 'h2'>(
+  props: PolymorphicProps<
+    As,
+    'h2',
+    {
+      as?: As
+      variant?: 'large' | 'medium' | 'small'
+    }
+  >,
+  forwardedRef: React.ForwardedRef<unknown>,
+) => {
+  const {as: Component = 'h2', className, variant, ...restProps} = props
   const innerRef = React.useRef<HTMLHeadingElement>(null)
   useRefObjectAsForwardedRef(forwardedRef, innerRef)
 
@@ -33,22 +39,14 @@ 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>
+  return <Component className={clsx(className, classes.Heading)} data-variant={variant} {...restProps} ref={innerRef} />
+}
+
+const Heading = fixedForwardRef(UnwrappedHeading)
 
-Heading.displayName = 'Heading'
+// We can't assign displayName to the result of fixedForwardRef directly,
+// so we assign it to the component and cast it to the expected type
+Object.assign(Heading, {displayName: 'Heading'})
 
 export type HeadingProps = ComponentProps<typeof Heading>
 export default 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/react/src/Hidden/Hidden.examples.stories.tsx

@@ -61,7 +61,7 @@ export const PullRequestPage = () => (
         <StateLabel status="pullOpened">Open</StateLabel>
         <Hidden when={['narrow']}>
           <Text sx={{fontSize: 1, color: 'fg.muted'}}>
-            <Link href="#" muted sx={{fontWeight: 'bold'}}>
+            <Link href="#" muted style={{fontWeight: 'var(--base-text-weight-semibold)'}}>
               broccolinisoup
             </Link>{' '}
             wants to merge 3 commits into

packages/react/src/Link/Link.docs.json

@@ -58,11 +58,6 @@
       "name": "as",
       "type": "React.ElementType",
       "defaultValue": "\"a\""
-    },
-    {
-      "name": "sx",
-      "type": "SystemStyleObject",
-      "deprecated": true
     }
   ],
   "subcomponents": []