Update types for IconButton, ActionMenu.Button, TabNav.Link and internal consumers

.changeset/cuddly-rules-taste.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': minor
+---
+
+update types for button extensions

src/ActionList/Item.tsx

@@ -10,6 +10,7 @@ import {ActionListGroupProps, GroupContext} from './Group'
 import {ActionListProps, ListContext} from './List'
 import {Selection} from './Selection'
 import {ActionListItemProps, Slots, TEXT_ROW_HEIGHT, getVariantStyles} from './shared'
+import {defaultSxProp} from '../utils/defaultSxProp'
 
 const LiBox = styled.li<SxProp>(sx)
 
@@ -21,7 +22,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
       selected = undefined,
       active = false,
       onSelect,
-      sx: sxProp = {},
+      sx: sxProp = defaultSxProp,
       id,
       role,
       _PrivateItemWrapper,

src/ActionList/List.tsx

@@ -4,6 +4,7 @@ import styled from 'styled-components'
 import sx, {SxProp, merge} from '../sx'
 import {AriaRole} from '../utils/types'
 import {ActionListContainerContext} from './ActionListContainerContext'
+import {defaultSxProp} from '../utils/defaultSxProp'
 
 export type ActionListProps = React.PropsWithChildren<{
   /**
@@ -32,7 +33,7 @@ const ListBox = styled.ul<SxProp>(sx)
 
 export const List = React.forwardRef<HTMLUListElement, ActionListProps>(
   (
-    {variant = 'inset', selectionVariant, showDividers = false, role, sx: sxProp = {}, ...props},
+    {variant = 'inset', selectionVariant, showDividers = false, role, sx: sxProp = defaultSxProp, ...props},
     forwardedRef,
   ): JSX.Element => {
     const styles = {

src/ActionMenu.tsx

@@ -9,6 +9,8 @@ import {Button, ButtonProps} from './Button'
 import {useId} from './hooks/useId'
 import {MandateProps} from './utils/types'
 import {merge, BetterSystemStyleObject} from './sx'
+import {defaultSxProp} from './utils/defaultSxProp'
+import {ForwardRefComponent as PolymorphicForwardRefComponent} from './utils/polymorphic'
 
 export type MenuContextProps = Pick<
   AnchoredOverlayProps,
@@ -68,34 +70,30 @@ const Menu: React.FC<React.PropsWithChildren<ActionMenuProps>> = ({
 }
 
 export type ActionMenuAnchorProps = {children: React.ReactElement}
-const Anchor = React.forwardRef<AnchoredOverlayProps['anchorRef'], ActionMenuAnchorProps>(
-  ({children, ...anchorProps}, anchorRef) => {
-    return React.cloneElement(children, {...anchorProps, ref: anchorRef})
-  },
-)
+const Anchor = React.forwardRef<HTMLElement, ActionMenuAnchorProps>(({children, ...anchorProps}, anchorRef) => {
+  return React.cloneElement(children, {...anchorProps, ref: anchorRef})
+})
 
 /** this component is syntactical sugar 🍭 */
 export type ActionMenuButtonProps = ButtonProps
-const MenuButton = React.forwardRef<AnchoredOverlayProps['anchorRef'], ButtonProps>(
-  ({sx: sxProp = {}, ...props}, anchorRef) => {
-    return (
-      <Anchor ref={anchorRef}>
-        <Button
-          type="button"
-          trailingIcon={TriangleDownIcon}
-          sx={merge<BetterSystemStyleObject>(
-            {
-              // override the margin on caret for optical alignment
-              '[data-component=trailingIcon]': {marginX: -1},
-            },
-            sxProp,
-          )}
-          {...props}
-        />
-      </Anchor>
-    )
-  },
-)
+const MenuButton = React.forwardRef(({sx: sxProp = defaultSxProp, ...props}, anchorRef) => {
+  return (
+    <Anchor ref={anchorRef}>
+      <Button
+        type="button"
+        trailingIcon={TriangleDownIcon}
+        sx={merge<BetterSystemStyleObject>(
+          {
+            // override the margin on caret for optical alignment
+            '[data-component=trailingIcon]': {marginX: -1},
+          },
+          sxProp,
+        )}
+        {...props}
+      />
+    </Anchor>
+  )
+}) as PolymorphicForwardRefComponent<'button', ActionMenuButtonProps>
 
 type MenuOverlayProps = Partial<OverlayProps> &
   Pick<AnchoredOverlayProps, 'align'> & {

src/Button/Button.tsx

@@ -3,16 +3,13 @@ import {ButtonProps} from './types'
 import {ButtonBase} from './ButtonBase'
 import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
 
-const ButtonComponent: PolymorphicForwardRefComponent<'button', ButtonProps> = forwardRef<
-  HTMLButtonElement,
-  ButtonProps
->(({children, ...props}, forwardedRef): JSX.Element => {
+const ButtonComponent = forwardRef(({children, ...props}, forwardedRef): JSX.Element => {
   return (
     <ButtonBase ref={forwardedRef} as="button" {...props}>
       {children}
     </ButtonBase>
   )
-})
+}) as PolymorphicForwardRefComponent<'button', ButtonProps>
 
 ButtonComponent.displayName = 'Button'
 

src/Button/ButtonBase.tsx

@@ -1,14 +1,13 @@
 import React, {ComponentPropsWithRef, forwardRef, useMemo} from 'react'
 import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
 import Box from '../Box'
-import {merge, SxProp} from '../sx'
+import {BetterSystemStyleObject, merge} from '../sx'
 import {useTheme} from '../ThemeProvider'
 import {ButtonProps, StyledButton} from './types'
 import {getVariantStyles, getSizeStyles, getButtonStyles} from './styles'
 import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef'
-declare let __DEV__: boolean
+import {defaultSxProp} from '../utils/defaultSxProp'
 
-const defaultSxProp = {}
 const iconWrapStyles = {
   display: 'inline-block',
 }
@@ -17,18 +16,18 @@ const trailingIconStyles = {
   ml: 2,
 }
 
-const ButtonBase = forwardRef<HTMLElement, ButtonProps>(
+const ButtonBase = forwardRef(
   ({children, as: Component = 'button', sx: sxProp = defaultSxProp, ...props}, forwardedRef): JSX.Element => {
     const {leadingIcon: LeadingIcon, trailingIcon: TrailingIcon, variant = 'default', size = 'medium', ...rest} = props
-    const innerRef = React.useRef<HTMLElement>(null)
+    const innerRef = React.useRef<HTMLButtonElement>(null)
     useRefObjectAsForwardedRef(forwardedRef, innerRef)
 
     const {theme} = useTheme()
     const baseStyles = useMemo(() => {
       return merge.all([getButtonStyles(theme), getSizeStyles(size, variant, false), getVariantStyles(variant, theme)])
     }, [theme, size, variant])
-    const sxStyles = useMemo(() => {
-      return merge(baseStyles, sxProp as SxProp)
+    const sxStyles: BetterSystemStyleObject = useMemo(() => {
+      return merge<BetterSystemStyleObject>(baseStyles, sxProp)
     }, [baseStyles, sxProp])
 
     if (__DEV__) {
@@ -40,7 +39,10 @@ const ButtonBase = forwardRef<HTMLElement, ButtonProps>(
        */
       // eslint-disable-next-line react-hooks/rules-of-hooks
       React.useEffect(() => {
-        if (!(innerRef.current instanceof HTMLButtonElement) && !(innerRef.current instanceof HTMLAnchorElement)) {
+        if (
+          !(innerRef.current instanceof HTMLButtonElement) &&
+          !((innerRef.current as unknown) instanceof HTMLAnchorElement)
+        ) {
           // eslint-disable-next-line no-console
           console.warn('This component should be an instanceof a semantic button or anchor')
         }

src/Button/ButtonCounter.tsx

@@ -1,12 +1,13 @@
 import React from 'react'
 import {SxProp} from '../sx'
 import CounterLabel from '../CounterLabel'
+import {defaultSxProp} from '../utils/defaultSxProp'
 
 export type CounterProps = {
   children: number
 } & SxProp
 
-const Counter = ({children, sx: sxProp = {}, ...props}: CounterProps) => {
+const Counter = ({children, sx: sxProp = defaultSxProp, ...props}: CounterProps) => {
   return (
     <CounterLabel data-component="ButtonCounter" sx={{ml: 2, ...sxProp}} {...props}>
       {children}

src/Button/IconButton.stories.tsx

@@ -1,10 +1,10 @@
-import React from 'react'
+import React, {ComponentProps} from 'react'
 import {EyeClosedIcon, EyeIcon, SearchIcon, XIcon, HeartIcon} from '@primer/octicons-react'
 import {Story, Meta} from '@storybook/react'
 import {IconButton} from '.'
 import {OcticonArgType} from '../utils/story-helpers'
 
-export default {
+const meta: Meta<ComponentProps<typeof IconButton>> = {
   title: 'Components/IconButton',
   argTypes: {
     size: {
@@ -33,6 +33,8 @@ export default {
     'aria-label': 'Icon button description',
     icon: XIcon,
   },
-} as Meta<typeof IconButton>
+}
 
-export const Playground: Story<typeof IconButton> = args => <IconButton {...args} />
+export default meta
+
+export const Playground: Story<ComponentProps<typeof IconButton>> = args => <IconButton {...args} />

src/Button/IconButton.tsx

@@ -4,9 +4,11 @@ import {useTheme} from '../ThemeProvider'
 import Box from '../Box'
 import {IconButtonProps, StyledButton} from './types'
 import {getBaseStyles, getSizeStyles, getVariantStyles} from './styles'
+import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
+import {defaultSxProp} from '../utils/defaultSxProp'
 
-const IconButton = forwardRef<HTMLButtonElement, IconButtonProps>((props, forwardedRef): JSX.Element => {
-  const {variant = 'default', size = 'medium', sx: sxProp = {}, icon: Icon, ...rest} = props
+const IconButton = forwardRef((props, forwardedRef): JSX.Element => {
+  const {variant = 'default', size = 'medium', sx: sxProp = defaultSxProp, icon: Icon, ...rest} = props
   const {theme} = useTheme()
   const sxStyles = merge.all([
     getBaseStyles(theme),
@@ -15,12 +17,17 @@ const IconButton = forwardRef<HTMLButtonElement, IconButtonProps>((props, forwar
     sxProp as SxProp,
   ])
   return (
-    <StyledButton sx={sxStyles} {...rest} ref={forwardedRef}>
+    <StyledButton
+      sx={sxStyles}
+      {...rest}
+      // @ts-expect-error StyledButton wants both Anchor and Button refs, not one or the other
+      ref={forwardedRef}
+    >
       <Box as="span" sx={{display: 'inline-block'}}>
         <Icon />
       </Box>
     </StyledButton>
   )
-})
+}) as PolymorphicForwardRefComponent<'button' | 'a', IconButtonProps>
 
 export {IconButton}

src/Button/LinkButton.tsx

@@ -3,11 +3,12 @@ import {merge, SxProp} from '../sx'
 import {LinkButtonProps} from './types'
 import {ButtonBase, ButtonBaseProps} from './ButtonBase'
 import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
+import {defaultSxProp} from '../utils/defaultSxProp'
 
 type MyProps = LinkButtonProps & ButtonBaseProps
 
-const LinkButton = forwardRef<HTMLElement, MyProps>(
-  ({children, as: Component = 'a', sx = {}, ...props}, forwardedRef): JSX.Element => {
+const LinkButton = forwardRef(
+  ({children, as: Component = 'a', sx = defaultSxProp, ...props}, forwardedRef): JSX.Element => {
     const style = {
       width: 'fit-content',
       '&:hover:not([disabled])': {
@@ -23,11 +24,17 @@ const LinkButton = forwardRef<HTMLElement, MyProps>(
     }
     const sxStyle = merge.all([style, sx as SxProp])
     return (
-      <ButtonBase as={Component} ref={forwardedRef} sx={sxStyle} {...props}>
+      <ButtonBase
+        as={Component}
+        // @ts-expect-error ButtonBase wants both Anchor and Button refs
+        ref={forwardedRef}
+        sx={sxStyle}
+        {...props}
+      >
         {children}
       </ButtonBase>
     )
   },
-) as PolymorphicForwardRefComponent<'a', ButtonBaseProps>
+) as PolymorphicForwardRefComponent<'a', MyProps>
 
 export {LinkButton}

src/Button/types.ts

@@ -1,6 +1,5 @@
-import React, {ComponentPropsWithRef} from 'react'
+import React from 'react'
 import styled from 'styled-components'
-import {IconProps} from '@primer/octicons-react'
 import sx, {SxProp} from '../sx'
 import getGlobalFocusStyles from '../_getGlobalFocusStyles'
 
@@ -13,14 +12,9 @@ export type VariantType = 'default' | 'primary' | 'invisible' | 'danger' | 'outl
 
 export type Size = 'small' | 'medium' | 'large'
 
-/**
- * Remove styled-components polymorphic as prop, which conflicts with radix's
- */
-type StyledButtonProps = Omit<ComponentPropsWithRef<typeof StyledButton>, 'as'>
-
 type ButtonA11yProps =
-  | {'aria-label': string; 'aria-labelledby'?: never}
-  | {'aria-label'?: never; 'aria-labelledby': string}
+  | {'aria-label': string; 'aria-labelledby'?: undefined}
+  | {'aria-label'?: undefined; 'aria-labelledby': string}
 
 export type ButtonBaseProps = {
   /**
@@ -36,24 +30,23 @@ export type ButtonBaseProps = {
    */
   disabled?: boolean
 } & SxProp &
-  React.ButtonHTMLAttributes<HTMLButtonElement> &
-  StyledButtonProps
+  React.ButtonHTMLAttributes<HTMLButtonElement>
 
 export type ButtonProps = {
   /**
    * The leading icon comes before button content
    */
-  leadingIcon?: React.FunctionComponent<React.PropsWithChildren<IconProps>>
+  leadingIcon?: React.ComponentType | null | undefined
   /**
    * The trailing icon comes after button content
    */
-  trailingIcon?: React.FunctionComponent<React.PropsWithChildren<IconProps>>
+  trailingIcon?: React.ComponentType | null | undefined
   children: React.ReactNode
 } & ButtonBaseProps
 
 export type IconButtonProps = ButtonA11yProps & {
-  icon: React.FunctionComponent<React.PropsWithChildren<IconProps>>
-} & ButtonBaseProps
+  icon: React.ComponentType
+} & Omit<ButtonBaseProps, 'aria-label' | 'aria-labelledby'>
 
 // adopted from React.AnchorHTMLAttributes
 export type LinkButtonProps = {

src/NavList/NavList.tsx

@@ -11,6 +11,7 @@ import {
 import Box from '../Box'
 import StyledOcticon from '../StyledOcticon'
 import sx, {merge, SxProp} from '../sx'
+import {defaultSxProp} from '../utils/defaultSxProp'
 import {useId} from '../hooks/useId'
 
 // ----------------------------------------------------------------------------
@@ -43,7 +44,7 @@ export type NavListItemProps = {
 } & SxProp
 
 const Item = React.forwardRef<HTMLAnchorElement, NavListItemProps>(
-  ({'aria-current': ariaCurrent, children, sx: sxProp = {}, ...props}, ref) => {
+  ({'aria-current': ariaCurrent, children, sx: sxProp = defaultSxProp, ...props}, ref) => {
     const {depth} = React.useContext(SubNavContext)
 
     // Get SubNav from children
@@ -102,7 +103,7 @@ const ItemWithSubNavContext = React.createContext<{buttonId: string; subNavId: s
 
 // TODO: ref prop
 // TODO: Animate open/close transition
-function ItemWithSubNav({children, subNav, sx: sxProp = {}}: ItemWithSubNavProps) {
+function ItemWithSubNav({children, subNav, sx: sxProp = defaultSxProp}: ItemWithSubNavProps) {
   const buttonId = useId()
   const subNavId = useId()
   const [isOpen, setIsOpen] = React.useState(false)
@@ -167,7 +168,7 @@ const SubNavContext = React.createContext<{depth: number}>({depth: 0})
 
 // TODO: ref prop
 // NOTE: SubNav must be a direct child of an Item
-const SubNav = ({children, sx: sxProp = {}}: NavListSubNavProps) => {
+const SubNav = ({children, sx: sxProp = defaultSxProp}: NavListSubNavProps) => {
   const {buttonId, subNavId, isOpen} = React.useContext(ItemWithSubNavContext)
   const {depth} = React.useContext(SubNavContext)
 

src/PageHeader/PageHeader.stories.tsx

@@ -217,7 +217,7 @@ const Template: Story = args => (
         }}
       >
         <PageHeader.LeadingAction hidden={!args.hasLeadingAction}>
-          <IconButton icon={SidebarExpandIcon} variant="invisible" />{' '}
+          <IconButton aria-label="Expand" icon={SidebarExpandIcon} variant="invisible" />{' '}
         </PageHeader.LeadingAction>
         <PageHeader.LeadingVisual hidden={!args.hasLeadingVisual}>{<args.LeadingVisual />}</PageHeader.LeadingVisual>
         <PageHeader.Title as={args['Title.as']} hidden={!args.hasTitle}>
@@ -227,7 +227,7 @@ const Template: Story = args => (
           <Label>Beta</Label>
         </PageHeader.TrailingVisual>
         <PageHeader.TrailingAction hidden={!args.hasTrailingAction}>
-          <IconButton icon={PencilIcon} variant="invisible" />
+          <IconButton aria-label="Edit" icon={PencilIcon} variant="invisible" />
         </PageHeader.TrailingAction>
         <PageHeader.Actions hidden={!args.hasActions}>
           <Hidden on={['narrow']}>