Update types for IconButton, ActionMenu.Button, TabNav.Link and inter…

…nal consumers (#2677)

* update types for many buttony things

* update anchor type

* changeset

* avoid a type assignment for button component

* use default sx propr broader

* slight type tweak

* fixup invalid types

* scope readonly to dev

* update tabnavlink to work with button props

* explitly allow href, even if as doesnt support it

* lint

* fix unused

* fix types

* remove a prop re-added in a merge

.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

@@ -8,6 +8,7 @@ import {ActionListContainerContext} from './ActionList/ActionListContainerContex
 import {Button, ButtonProps} from './Button'
 import {useId} from './hooks/useId'
 import {MandateProps} from './utils/types'
+import {ForwardRefComponent as PolymorphicForwardRefComponent} from './utils/polymorphic'
 
 export type MenuContextProps = Pick<
   AnchoredOverlayProps,
@@ -67,21 +68,19 @@ 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>(({...props}, anchorRef) => {
+const MenuButton = React.forwardRef((props, anchorRef) => {
   return (
     <Anchor ref={anchorRef}>
       <Button type="button" trailingAction={TriangleDownIcon} {...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,15 +1,14 @@
 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, getButtonStyles, getAlignContentSize} from './styles'
 import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef'
-declare let __DEV__: boolean
+import {defaultSxProp} from '../utils/defaultSxProp'
 
-const defaultSxProp = {}
-const ButtonBase = forwardRef<HTMLElement, ButtonProps>(
+const ButtonBase = forwardRef(
   ({children, as: Component = 'button', sx: sxProp = defaultSxProp, ...props}, forwardedRef): JSX.Element => {
     const {
       leadingIcon: LeadingIcon,
@@ -22,15 +21,15 @@ const ButtonBase = forwardRef<HTMLElement, ButtonProps>(
       ...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), getVariantStyles(variant, theme)])
     }, [theme, variant])
     const sxStyles = useMemo(() => {
-      return merge(baseStyles, sxProp as SxProp)
+      return merge<BetterSystemStyleObject>(baseStyles, sxProp)
     }, [baseStyles, sxProp])
     const iconWrapStyles = {
       display: 'flex',
@@ -46,7 +45,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

@@ -1,18 +1,27 @@
 import React, {forwardRef} from 'react'
-import {merge, SxProp} from '../sx'
+import {merge, BetterSystemStyleObject} from '../sx'
 import {useTheme} from '../ThemeProvider'
 import {IconButtonProps, StyledButton} from './types'
 import {getBaseStyles, 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), getVariantStyles(variant, theme), sxProp as SxProp])
+  const sxStyles = merge.all<BetterSystemStyleObject>([getBaseStyles(theme), getVariantStyles(variant, theme), sxProp])
   return (
-    <StyledButton data-component="IconButton" data-size={size} sx={sxStyles} {...rest} ref={forwardedRef}>
+    <StyledButton
+      data-component="IconButton"
+      data-size={size}
+      sx={sxStyles}
+      {...rest}
+      // @ts-expect-error StyledButton wants both Anchor and Button refs
+      ref={forwardedRef}
+    >
       <Icon />
     </StyledButton>
   )
-})
+}) as PolymorphicForwardRefComponent<'button' | 'a', IconButtonProps>
 
 export {IconButton}

src/Button/LinkButton.tsx

@@ -1,20 +1,27 @@
 import React, {forwardRef} from 'react'
-import {merge, SxProp} from '../sx'
+import {BetterSystemStyleObject, merge} 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 sxStyle = merge.all([sx as SxProp])
+const LinkButton = forwardRef(
+  ({children, as: Component = 'a', sx = defaultSxProp, ...props}, forwardedRef): JSX.Element => {
+    const sxStyle = merge.all<BetterSystemStyleObject>([sx])
     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'
 
@@ -15,14 +14,9 @@ export type Size = 'small' | 'medium' | 'large'
 
 export type AlignContent = 'start' | 'center'
 
-/**
- * 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 = {
   /**
@@ -42,22 +36,21 @@ export type ButtonBaseProps = {
    */
   block?: 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
   /**
    * Trailing action appears to the right of the trailing visual and is always locked to the end
    */
-  trailingAction?: React.FunctionComponent<React.PropsWithChildren<IconProps>>
+  trailingAction?: React.ComponentType | null | undefined
   children: React.ReactNode
   /**
    * Content alignment for when visuals are present
@@ -66,8 +59,8 @@ export type ButtonProps = {
 } & 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']}>

src/PageHeader/PageHeader.test.tsx

@@ -91,11 +91,11 @@ describe('PageHeader', () => {
         <PageHeader.ContextArea>ContextArea</PageHeader.ContextArea>
         <PageHeader.TitleArea>
           <PageHeader.LeadingAction>
-            <IconButton data-testid="LeadingAction" icon={SidebarExpandIcon} variant="invisible" />
+            <IconButton aria-label="Expand" data-testid="LeadingAction" icon={SidebarExpandIcon} variant="invisible" />
           </PageHeader.LeadingAction>
           <PageHeader.Title>Title</PageHeader.Title>
           <PageHeader.TrailingAction>
-            <IconButton data-testid="TrailingAction" icon={PencilIcon} variant="invisible" />
+            <IconButton aria-label="edit" data-testid="TrailingAction" icon={PencilIcon} variant="invisible" />
           </PageHeader.TrailingAction>
         </PageHeader.TitleArea>
         <PageHeader.Description></PageHeader.Description>
@@ -129,7 +129,7 @@ describe('PageHeader', () => {
         <PageHeader.TitleArea variant="large">
           <PageHeader.LeadingAction>
             Leading Action
-            <IconButton icon={SidebarExpandIcon} variant="invisible" />
+            <IconButton aria-label="expand" icon={SidebarExpandIcon} variant="invisible" />
           </PageHeader.LeadingAction>
           <PageHeader.LeadingVisual>
             Leading Visual
@@ -138,7 +138,7 @@ describe('PageHeader', () => {
           <PageHeader.Title>Title</PageHeader.Title>
           <PageHeader.TrailingAction>
             Trailing Action
-            <IconButton icon={PencilIcon} variant="invisible" />
+            <IconButton aria-label="edit" icon={PencilIcon} variant="invisible" />
           </PageHeader.TrailingAction>
           <PageHeader.TrailingVisual>
             Trailing Visual