diff --git a/.changeset/curvy-frogs-like.md b/.changeset/curvy-frogs-like.md new file mode 100644 index 00000000000..ab9172d96c1 --- /dev/null +++ b/.changeset/curvy-frogs-like.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +Update internal implementations of combined refs to improve performance and add support for React 19 callback refs diff --git a/package-lock.json b/package-lock.json index af2c7ed660d..33593b20ae9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6898,9 +6898,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -6918,9 +6915,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -6938,9 +6932,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -6958,9 +6949,6 @@ "riscv64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -6978,9 +6966,6 @@ "riscv64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -6998,9 +6983,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -7018,9 +7000,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -7038,9 +7017,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -7253,9 +7229,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -7270,9 +7243,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -7287,9 +7257,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -7304,9 +7271,6 @@ "riscv64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -7321,9 +7285,6 @@ "riscv64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -7338,9 +7299,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -7355,9 +7313,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -7372,9 +7327,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -8847,9 +8799,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -8867,9 +8816,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -8887,9 +8833,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -8907,9 +8850,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -8927,9 +8867,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -8947,9 +8884,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -18945,9 +18879,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -18969,9 +18900,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -18993,9 +18921,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -19017,9 +18942,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -28526,9 +28448,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -28546,9 +28465,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -28566,9 +28482,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -28586,9 +28499,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -28606,9 +28516,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -28626,9 +28533,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ diff --git a/packages/react/src/ActionBar/ActionBar.tsx b/packages/react/src/ActionBar/ActionBar.tsx index 299ff7ffb6c..3e5daa579c0 100644 --- a/packages/react/src/ActionBar/ActionBar.tsx +++ b/packages/react/src/ActionBar/ActionBar.tsx @@ -352,7 +352,7 @@ function useActionBarItem(ref: React.RefObject, registryProp export const ActionBarIconButton = forwardRef( ({disabled, onClick, ...props}: ActionBarIconButtonProps, forwardedRef) => { const ref = useRef(null) - const mergedRef = useMergedRefs(forwardedRef, ref) + const mergedRef = useMergedRefs(ref, forwardedRef) const {size} = React.useContext(ActionBarContext) diff --git a/packages/react/src/Autocomplete/Autocomplete.test.tsx b/packages/react/src/Autocomplete/Autocomplete.test.tsx index 054a9b52e0a..b08ed7ae583 100644 --- a/packages/react/src/Autocomplete/Autocomplete.test.tsx +++ b/packages/react/src/Autocomplete/Autocomplete.test.tsx @@ -162,20 +162,25 @@ describe('Autocomplete', () => { }) it('closes the menu when the input is blurred', async () => { + const user = userEvent.setup() const {getByLabelText} = render( - , + <> + + + , ) const inputNode = getByLabelText(AUTOCOMPLETE_LABEL) + const outsideButton = screen.getByRole('button', {name: 'outside'}) expect(inputNode.getAttribute('aria-expanded')).not.toBe('true') - fireEvent.click(inputNode) + await user.click(inputNode) fireEvent.keyDown(inputNode, {key: 'ArrowDown'}) expect(inputNode.getAttribute('aria-expanded')).toBe('true') - // `userEvent.tab()` is unreliable in browser-mode Vitest for this case; blur is deterministic. - // eslint-disable-next-line github/no-blur - fireEvent.blur(inputNode) + await user.click(outsideButton) await waitFor(() => expect(inputNode.getAttribute('aria-expanded')).not.toBe('true')) }) diff --git a/packages/react/src/Autocomplete/AutocompleteInput.tsx b/packages/react/src/Autocomplete/AutocompleteInput.tsx index f7622d7013a..a4ace88a1f4 100644 --- a/packages/react/src/Autocomplete/AutocompleteInput.tsx +++ b/packages/react/src/Autocomplete/AutocompleteInput.tsx @@ -58,11 +58,17 @@ const AutocompleteInput = React.forwardRef( event => { onBlur && onBlur(event) - // HACK: wait a tick and check the focused element before hiding the autocomplete menu - // 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 + // HACK: wait a tick before hiding the menu so click interactions can complete. + // Use the blur event's relatedTarget to determine whether focus is moving into the + // autocomplete menu; if not, hide the menu when focus leaves the input. safeSetTimeout(() => { - if (document.activeElement !== inputRef.current) { + const nextFocusedElement = event.relatedTarget as Node | null + const menuElement = document.getElementById(`${id}-listbox`) + + if ( + !nextFocusedElement || + (nextFocusedElement !== menuElement && !menuElement?.contains(nextFocusedElement)) + ) { setShowMenu(false) // Reset the input's value to the text the user actually typed rather than leaving the @@ -75,7 +81,7 @@ const AutocompleteInput = React.forwardRef( } }, 0) }, - [onBlur, setShowMenu, inputRef, safeSetTimeout, autocompleteSuggestion, inputValue], + [onBlur, setShowMenu, inputRef, safeSetTimeout, autocompleteSuggestion, inputValue, id], ) const handleInputChange: ChangeEventHandler = event => { diff --git a/packages/react/src/Button/ButtonBase.tsx b/packages/react/src/Button/ButtonBase.tsx index 19357e9ae43..e1702ccb3ce 100644 --- a/packages/react/src/Button/ButtonBase.tsx +++ b/packages/react/src/Button/ButtonBase.tsx @@ -51,7 +51,7 @@ const ButtonBase = forwardRef(({children, as: Component = 'button', ...props}, f } = props const innerRef = React.useRef(null) - const mergedRef = useMergedRefs(forwardedRef, innerRef) + const combinedRefs = useMergedRefs(forwardedRef, innerRef) const uuid = useId(id) const loadingAnnouncementID = `${uuid}-loading-announcement` @@ -88,8 +88,7 @@ const ButtonBase = forwardRef(({children, as: Component = 'button', ...props}, f aria-disabled={loading ? true : undefined} data-component="Button" {...rest} - // @ts-ignore temporary disable as we migrate to css modules, until we remove PolymorphicForwardRefComponent - ref={mergedRef} + ref={combinedRefs} className={clsx(classes.ButtonBase, className)} data-block={block ? 'block' : null} data-inactive={inactive ? true : undefined} @@ -124,7 +123,7 @@ const ButtonBase = forwardRef(({children, as: Component = 'button', ...props}, f { /* If there are no leading/trailing visuals/actions to replace with a loading spinner, - render a loading spiner in place of the button content. */ + render a loading spinner in place of the button content. */ loading && !LeadingVisual && !TrailingVisual && diff --git a/packages/react/src/Link/Link.tsx b/packages/react/src/Link/Link.tsx index 5ae1e74f5ba..2d490995978 100644 --- a/packages/react/src/Link/Link.tsx +++ b/packages/react/src/Link/Link.tsx @@ -1,5 +1,5 @@ import {clsx} from 'clsx' -import React, {type ForwardedRef, type ElementRef} from 'react' +import React, {useEffect, type ForwardedRef, type ElementRef} from 'react' import {useDevOnlyEffect} from '../internal/hooks/useDevOnlyEffect' import {useMergedRefs} from '../hooks' import classes from './Link.module.css' @@ -23,6 +23,30 @@ export const UnwrappedLink = ( const innerRef = React.useRef>(null) const mergedRef = useMergedRefs(ref, innerRef) + if (__DEV__) { + /** + * The Linter yells because it thinks this conditionally calls an effect, + * but since this is a compile-time flag and not a runtime conditional + * this is safe, and ensures the entire effect is kept out of prod builds + * shaving precious bytes from the output, and avoiding mounting a noop effect + */ + // eslint-disable-next-line react-hooks/rules-of-hooks + useEffect(() => { + if ( + innerRef.current && + !(innerRef.current instanceof HTMLButtonElement) && + !(innerRef.current instanceof HTMLAnchorElement) + ) { + // eslint-disable-next-line no-console + console.error( + 'Error: Found `Link` component that renders an inaccessible element', + innerRef.current, + 'Please ensure `Link` always renders as or