Handle Fragment triggers safely in Tooltip

.changeset/strong-tooltips-anchor.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+Tooltip: Prevent React Fragment triggers from crashing and provide a clear development warning

packages/react/src/TooltipV2/Tooltip.tsx

@@ -128,6 +128,11 @@ export const Tooltip: ForwardRefExoticComponent<
   ) => {
     const tooltipId = useId(id)
     const child = Children.only(children)
+    const isFragmentTrigger = React.isValidElement(child) && child.type === React.Fragment
+    warning(
+      isFragmentTrigger,
+      'The `Tooltip` component does not support a React Fragment as its trigger. Pass a single interactive element instead.',
+    )
     const elementChild = React.isValidElement(child) ? child : undefined
     const childRef =
       reactMajorVersion > 18 ? elementChild?.props.ref : (elementChild as {ref?: React.Ref<unknown>} | undefined)?.ref
@@ -153,7 +158,7 @@ export const Tooltip: ForwardRefExoticComponent<
       try {
         if (
           tooltipElRef.current &&
-          readTriggerRef.current &&
+          readTriggerRef.current instanceof HTMLElement &&
           tooltipElRef.current.hasAttribute('popover') &&
           !tooltipElRef.current.matches(':popover-open') &&
           !_privateDisableTooltip
@@ -231,12 +236,19 @@ export const Tooltip: ForwardRefExoticComponent<
 
     useEffect(() => {
       if (!tooltipElRef.current || !readTriggerRef.current) return
+      const trigger = readTriggerRef.current
+      const isInvalidTrigger = !(trigger instanceof HTMLElement)
+      warning(
+        isInvalidTrigger,
+        'The `Tooltip` component expects its trigger ref to resolve to an HTML element. Ensure the trigger forwards its ref to a single interactive element instead of a React Fragment.',
+      )
+      if (isInvalidTrigger) return
       /*
        * ACCESSIBILITY CHECKS
        */
       // Has trigger element or any of its children interactive elements?
-      const isTriggerInteractive = isInteractive(readTriggerRef.current)
-      const triggerChildren = readTriggerRef.current.childNodes
+      const isTriggerInteractive = isInteractive(trigger)
+      const triggerChildren = trigger.childNodes
       // two levels deep
       const hasInteractiveDescendant = Array.from(triggerChildren).some(child => {
         return (
@@ -253,8 +265,8 @@ export const Tooltip: ForwardRefExoticComponent<
       // If the tooltip is used for labelling the interactive element, the trigger element or any of its children should not have aria-label
       // eslint-disable-next-line react-you-might-not-need-an-effect/no-event-handler
       if (type === 'label') {
-        const hasAriaLabel = readTriggerRef.current.hasAttribute('aria-label')
-        const hasAriaLabelInChildren = Array.from(readTriggerRef.current.childNodes).some(
+        const hasAriaLabel = trigger.hasAttribute('aria-label')
+        const hasAriaLabelInChildren = Array.from(trigger.childNodes).some(
           child => child instanceof HTMLElement && child.hasAttribute('aria-label'),
         )
         warning(
@@ -291,6 +303,8 @@ export const Tooltip: ForwardRefExoticComponent<
     // Normalize keybindingHint to an array for uniform rendering
     const keybindingHints = Array.isArray(keybindingHint) ? keybindingHint : [keybindingHint]
 
+    if (isFragmentTrigger) return child
+
     return (
       <TooltipContext.Provider value={value}>
         <>

packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx

@@ -2,7 +2,7 @@ import type React from 'react'
 import {describe, expect, it, vi} from 'vitest'
 import type {TooltipProps} from '../Tooltip'
 import {Tooltip} from '../Tooltip'
-import {render as HTMLRender} from '@testing-library/react'
+import {fireEvent, render as HTMLRender} from '@testing-library/react'
 import BaseStyles from '../../BaseStyles'
 import {Button, IconButton} from '../../Button'
 import {ActionMenu} from '../../ActionMenu'
@@ -12,9 +12,9 @@ import {XIcon} from '@primer/octicons-react'
 import classes from '../Tooltip.module.css'
 
 import type {JSX} from 'react'
-import {createRef} from 'react'
+import {createRef, forwardRef, useImperativeHandle} from 'react'
 import {FeatureFlags} from '../../FeatureFlags'
-import {implementsClassName, withExpectedConsoleError} from '../../utils/testing'
+import {implementsClassName, withExpectedConsoleError, withExpectedConsoleWarning} from '../../utils/testing'
 
 const TooltipComponent = (props: Omit<TooltipProps, 'text'> & {text?: string}) => (
   <Tooltip text="Tooltip text" {...props}>
@@ -31,6 +31,17 @@ const TooltipComponentWithExistingDescription = (props: Omit<TooltipProps, 'text
   </>
 )
 
+const InvalidRefTrigger = forwardRef<HTMLElement, React.ButtonHTMLAttributes<HTMLButtonElement>>(
+  ({children, ...props}, forwardedRef) => {
+    useImperativeHandle(forwardedRef, () => ({}) as HTMLElement, [])
+    return (
+      <button type="button" {...props}>
+        {children}
+      </button>
+    )
+  },
+)
+
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 function ExampleWithActionMenu({actionMenuTrigger}: {actionMenuTrigger: React.ReactElement<any>}): JSX.Element {
   return (
@@ -146,6 +157,20 @@ describe('Tooltip', () => {
       )
     })
   })
+  it('should warn and render the trigger unchanged if it is a React Fragment', () => {
+    withExpectedConsoleWarning(() => {
+      const {getByRole, queryByText} = HTMLRender(
+        <Tooltip text="Tooltip text">
+          <>
+            <Button>Button Text</Button>
+          </>
+        </Tooltip>,
+      )
+
+      expect(getByRole('button', {name: 'Button Text'})).toBeInTheDocument()
+      expect(queryByText('Tooltip text')).not.toBeInTheDocument()
+    })
+  })
   it('does not throw an error when the trigger element is a button in a fieldset', () => {
     const {getByRole} = HTMLRender(
       <fieldset>
@@ -288,6 +313,20 @@ describe('Tooltip forwarded ref (primer_react_merged_forwarded_refs)', () => {
         expect(buttonRefCallback).toHaveBeenCalledWith(button)
         expect(tooltipRefCallback).toHaveBeenCalledWith(button)
       })
+
+      it('warns without crashing when a custom trigger ref does not resolve to an HTML element', () => {
+        withExpectedConsoleWarning(() => {
+          const {getByRole} = HTMLRender(
+            <FeatureFlags flags={{primer_react_merged_forwarded_refs: enabled}}>
+              <Tooltip text="Tooltip text">
+                <InvalidRefTrigger>Button Text</InvalidRefTrigger>
+              </Tooltip>
+            </FeatureFlags>,
+          )
+
+          expect(() => fireEvent.focus(getByRole('button', {name: 'Button Text'}))).not.toThrow()
+        })
+      })
     })
   }
 })