diff --git a/.changeset/red-wombats-whisper.md b/.changeset/red-wombats-whisper.md new file mode 100644 index 00000000000..67707fad452 --- /dev/null +++ b/.changeset/red-wombats-whisper.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +Replace `useCombinedRefs` with `useRefObjectAsForwardedRef` diff --git a/src/Autocomplete/AutocompleteInput.tsx b/src/Autocomplete/AutocompleteInput.tsx index 6db72d4f20e..d1d42a0851a 100644 --- a/src/Autocomplete/AutocompleteInput.tsx +++ b/src/Autocomplete/AutocompleteInput.tsx @@ -2,7 +2,6 @@ import React, { ChangeEventHandler, FocusEventHandler, KeyboardEventHandler, - MutableRefObject, useCallback, useContext, useEffect, @@ -11,7 +10,7 @@ import React, { import {ForwardRefComponent as PolymorphicForwardRefComponent} from '@radix-ui/react-polymorphic' import {AutocompleteContext} from './AutocompleteContext' import TextInput from '../TextInput' -import {useCombinedRefs} from '../hooks/useCombinedRefs' +import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef' import {ComponentProps} from '../utils/types' type InternalAutocompleteInputProps = { @@ -39,7 +38,7 @@ const AutocompleteInput = React.forwardRef( setShowMenu, showMenu } = autocompleteContext - const combinedInputRef = useCombinedRefs(inputRef, forwardedRef) + useRefObjectAsForwardedRef(forwardedRef, inputRef) const [highlightRemainingText, setHighlightRemainingText] = useState(true) const handleInputFocus: FocusEventHandler = useCallback( @@ -58,12 +57,12 @@ const AutocompleteInput = React.forwardRef( // this prevents the menu from hiding when the user is clicking an option in the Autoselect.Menu, // but still hides the menu when the user blurs the input by tabbing out or clicking somewhere else on the page setTimeout(() => { - if (document.activeElement !== combinedInputRef.current) { + if (document.activeElement !== inputRef.current) { setShowMenu(false) } }, 0) }, - [onBlur, setShowMenu, combinedInputRef] + [onBlur, setShowMenu, inputRef] ) const handleInputChange: ChangeEventHandler = useCallback( @@ -157,7 +156,7 @@ const AutocompleteInput = React.forwardRef( onKeyDown={handleInputKeyDown} onKeyPress={onInputKeyPress} onKeyUp={handleInputKeyUp} - ref={combinedInputRef as MutableRefObject} + ref={inputRef} aria-controls={`${id}-listbox`} aria-autocomplete="both" role="combobox" diff --git a/src/Autocomplete/AutocompleteOverlay.tsx b/src/Autocomplete/AutocompleteOverlay.tsx index f1150b9256a..74ef2e5f77d 100644 --- a/src/Autocomplete/AutocompleteOverlay.tsx +++ b/src/Autocomplete/AutocompleteOverlay.tsx @@ -3,7 +3,7 @@ import {useAnchoredPosition} from '../hooks' import Overlay, {OverlayProps} from '../Overlay' import {ComponentProps} from '../utils/types' import {AutocompleteContext} from './AutocompleteContext' -import {useCombinedRefs} from '../hooks/useCombinedRefs' +import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef' type AutocompleteOverlayInternalProps = { /** @@ -39,7 +39,7 @@ function AutocompleteOverlay({ [showMenu, selectedItemLength] ) - const combinedOverlayRef = useCombinedRefs(scrollContainerRef, floatingElementRef) + useRefObjectAsForwardedRef(scrollContainerRef, floatingElementRef) const closeOptionList = useCallback(() => { setShowMenu(false) @@ -55,7 +55,7 @@ function AutocompleteOverlay({ preventFocusOnOpen={true} onClickOutside={closeOptionList} onEscape={closeOptionList} - ref={combinedOverlayRef as React.RefObject} + ref={floatingElementRef as React.RefObject} top={position?.top} left={position?.left} visibility={showMenu ? 'visible' : 'hidden'} diff --git a/src/Dialog.tsx b/src/Dialog.tsx index 05b078d95b2..4ae084b7640 100644 --- a/src/Dialog.tsx +++ b/src/Dialog.tsx @@ -7,7 +7,7 @@ import useDialog from './hooks/useDialog' import sx, {SxProp} from './sx' import Text from './Text' import {ComponentProps} from './utils/types' -import {useCombinedRefs} from './hooks/useCombinedRefs' +import {useRefObjectAsForwardedRef} from './hooks/useRefObjectAsForwardedRef' const noop = () => null @@ -95,7 +95,8 @@ type InternalDialogProps = { const Dialog = forwardRef( ({children, onDismiss = noop, isOpen, initialFocusRef, returnFocusRef, ...props}, forwardedRef) => { const overlayRef = useRef(null) - const modalRef = useCombinedRefs(forwardedRef) + const modalRef = useRef(null) + useRefObjectAsForwardedRef(forwardedRef, modalRef) const closeButtonRef = useRef(null) const onCloseClick = () => { diff --git a/src/Dialog/Dialog.tsx b/src/Dialog/Dialog.tsx index 58ec4428240..e9850c35f6d 100644 --- a/src/Dialog/Dialog.tsx +++ b/src/Dialog/Dialog.tsx @@ -11,7 +11,7 @@ import {XIcon} from '@primer/octicons-react' import {useFocusZone} from '../hooks/useFocusZone' import {FocusKeys} from '@primer/behaviors' import Portal from '../Portal' -import {useCombinedRefs} from '../hooks/useCombinedRefs' +import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef' import {useSSRSafeId} from '@react-aria/ssr' const ANIMATION_DURATION = '200ms' @@ -274,7 +274,7 @@ const _Dialog = React.forwardRef(null) - const combinedRef = useCombinedRefs(dialogRef, forwardedRef) + useRefObjectAsForwardedRef(forwardedRef, dialogRef) const backdropRef = useRef(null) useFocusTrap({containerRef: dialogRef, restoreFocusOnCleanUp: true, initialFocusRef: autoFocusedFooterButtonRef}) @@ -297,7 +297,7 @@ const _Dialog = React.forwardRef( forwardedRef ): ReactElement => { const overlayRef = useRef(null) - const combinedRef = useCombinedRefs(overlayRef, forwardedRef) + useRefObjectAsForwardedRef(forwardedRef, overlayRef) const {theme} = useTheme() const slideAnimationDistance = parseInt(get('space.2')(theme).replace('px', '')) const slideAnimationEasing = get('animation.easeOutCubic')(theme) @@ -158,10 +158,10 @@ const Overlay = React.forwardRef( }) useEffect(() => { - if (height === 'initial' && combinedRef.current?.clientHeight) { - combinedRef.current.style.height = `${combinedRef.current.clientHeight}px` + if (height === 'initial' && overlayRef.current?.clientHeight) { + overlayRef.current.style.height = `${overlayRef.current.clientHeight}px` } - }, [height, combinedRef]) + }, [height]) useLayoutEffect(() => { const {x, y} = getSlideAnimationStartingVector(anchorSide) @@ -185,7 +185,7 @@ const Overlay = React.forwardRef( height={height} role={role} {...rest} - ref={combinedRef} + ref={overlayRef} style={ { top: `${top || 0}px`, diff --git a/src/TextInputWithTokens.tsx b/src/TextInputWithTokens.tsx index 1039acbcd94..6cac39ff3b5 100644 --- a/src/TextInputWithTokens.tsx +++ b/src/TextInputWithTokens.tsx @@ -3,8 +3,7 @@ import {isFocusable} from '@primer/behaviors/utils' import {omit} from '@styled-system/props' import React, {FocusEventHandler, KeyboardEventHandler, MouseEventHandler, RefObject, useRef, useState} from 'react' import Box from './Box' -import {useProvidedRefOrCreate} from './hooks' -import {useCombinedRefs} from './hooks/useCombinedRefs' +import {useRefObjectAsForwardedRef} from './hooks/useRefObjectAsForwardedRef' import {useFocusZone} from './hooks/useFocusZone' import Text from './Text' import {TextInputProps} from './TextInput' @@ -93,12 +92,11 @@ function TextInputWithTokensInnerComponent, - externalRef: React.ForwardedRef + forwardedRef: React.ForwardedRef ) { const {onBlur, onFocus, onKeyDown, ...inputPropsRest} = omit(rest) - const ref = useProvidedRefOrCreate(externalRef as React.RefObject) - const localInputRef = useRef(null) - const combinedInputRef = useCombinedRefs(localInputRef, ref) + const ref = useRef(null) + useRefObjectAsForwardedRef(forwardedRef, ref) const [selectedTokenIndex, setSelectedTokenIndex] = useState() const [tokensAreTruncated, setTokensAreTruncated] = useState(Boolean(visibleTokenCount)) const {containerRef} = useFocusZone( @@ -124,7 +122,7 @@ function TextInputWithTokensInnerComponent tokens.length || nextIndex < 1) { - return combinedInputRef.current || undefined + return ref.current || undefined } return containerRef.current?.children[nextIndex] as HTMLElement @@ -230,7 +228,7 @@ function TextInputWithTokensInnerComponent { - combinedInputRef.current?.focus() + ref.current?.focus() } const preventTokenClickPropagation: MouseEventHandler = event => { @@ -323,7 +321,7 @@ function TextInputWithTokensInnerComponent ) => { - const inputRef = useCombinedRefs(children.ref) + const inputRef = useRef(null) + useRefObjectAsForwardedRef(children.ref ?? noop, inputRef) + const externalInput = requireChildrenToBeInput(children, inputRef) const emitSyntheticChange = useSyntheticChange({ diff --git a/src/hooks/index.ts b/src/hooks/index.ts index a12912fc1c8..97de8c06c0d 100644 --- a/src/hooks/index.ts +++ b/src/hooks/index.ts @@ -13,3 +13,4 @@ export {useProvidedStateOrCreate} from './useProvidedStateOrCreate' export {useMenuInitialFocus} from './useMenuInitialFocus' export {useMenuKeyboardNavigation} from './useMenuKeyboardNavigation' export {useMnemonics} from './useMnemonics' +export {useRefObjectAsForwardedRef} from './useRefObjectAsForwardedRef' diff --git a/src/hooks/useCombinedRefs.ts b/src/hooks/useCombinedRefs.ts deleted file mode 100644 index 8eb341ea1a6..00000000000 --- a/src/hooks/useCombinedRefs.ts +++ /dev/null @@ -1,40 +0,0 @@ -import {ForwardedRef, useRef} from 'react' -import useLayoutEffect from '../utils/useIsomorphicLayoutEffect' - -/** - * Creates a ref by combining multiple constituent refs. The ref returned by this hook - * should be passed as the ref for the element that needs to be shared. This is - * particularly useful when you are using `React.forwardRef` in your component but you - * also want to be able to access the local element. This is a small anti-pattern, - * though, as it breaks encapsulation. - * @param refs - */ -export function useCombinedRefs(...refs: (ForwardedRef | null | undefined)[]) { - const combinedRef = useRef(null) - - useLayoutEffect(() => { - function setRefs(current: T | null = null) { - for (const ref of refs) { - if (!ref) { - return - } - if (typeof ref === 'function') { - ref(current) - } else { - ref.current = current - } - } - } - - setRefs(combinedRef.current) - - return () => { - // ensure the refs get updated on unmount - // eslint-disable-next-line react-hooks/exhaustive-deps - setRefs(combinedRef.current) - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [...refs, combinedRef.current]) - - return combinedRef -} diff --git a/src/hooks/useRefObjectAsForwardedRef.ts b/src/hooks/useRefObjectAsForwardedRef.ts new file mode 100644 index 00000000000..ad2664ad5dc --- /dev/null +++ b/src/hooks/useRefObjectAsForwardedRef.ts @@ -0,0 +1,12 @@ +import {ForwardedRef, RefObject, useImperativeHandle} from 'react' + +/** + * Use a ref object as the imperative handle for a forwarded ref. This can be used to + * synchronize the ref object with the forwarded ref and allow local access the reference + * instance with `.current`. + * + * **NOTE**: The `refObject` should be passed to the underlying element, NOT the `forwardedRef`. + */ +export function useRefObjectAsForwardedRef(forwardedRef: ForwardedRef, refObject: RefObject): void { + useImperativeHandle(forwardedRef, () => refObject.current) +}