fix(polymorphic): Improve prop passthrough for ActionList.LinkItem and Breadcrumbs.Item

.changeset/curly-moments-show.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': minor
+---
+
+fix(polymorphic): Improve prop passthrough for ActionList.LinkItem and Breadcrumbs.Item

packages/react/src/ActionList/LinkItem.test.tsx

@@ -0,0 +1,89 @@
+import {describe, it, expect, vi} from 'vitest'
+import {render, screen, fireEvent} from '@testing-library/react'
+import React from 'react'
+import {ActionList} from '.'
+import Link from '../Link'
+import {implementsClassName} from '../utils/testing'
+
+describe('ActionList.LinkItem', () => {
+  implementsClassName(ActionList.LinkItem)
+
+  it('renders as an anchor by default', () => {
+    render(
+      <ActionList>
+        <ActionList.LinkItem href="#home">Home</ActionList.LinkItem>
+      </ActionList>,
+    )
+
+    const link = screen.getByRole('link', {name: 'Home'})
+    expect(link).toHaveAttribute('href', '#home')
+    expect(link.tagName).toBe('A')
+  })
+
+  it('calls onClick handler', () => {
+    const onClick = vi.fn()
+
+    render(
+      <ActionList>
+        <ActionList.LinkItem role="link" onClick={onClick}>
+          Home
+        </ActionList.LinkItem>
+      </ActionList>,
+    )
+
+    fireEvent.click(screen.getByRole('link', {name: 'Home'}))
+    expect(onClick).toHaveBeenCalledTimes(1)
+  })
+
+  describe('as prop', () => {
+    it('supports polymorphic LinkItem with as prop', () => {
+      const CustomLink = React.forwardRef<
+        HTMLAnchorElement,
+        React.AnchorHTMLAttributes<HTMLAnchorElement> & {custom: boolean}
+      >(({children, custom, ...props}, ref) => (
+        <a ref={ref} data-custom-link={custom} {...props}>
+          {children}
+        </a>
+      ))
+      CustomLink.displayName = 'CustomLink'
+
+      render(
+        <ActionList>
+          <ActionList.LinkItem as={CustomLink} href="#docs" custom={true}>
+            Docs
+          </ActionList.LinkItem>
+        </ActionList>,
+      )
+
+      const link = screen.getByRole('link', {name: 'Docs'})
+      expect(link).toHaveAttribute('data-custom-link', 'true')
+      expect(link).toHaveAttribute('href', '#docs')
+    })
+
+    it('passes through additional props to the element specified by as', () => {
+      render(
+        <ActionList>
+          <ActionList.LinkItem as={Link} href="#home" data-testid="home-link" inline>
+            Home
+          </ActionList.LinkItem>
+        </ActionList>,
+      )
+
+      const homeLink = screen.getByTestId('home-link')
+      expect(homeLink).toHaveAttribute('href', '#home')
+      expect(homeLink).toHaveAttribute('data-inline', 'true')
+    })
+  })
+
+  it('renders inactive text when inactiveText is provided', () => {
+    render(
+      <ActionList>
+        <ActionList.LinkItem href="#home" inactiveText="Unavailable due to an outage">
+          Home
+        </ActionList.LinkItem>
+      </ActionList>,
+    )
+
+    expect(screen.getByText('Unavailable due to an outage')).toBeInTheDocument()
+  })
+})

packages/react/src/ActionList/LinkItem.tsx

@@ -1,8 +1,10 @@
-import React from 'react'
-import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
+import type React from 'react'
+import type {ForwardedRef} from 'react'
+import type {WithSlotMarker} from '../utils/types/Slots'
 import Link from '../Link'
 import {Item} from './Item'
 import type {ActionListItemProps} from './shared'
+import {type PolymorphicProps, fixedForwardRef} from '../utils/modern-polymorphic'
 
 // adopted from React.AnchorHTMLAttributes
 type LinkProps = {
@@ -25,8 +27,13 @@ export type ActionListLinkItemProps = Pick<
 > &
   LinkProps
 
-export const LinkItem = React.forwardRef(
-  ({active, inactiveText, variant, size, as: Component, className, ...props}, forwardedRef) => {
+type LinkItemProps<As extends React.ElementType = 'a'> = PolymorphicProps<As, 'a', ActionListLinkItemProps>
+
+const LinkItemComponent = fixedForwardRef(
+  <As extends React.ElementType = 'a'>(
+    {active, inactiveText, variant, size, as: Component, className, ...props}: LinkItemProps<As>,
+    forwardedRef: ForwardedRef<unknown>,
+  ) => {
     return (
       <Item
         className={className}
@@ -40,21 +47,29 @@ export const LinkItem = React.forwardRef(
             onClick && onClick(event)
             props.onClick && props.onClick(event as React.MouseEvent<HTMLAnchorElement>)
           }
-          return inactiveText ? (
-            <span {...rest}>{children}</span>
-          ) : (
-            <Link as={Component} {...rest} {...props} onClick={clickHandler} ref={forwardedRef}>
+          if (inactiveText) {
+            return <span {...rest}>{children}</span>
+          }
+
+          // Type safety for the polymorphic `as` prop is enforced at the
+          // LinkItem boundary via fixedForwardRef. Internally we widen
+          // Link's type so TypeScript doesn't re-check the generic
+          // constraint across two polymorphic layers.
+          const InternalLink: React.ElementType = Link
+          return (
+            <InternalLink as={Component} {...rest} {...props} onClick={clickHandler} ref={forwardedRef}>
               {children}
-            </Link>
+            </InternalLink>
           )
         }}
       >
         {props.children}
       </Item>
     )
   },
-) as PolymorphicForwardRefComponent<'a', ActionListLinkItemProps>
-
-LinkItem.displayName = 'ActionList.LinkItem'
+)
 
-LinkItem.__SLOT__ = Symbol('ActionList.LinkItem')
+export const LinkItem: WithSlotMarker<typeof LinkItemComponent> = Object.assign(LinkItemComponent, {
+  displayName: 'ActionList.LinkItem',
+  __SLOT__: Symbol('ActionList.LinkItem'),
+})

packages/react/src/Breadcrumbs/Breadcrumbs.tsx

@@ -11,6 +11,7 @@ import type {ResizeObserverEntry} from '../hooks/useResizeObserver'
 import {useOnEscapePress} from '../hooks/useOnEscapePress'
 import {useOnOutsideClick} from '../hooks/useOnOutsideClick'
 import {useFeatureFlag} from '../FeatureFlags'
+import {type PolymorphicProps, fixedForwardRef} from '../utils/modern-polymorphic'
 
 export type BreadcrumbsProps = React.PropsWithChildren<{
   /**
@@ -118,11 +119,14 @@ const BreadcrumbsMenuItem = React.forwardRef<HTMLDetailsElement, BreadcrumbsMenu
         <div ref={menuContainerRef} className={classes.MenuOverlay}>
           <ActionList>
             {items.map((item, index) => {
-              const href = item.props.href
-              const children = item.props.children
-              const selected = item.props.selected
+              const {children, selected, as: Component, ...itemProps} = item.props
               return (
-                <ActionList.LinkItem key={index} href={href} aria-current={selected ? 'page' : undefined}>
+                <ActionList.LinkItem
+                  key={index}
+                  as={Component}
+                  {...itemProps}
+                  aria-current={selected ? 'page' : undefined}
+                >
                   {children}
                 </ActionList.LinkItem>
               )
@@ -360,41 +364,39 @@ const ItemSeparator = () => {
   )
 }
 
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-type DistributiveOmit<T, TOmitted extends PropertyKey> = T extends any ? Omit<T, TOmitted> : never
-
-type StyledBreadcrumbsItemProps<As extends React.ElementType> = {
-  as?: As
-  to?: To
-  selected?: boolean
-  className?: string
-  style?: React.CSSProperties
-} & DistributiveOmit<React.ComponentPropsWithRef<React.ElementType extends As ? 'a' : As>, 'as'>
-
-function BreadcrumbsItemComponent<As extends React.ElementType>(
-  props: StyledBreadcrumbsItemProps<As>,
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  ref: ForwardedRef<any>,
-) {
-  const {as: Component = 'a', selected, className, ...rest} = props
-  return (
-    <Component
-      className={clsx(className, classes.Item, selected && 'selected')}
-      aria-current={selected ? 'page' : undefined}
-      ref={ref}
-      {...rest}
-    />
-  )
-}
-
-BreadcrumbsItemComponent.displayName = 'Breadcrumbs.Item'
-
-const BreadcrumbsItem = React.forwardRef(BreadcrumbsItemComponent)
+type StyledBreadcrumbsItemProps<As extends React.ElementType = 'a'> = PolymorphicProps<
+  As,
+  'a',
+  {
+    to?: To
+    selected?: boolean
+  }
+>
+
+const BreadcrumbsItem = fixedForwardRef(
+  <As extends React.ElementType = 'a'>(
+    props: StyledBreadcrumbsItemProps<As>,
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    ref: ForwardedRef<any>,
+  ) => {
+    const {as: Component = 'a', selected, className, ...rest} = props
+    return (
+      <Component
+        className={clsx(className, classes.Item, selected && 'selected')}
+        aria-current={selected ? 'page' : undefined}
+        ref={ref}
+        {...rest}
+      />
+    )
+  },
+)
 
 Breadcrumbs.displayName = 'Breadcrumbs'
 
 export type BreadcrumbsItemProps<As extends React.ElementType = 'a'> = StyledBreadcrumbsItemProps<As>
-export default Object.assign(Breadcrumbs, {Item: BreadcrumbsItem})
+
+const BreadcrumbsItemWithDisplayName = Object.assign(BreadcrumbsItem, {displayName: 'Breadcrumbs.Item'})
+export default Object.assign(Breadcrumbs, {Item: BreadcrumbsItemWithDisplayName})
 
 /**
  * @deprecated Use the `Breadcrumbs` component instead (i.e. `<Breadcrumb>` → `<Breadcrumbs>`)

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

@@ -5,6 +5,7 @@ import userEvent from '@testing-library/user-event'
 import {FeatureFlags} from '../../FeatureFlags'
 import {implementsClassName} from '../../utils/testing'
 import classes from '../Breadcrumbs.module.css'
+import Link from '../../Link'
 
 // Helper function to render with theme and feature flags
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -548,4 +549,69 @@ describe('Breadcrumbs', () => {
       expect(container.firstChild).toHaveAttribute('data-variant', 'spacious')
     })
   })
+
+  describe('as prop', () => {
+    it('supports polymorphic Breadcrumbs.Item with as prop', () => {
+      renderWithTheme(
+        <Breadcrumbs>
+          <Breadcrumbs.Item as="span">Home</Breadcrumbs.Item>
+          <Breadcrumbs.Item as="span" selected>
+            Docs
+          </Breadcrumbs.Item>
+        </Breadcrumbs>,
+        {primer_react_breadcrumbs_overflow_menu: true},
+      )
+
+      const homeItem = screen.getByText('Home')
+      const docsItem = screen.getByText('Docs')
+
+      expect(homeItem.nodeName).toEqual('SPAN')
+      expect(docsItem.nodeName).toEqual('SPAN')
+    })
+
+    it('passes through additional props to the element specified by as', () => {
+      renderWithTheme(
+        <Breadcrumbs>
+          <Breadcrumbs.Item as={Link} href="/home" data-testid="home-link" inline>
+            Home
+          </Breadcrumbs.Item>
+        </Breadcrumbs>,
+        {primer_react_breadcrumbs_overflow_menu: true},
+      )
+
+      const homeLink = screen.getByTestId('home-link')
+      expect(homeLink).toHaveAttribute('href', '/home')
+      expect(homeLink).toHaveAttribute('data-inline', 'true')
+    })
+
+    it('passes through additional props to the element specified by as in overflow menu', async () => {
+      const user = userEvent.setup()
+
+      renderWithTheme(
+        <Breadcrumbs overflow="menu">
+          <Breadcrumbs.Item as={Link} href="/home" data-testid="home-link" inline>
+            Home
+          </Breadcrumbs.Item>
+          <Breadcrumbs.Item href="/docs">Docs</Breadcrumbs.Item>
+          <Breadcrumbs.Item href="/components">Components</Breadcrumbs.Item>
+          <Breadcrumbs.Item href="/breadcrumbs">Breadcrumbs</Breadcrumbs.Item>
+          <Breadcrumbs.Item href="/examples">Examples</Breadcrumbs.Item>
+          <Breadcrumbs.Item href="/advanced" selected>
+            Advanced
+          </Breadcrumbs.Item>
+        </Breadcrumbs>,
+        {primer_react_breadcrumbs_overflow_menu: true},
+      )
+
+      // Open the overflow menu
+      const menuButton = screen.getByRole('button', {name: /more breadcrumb items/i})
+      await user.click(menuButton)
+
+      // Verify the custom link in the overflow menu has the correct href
+      await waitFor(() => {
+        const homeLink = screen.getByTestId('home-link')
+        expect(homeLink).toHaveAttribute('href', '/home')
+      })
+    })
+  })
 })

packages/react/src/NavList/NavList.test.tsx

@@ -458,4 +458,45 @@ describe('NavList.ShowMoreItem with pages', () => {
     expect(queryByRole('link', {name: 'Item 6'})).not.toBeInTheDocument()
     expect(queryByRole('link', {name: 'Item 7'})).not.toBeInTheDocument()
   })
+
+  it('passes through as props to the link items', () => {
+    const CustomLink = React.forwardRef<
+      HTMLAnchorElement,
+      React.AnchorHTMLAttributes<HTMLAnchorElement> & {custom: boolean}
+    >(({children, custom, ...props}, ref) => (
+      <a ref={ref} data-custom-link={custom} {...props}>
+        {children}
+      </a>
+    ))
+    CustomLink.displayName = 'CustomLink'
+
+    const {queryByRole} = render(
+      <NavList>
+        <NavList.Item as={CustomLink} href="#item1" custom={true}>
+          Item 1
+        </NavList.Item>
+        <NavList.Item as={CustomLink} href="#item2" custom={false}>
+          Item 2
+        </NavList.Item>
+        <NavList.GroupExpand
+          label="More"
+          items={[
+            {text: 'Item 3', href: '#item3'},
+            {text: 'Item 4', href: '#item4'},
+          ]}
+        />
+      </NavList>,
+    )
+
+    act(() => {
+      queryByRole('button', {name: 'More'})?.click()
+    })
+
+    expect(queryByRole('link', {name: 'Item 1'})).toHaveAttribute('href', '#item1')
+    expect(queryByRole('link', {name: 'Item 1'})).toHaveAttribute('data-custom-link', 'true')
+    expect(queryByRole('link', {name: 'Item 2'})).toHaveAttribute('href', '#item2')
+    expect(queryByRole('link', {name: 'Item 2'})).toHaveAttribute('data-custom-link', 'false')
+    expect(queryByRole('link', {name: 'Item 3'})).toHaveAttribute('href', '#item3')
+    expect(queryByRole('link', {name: 'Item 4'})).toHaveAttribute('href', '#item4')
+  })
 })