Merge ac0688b into ece5160

Author: iansan5653

contributor-docs/behaviors.md

@@ -44,7 +44,7 @@ Generic behaviors provide functionality that is not specific to any single compo
 
 ### Examples
 
-- `useProvidedRefOrCreate`
+- `useCombinedRefs`
 - `usePosition`
 - `useClickAway`
 - `useTypeAhead`
@@ -123,7 +123,7 @@ There are no restrictions on return values of generic behavior hooks. In fact, s
 
 - Often, a behavior will need to act on a real DOM element.
 - In this case, the hook should return a ref as part of the returned props for that element. The ref will get spread onto the element, giving the ref access to it.
-- Whenever you need a ref, it must be accepted as an optional setting to the hook. The hook then uses the `useProvidedRefOrCreate` hook to resolve a usable ref. Remember to return the resulting ref from the hook.
+- Whenever you need a ref, it must be accepted as an optional setting to the hook.
 
 ## Testing behaviors
 

packages/react/src/ActionList/List.tsx

@@ -1,11 +1,11 @@
-import React, {type JSX} from 'react'
+import React, {useRef, type JSX} from 'react'
 import {fixedForwardRef} from '../utils/modern-polymorphic'
 import {ActionListContainerContext} from './ActionListContainerContext'
 import {useSlots} from '../hooks/useSlots'
 import {Heading} from './Heading'
 import {useId} from '../hooks/useId'
 import {ListContext, type ActionListProps} from './shared'
-import {useProvidedRefOrCreate} from '../hooks'
+import {useCombinedRefs} from '../hooks'
 import {FocusKeys, useFocusZone} from '../hooks/useFocusZone'
 import {clsx} from 'clsx'
 import classes from './ActionList.module.css'
@@ -41,7 +41,8 @@ const UnwrappedList = <As extends React.ElementType = 'ul'>(
 
   const ariaLabelledBy = slots.heading ? (slots.heading.props.id ?? headingId) : listLabelledBy
   const listRole = role || listRoleFromContainer
-  const listRef = useProvidedRefOrCreate(forwardedRef as React.RefObject<HTMLUListElement>)
+  const listRef = useRef<HTMLElement>(null)
+  const combinedRef = useCombinedRefs(forwardedRef, listRef)
 
   let enableFocusZone = false
   if (enableFocusZoneFromContainer !== undefined) enableFocusZone = enableFocusZoneFromContainer
@@ -66,12 +67,11 @@ const UnwrappedList = <As extends React.ElementType = 'ul'>(
       }}
     >
       {slots.heading}
-      {/* @ts-expect-error ref needs a non nullable ref */}
       <Component
         className={clsx(classes.ActionList, className)}
         role={listRole}
         aria-labelledby={ariaLabelledBy}
-        ref={listRef}
+        ref={combinedRef}
         data-dividers={showDividers}
         data-variant={variant}
         {...restProps}

packages/react/src/ActionMenu/ActionMenu.tsx

@@ -1,9 +1,9 @@
-import React, {useCallback, useContext, useMemo, useEffect, useState} from 'react'
+import React, {useCallback, useContext, useMemo, useEffect, useState, useRef} from 'react'
 import {TriangleDownIcon, ChevronRightIcon} from '@primer/octicons-react'
 import type {AnchoredOverlayProps} from '../AnchoredOverlay'
 import {AnchoredOverlay} from '../AnchoredOverlay'
 import type {OverlayProps} from '../Overlay'
-import {useProvidedRefOrCreate, useProvidedStateOrCreate, useMenuKeyboardNavigation} from '../hooks'
+import {useProvidedStateOrCreate, useMenuKeyboardNavigation, useCombinedRefs} from '../hooks'
 import {Divider} from '../ActionList/Divider'
 import {ActionListContainerContext} from '../ActionList/ActionListContainerContext'
 import type {ButtonProps} from '../Button'
@@ -110,7 +110,9 @@ const Menu: FCWithSlotMarker<React.PropsWithChildren<ActionMenuProps>> = ({
   )
   const menuButtonChildId = React.isValidElement(menuButtonChild) ? menuButtonChild.props.id : undefined
 
-  const anchorRef = useProvidedRefOrCreate(externalAnchorRef)
+  const anchorRef = useRef<HTMLElement>(null)
+  const combinedRef = useCombinedRefs(anchorRef, externalAnchorRef)
+
   const anchorId = useId(menuButtonChildId)
   let renderAnchor: AnchoredOverlayProps['renderAnchor'] = null
   // 🚨 Hack for good API!
@@ -130,7 +132,7 @@ const Menu: FCWithSlotMarker<React.PropsWithChildren<ActionMenuProps>> = ({
             anchorChildren,
             mergeAnchorHandlers({...anchorProps}, anchorChildren.props),
           )
-          return React.cloneElement(child, {children: triggerButton, ref: anchorRef})
+          return React.cloneElement(child, {children: triggerButton, ref: combinedRef})
         }
       }
       return null
@@ -149,7 +151,7 @@ const Menu: FCWithSlotMarker<React.PropsWithChildren<ActionMenuProps>> = ({
               mergeAnchorHandlers({...anchorProps}, tooltipTrigger.props),
             )
             const tooltip = React.cloneElement(anchorChildren, {children: tooltipTriggerEl})
-            return React.cloneElement(child, {children: tooltip, ref: anchorRef})
+            return React.cloneElement(child, {children: tooltip, ref: combinedRef})
           }
         }
       } else {
@@ -278,7 +280,7 @@ const Overlay: FCWithSlotMarker<React.PropsWithChildren<MenuOverlayProps>> = ({
   // we typecast anchorRef as required instead of optional
   // because we know that we're setting it in context in Menu
   const {
-    anchorRef,
+    anchorRef: contextAnchorRef,
     renderAnchor,
     anchorId,
     open,
@@ -287,6 +289,9 @@ const Overlay: FCWithSlotMarker<React.PropsWithChildren<MenuOverlayProps>> = ({
     isSubmenu = false,
   } = React.useContext(MenuContext) as MandateProps<MenuContextProps, 'anchorRef'>
 
+  const anchorRef = useRef<HTMLElement>(null)
+  const combinedAnchorRef = useCombinedRefs(anchorRef, contextAnchorRef)
+
   const containerRef = React.useRef<HTMLDivElement>(null)
   const isNarrow = useResponsiveValue({narrow: true}, false)
 
@@ -328,7 +333,7 @@ const Overlay: FCWithSlotMarker<React.PropsWithChildren<MenuOverlayProps>> = ({
 
   return (
     <AnchoredOverlay
-      anchorRef={anchorRef}
+      anchorRef={combinedAnchorRef}
       renderAnchor={renderAnchor}
       anchorId={anchorId}
       open={open}

packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx

@@ -1,12 +1,12 @@
 import type React from 'react'
-import {useCallback, useEffect, type JSX} from 'react'
+import {useCallback, useEffect, useRef, type JSX} from 'react'
 import type {OverlayProps} from '../Overlay'
 import Overlay from '../Overlay'
 import type {FocusTrapHookSettings} from '../hooks/useFocusTrap'
 import {useFocusTrap} from '../hooks/useFocusTrap'
 import type {FocusZoneHookSettings} from '../hooks/useFocusZone'
 import {useFocusZone} from '../hooks/useFocusZone'
-import {useAnchoredPosition, useProvidedRefOrCreate, useRenderForcingRef} from '../hooks'
+import {useAnchoredPosition, useCombinedRefs, useRenderForcingRef} from '../hooks'
 import {useId} from '../hooks/useId'
 import type {AnchorPosition, PositionSettings} from '@primer/behaviors'
 import {type ResponsiveValue} from '../hooks/useResponsiveValue'
@@ -27,7 +27,7 @@ interface AnchoredOverlayPropsWithAnchor {
   /**
    * An override to the internal ref that will be spread on to the renderAnchor
    */
-  anchorRef?: React.RefObject<HTMLElement | null>
+  anchorRef?: React.Ref<HTMLElement | null>
 
   /**
    * An override to the internal id that will be spread on to the renderAnchor
@@ -46,7 +46,7 @@ interface AnchoredOverlayPropsWithoutAnchor {
    * An override to the internal renderAnchor ref that will be used to position the overlay.
    * When renderAnchor is null this can be used to make an anchor that is detached from ActionMenu.
    */
-  anchorRef: React.RefObject<HTMLElement | null>
+  anchorRef: React.Ref<HTMLElement | null>
   /**
    * An override to the internal id that will be spread on to the renderAnchor
    */
@@ -160,8 +160,12 @@ export const AnchoredOverlay: React.FC<React.PropsWithChildren<AnchoredOverlayPr
   displayCloseButton = true,
   closeButtonProps = defaultCloseButtonProps,
 }) => {
-  const anchorRef = useProvidedRefOrCreate(externalAnchorRef)
+  const anchorRef = useRef<HTMLElement>(null)
+  const combinedRef = useCombinedRefs(anchorRef, externalAnchorRef)
+
   const [overlayRef, updateOverlayRef] = useRenderForcingRef<HTMLDivElement>()
+  const combinedOverlayRef = useCombinedRefs(updateOverlayRef, overlayProps?.ref)
+
   const anchorId = useId(externalAnchorId)
 
   const onClickOutside = useCallback(() => onClose?.('click-outside'), [onClose])
@@ -235,7 +239,7 @@ export const AnchoredOverlay: React.FC<React.PropsWithChildren<AnchoredOverlayPr
     <>
       {renderAnchor &&
         renderAnchor({
-          ref: anchorRef,
+          ref: combinedRef,
           id: anchorId,
           'aria-haspopup': 'true',
           'aria-expanded': open,
@@ -261,12 +265,7 @@ export const AnchoredOverlay: React.FC<React.PropsWithChildren<AnchoredOverlayPr
           preventOverflow={preventOverflow}
           data-component="AnchoredOverlay"
           {...overlayProps}
-          ref={node => {
-            if (overlayProps?.ref) {
-              assignRef(overlayProps.ref, node)
-            }
-            updateOverlayRef(node)
-          }}
+          ref={combinedOverlayRef}
         >
           {showXIcon ? (
             <div className={classes.ResponsiveCloseButtonContainer}>
@@ -293,15 +292,4 @@ export const AnchoredOverlay: React.FC<React.PropsWithChildren<AnchoredOverlayPr
   )
 }
 
-function assignRef<T>(
-  ref: React.MutableRefObject<T | null> | ((instance: T | null) => void) | null | undefined,
-  value: T | null,
-) {
-  if (typeof ref === 'function') {
-    ref(value)
-  } else if (ref) {
-    ref.current = value
-  }
-}
-
 AnchoredOverlay.displayName = 'AnchoredOverlay'

packages/react/src/ButtonGroup/ButtonGroup.tsx

@@ -1,8 +1,8 @@
-import React, {type PropsWithChildren} from 'react'
+import React, {useRef, type PropsWithChildren} from 'react'
 import classes from './ButtonGroup.module.css'
 import {clsx} from 'clsx'
 import {FocusKeys, useFocusZone} from '../hooks/useFocusZone'
-import {useProvidedRefOrCreate} from '../hooks'
+import {useCombinedRefs} from '../hooks'
 import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
 
 export type ButtonGroupProps = PropsWithChildren<{
@@ -17,7 +17,8 @@ const ButtonGroup = React.forwardRef(function ButtonGroup(
   forwardRef,
 ) {
   const buttons = React.Children.map(children, (child, index) => <div key={index}>{child}</div>)
-  const buttonRef = useProvidedRefOrCreate(forwardRef as React.RefObject<HTMLDivElement | null>)
+  const buttonRef = useRef<HTMLDivElement>(null)
+  const combinedRef = useCombinedRefs(buttonRef, forwardRef)
 
   useFocusZone({
     containerRef: buttonRef,
@@ -27,8 +28,7 @@ const ButtonGroup = React.forwardRef(function ButtonGroup(
   })
 
   return (
-    //@ts-expect-error it needs a non nullable ref
-    <BaseComponent ref={buttonRef} className={clsx(className, classes.ButtonGroup)} role={role} {...rest}>
+    <BaseComponent ref={combinedRef} className={clsx(className, classes.ButtonGroup)} role={role} {...rest}>
       {buttons}
     </BaseComponent>
   )

packages/react/src/Checkbox/Checkbox.tsx

@@ -1,6 +1,13 @@
 import {clsx} from 'clsx'
-import {useProvidedRefOrCreate} from '../hooks'
-import React, {useContext, useEffect, type ChangeEventHandler, type InputHTMLAttributes, type ReactElement} from 'react'
+import {useCombinedRefs} from '../hooks'
+import React, {
+  useContext,
+  useEffect,
+  useRef,
+  type ChangeEventHandler,
+  type InputHTMLAttributes,
+  type ReactElement,
+} from 'react'
 import useLayoutEffect from '../utils/useIsomorphicLayoutEffect'
 import type {FormValidationStatus} from '../utils/types/FormValidationStatus'
 import {CheckboxGroupContext} from '../CheckboxGroup/CheckboxGroupContext'
@@ -45,7 +52,8 @@ const Checkbox = React.forwardRef<HTMLInputElement, CheckboxProps>(
     ref,
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
   ): ReactElement<any> => {
-    const checkboxRef = useProvidedRefOrCreate(ref as React.RefObject<HTMLInputElement>)
+    const checkboxRef = useRef<HTMLInputElement>(null)
+    const combinedRef = useCombinedRefs(checkboxRef, ref as React.RefObject<HTMLInputElement>)
     const checkboxGroupContext = useContext(CheckboxGroupContext)
     const handleOnChange: ChangeEventHandler<HTMLInputElement> = e => {
       checkboxGroupContext.onChange && checkboxGroupContext.onChange(e)
@@ -54,7 +62,7 @@ const Checkbox = React.forwardRef<HTMLInputElement, CheckboxProps>(
     const inputProps = {
       type: 'checkbox',
       disabled,
-      ref: checkboxRef,
+      ref: combinedRef,
       checked: indeterminate ? false : checked,
       defaultChecked,
       required,
@@ -84,7 +92,7 @@ const Checkbox = React.forwardRef<HTMLInputElement, CheckboxProps>(
         checkbox.setAttribute('aria-checked', checkbox.checked ? 'true' : 'false')
       }
     })
-    // @ts-expect-error inputProp needs a non nullable ref
+
     return <input {...inputProps} className={clsx(className, sharedClasses.Input, classes.Checkbox)} />
   },
 )

packages/react/src/Dialog/Dialog.tsx

@@ -1,7 +1,7 @@
 import React, {useCallback, useEffect, useRef, useState, type SyntheticEvent} from 'react'
 import type {ButtonProps} from '../Button'
 import {Button, IconButton} from '../Button'
-import {useOnEscapePress, useProvidedRefOrCreate} from '../hooks'
+import {useOnEscapePress} from '../hooks'
 import {useFocusTrap} from '../hooks/useFocusTrap'
 import {XIcon} from '@primer/octicons-react'
 import {useFocusZone} from '../hooks/useFocusZone'
@@ -426,7 +426,8 @@ const Footer = React.forwardRef<HTMLDivElement, StyledFooterProps>(function Foot
 Footer.displayName = 'Dialog.Footer'
 
 const Buttons: React.FC<React.PropsWithChildren<{buttons: DialogButtonProps[]}>> = ({buttons}) => {
-  const autoFocusRef = useProvidedRefOrCreate<HTMLButtonElement>(buttons.find(button => button.autoFocus)?.ref)
+  const autoFocusRef = useRef<HTMLButtonElement>(null)
+  const combinedRef = useCombinedRefs(autoFocusRef, buttons.find(button => button.autoFocus)?.ref)
   let autoFocusCount = 0
   const [hasRendered, setHasRendered] = useState(0)
   useEffect(() => {
@@ -448,8 +449,7 @@ const Buttons: React.FC<React.PropsWithChildren<{buttons: DialogButtonProps[]}>>
             {...buttonProps}
             // 'normal' value is equivalent to 'default', this is used for backwards compatibility
             variant={buttonType === 'normal' ? 'default' : buttonType}
-            // @ts-expect-error it needs a non nullable ref
-            ref={autoFocus && autoFocusCount === 0 ? (autoFocusCount++, autoFocusRef) : null}
+            ref={autoFocus && autoFocusCount === 0 ? (autoFocusCount++, combinedRef) : null}
           >
             {content}
           </Button>

packages/react/src/FilteredActionList/FilteredActionList.tsx

@@ -9,7 +9,6 @@ import {ActionList, type ActionListProps} from '../ActionList'
 import type {GroupedListProps, ListPropsBase, ItemInput, RenderItemFn} from './'
 import {useFocusZone} from '../hooks/useFocusZone'
 import {useId} from '../hooks/useId'
-import {useProvidedRefOrCreate} from '../hooks/useProvidedRefOrCreate'
 import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
 import useScrollFlash from '../hooks/useScrollFlash'
 import {VisuallyHidden} from '../VisuallyHidden'
@@ -22,6 +21,7 @@ import {isValidElementType} from 'react-is'
 import {useAnnouncements} from './useAnnouncements'
 import {clsx} from 'clsx'
 import {useVirtualizer} from '@tanstack/react-virtual'
+import {useCombinedRefs} from '../hooks'
 
 const menuScrollMargins: ScrollIntoViewOptions = {startMargin: 0, endMargin: 8}
 
@@ -189,10 +189,11 @@ export function FilteredActionList({
   const inputAndListContainerRef = useRef<HTMLDivElement>(null)
   const listRef = useRef<HTMLUListElement>(null)
 
-  const scrollContainerRef = useProvidedRefOrCreate<HTMLDivElement>(
-    providedScrollContainerRef as React.RefObject<HTMLDivElement>,
-  )
-  const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
+  const scrollContainerRef = useRef<HTMLDivElement>(null)
+  const combinedScrollContainerRef = useCombinedRefs(scrollContainerRef, providedScrollContainerRef)
+
+  const inputRef = useRef<HTMLInputElement>(null)
+  const combinedInputRef = useCombinedRefs(inputRef, providedInputRef)
 
   const usingRovingTabindex = _PrivateFocusManagement === 'roving-tabindex'
   const [listContainerElement, setListContainerElement] = useState<HTMLUListElement | null>(null)
@@ -548,8 +549,7 @@ export function FilteredActionList({
     <div ref={inputAndListContainerRef} className={clsx(className, classes.Root)} data-testid="filtered-action-list">
       <div className={classes.Header}>
         <TextInput
-          // @ts-expect-error it needs a non nullable ref
-          ref={inputRef}
+          ref={combinedInputRef}
           block
           width="auto"
           color="fg.default"
@@ -585,8 +585,7 @@ export function FilteredActionList({
           </label>
         </div>
       )}
-      {/* @ts-expect-error div needs a non nullable ref */}
-      <div ref={scrollContainerRef} className={classes.Container}>
+      <div ref={combinedScrollContainerRef} className={classes.Container}>
         {getBodyContent()}
       </div>
     </div>