feat: add React Compiler support for hooks

.changeset/react-compiler-hooks.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+Hooks: Improve rendering performance with React Compiler support

packages/react/script/react-compiler.mjs

@@ -14,24 +14,23 @@ const files = glob
 const unsupportedPatterns = [
   'src/ActionMenu/**/*.tsx',
   'src/Autocomplete/**/*.tsx',
+  'src/AvatarStack/**/*.tsx',
   'src/Banner/**/*.tsx',
   'src/Button/**/*.tsx',
+  'src/Checkbox/**/*.tsx',
   'src/ConfirmationDialog/**/*.tsx',
   'src/Dialog/**/*.tsx',
+  'src/Heading/**/*.tsx',
+  'src/Link/**/*.tsx',
+  'src/Pagehead/**/*.tsx',
   'src/PageLayout/**/*.tsx',
   'src/Pagination/**/*.tsx',
+  'src/Portal/**/*.tsx',
   'src/SelectPanel/**/*.tsx',
+  'src/SideNav.tsx',
   'src/UnderlineNav/**/*.tsx',
   'src/experimental/SelectPanel2/**/*.tsx',
-  'src/hooks/useAnchoredPosition.ts',
-  'src/hooks/useFocusTrap.ts',
-  'src/hooks/useFocusZone.ts',
-  'src/hooks/useMenuInitialFocus.ts',
-  'src/hooks/useOnEscapePress.ts',
-  'src/hooks/useResizeObserver.ts',
-  'src/hooks/useSafeTimeout.ts',
-  'src/hooks/useScrollFlash.ts',
-  'src/hooks/useMergedRefs.ts',
+  'src/internal/components/CheckboxOrRadioGroup/**/*.tsx',
   'src/TooltipV2/**/*.tsx',
 ]
 

packages/react/src/SelectPanel/SelectPanel.test.tsx

@@ -1673,11 +1673,10 @@ for (const usingRemoveActiveDescendant of [false, true]) {
         const input = screen.getByPlaceholderText('Filter items')
         const options = screen.getAllByRole('option')
 
-        // Wait a tick for the effect to run
-        await new Promise(resolve => setTimeout(resolve, 0))
-
-        // aria-activedescendant should be set to the first item
-        expect(input.getAttribute('aria-activedescendant')).toBe(options[0].id)
+        await waitFor(() => {
+          // aria-activedescendant should be set to the first item
+          expect(input.getAttribute('aria-activedescendant')).toBe(options[0].id)
+        })
       })
 
       it('should not set aria-activedescendant on mouse hover until after first interaction when setInitialFocus is true', async () => {

packages/react/src/hooks/useAnchoredPosition.ts

@@ -4,6 +4,7 @@ import type {AnchorPosition, PositionSettings} from '@primer/behaviors'
 import {useProvidedRefOrCreate} from './useProvidedRefOrCreate'
 import {useResizeObserver} from './useResizeObserver'
 import useLayoutEffect from '../utils/useIsomorphicLayoutEffect'
+import {useValueWithDependencies} from './useDependencies'
 
 /**
  * Returns all scrollable ancestor elements of the given element, plus the window.
@@ -34,6 +35,15 @@ export interface AnchoredPositionHookSettings extends Partial<PositionSettings>
   enabled?: boolean
 }
 
+function topPositionChanged(prevPosition: AnchorPosition | undefined, newPosition: AnchorPosition) {
+  return (
+    prevPosition &&
+    ['outside-top', 'inside-top'].includes(prevPosition.anchorSide) &&
+    // either the anchor changed or the element is trying to shrink in height
+    (prevPosition.anchorSide !== newPosition.anchorSide || prevPosition.top < newPosition.top)
+  )
+}
+
 /**
  * Calculates the top and left values for an absolutely-positioned floating element
  * to be anchored to some anchor element. Returns refs for the floating element
@@ -54,21 +64,12 @@ export function useAnchoredPosition(
   const floatingElementRef = useProvidedRefOrCreate(settings?.floatingElementRef)
   const anchorElementRef = useProvidedRefOrCreate(settings?.anchorElementRef)
   const enabled = settings?.enabled ?? true
+  const positionSettingsState = useValueWithDependencies(settings, [enabled, ...dependencies])
   const savedOnPositionChange = React.useRef(settings?.onPositionChange)
   const [position, setPosition] = React.useState<AnchorPosition | undefined>(undefined)
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars
-  const [_, setPrevHeight] = React.useState<number | undefined>(undefined)
-
-  const topPositionChanged = (prevPosition: AnchorPosition | undefined, newPosition: AnchorPosition) => {
-    return (
-      prevPosition &&
-      ['outside-top', 'inside-top'].includes(prevPosition.anchorSide) &&
-      // either the anchor changed or the element is trying to shrink in height
-      (prevPosition.anchorSide !== newPosition.anchorSide || prevPosition.top < newPosition.top)
-    )
-  }
+  const [, setPrevHeight] = React.useState<number | undefined>(undefined)
 
-  const updateElementHeight = () => {
+  const updateElementHeight = React.useCallback(() => {
     let heightUpdated = false
     setPrevHeight(prevHeight => {
       // if the element is trying to shrink in height, restore to old height to prevent it from jumping
@@ -81,39 +82,40 @@ export function useAnchoredPosition(
       return prevHeight
     })
     return heightUpdated
-  }
+  }, [floatingElementRef])
 
-  const updatePosition = React.useCallback(
-    () => {
-      if (!enabled) return
-      if (floatingElementRef.current instanceof Element && anchorElementRef.current instanceof Element) {
-        const newPosition = getAnchoredPosition(floatingElementRef.current, anchorElementRef.current, settings)
-        setPosition(prev => {
-          if (settings?.pinPosition && topPositionChanged(prev, newPosition)) {
-            const anchorTop = anchorElementRef.current?.getBoundingClientRect().top ?? 0
-            const elementStillFitsOnTop = anchorTop > (floatingElementRef.current?.clientHeight ?? 0)
-
-            if (elementStillFitsOnTop && updateElementHeight()) {
-              return prev
-            }
+  const updatePosition = React.useCallback(() => {
+    if (!enabled) return
+    if (floatingElementRef.current instanceof Element && anchorElementRef.current instanceof Element) {
+      const newPosition = getAnchoredPosition(
+        floatingElementRef.current,
+        anchorElementRef.current,
+        positionSettingsState.value,
+      )
+      setPosition(prev => {
+        if (positionSettingsState.value?.pinPosition && topPositionChanged(prev, newPosition)) {
+          const anchorTop = anchorElementRef.current?.getBoundingClientRect().top ?? 0
+          const elementStillFitsOnTop = anchorTop > (floatingElementRef.current?.clientHeight ?? 0)
+
+          // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
+          if (elementStillFitsOnTop && updateElementHeight()) {
+            return prev
           }
+        }
 
-          if (prev && prev.anchorSide === newPosition.anchorSide) {
-            // if the position hasn't changed, don't update
-            savedOnPositionChange.current?.(newPosition)
-          }
+        if (prev && prev.anchorSide === newPosition.anchorSide) {
+          // if the position hasn't changed, don't update
+          savedOnPositionChange.current?.(newPosition)
+        }
 
-          return newPosition
-        })
-      } else {
-        setPosition(undefined)
-        savedOnPositionChange.current?.(undefined)
-      }
-      setPrevHeight(floatingElementRef.current?.clientHeight)
-    },
-    // eslint-disable-next-line react-hooks/exhaustive-deps, react-hooks/use-memo
-    [floatingElementRef, anchorElementRef, enabled, ...dependencies],
-  )
+        return newPosition
+      })
+    } else {
+      setPosition(undefined)
+      savedOnPositionChange.current?.(undefined)
+    }
+    setPrevHeight(floatingElementRef.current?.clientHeight)
+  }, [anchorElementRef, enabled, floatingElementRef, positionSettingsState, updateElementHeight])
 
   useLayoutEffect(() => {
     savedOnPositionChange.current = settings?.onPositionChange

packages/react/src/hooks/useDependencies.ts

@@ -0,0 +1,44 @@
+import React from 'react'
+
+interface DependencyState<T> {
+  dependencies: React.DependencyList
+  signal: number
+  value: T
+}
+
+export function areDependenciesEqual(
+  previousDependencies: React.DependencyList,
+  nextDependencies: React.DependencyList,
+): boolean {
+  if (previousDependencies.length !== nextDependencies.length) {
+    return false
+  }
+
+  for (let i = 0; i < previousDependencies.length; i++) {
+    if (!Object.is(previousDependencies[i], nextDependencies[i])) {
+      return false
+    }
+  }
+
+  return true
+}
+
+export function useDependencySignal(dependencies: React.DependencyList): number {
+  return useValueWithDependencies(undefined, dependencies).signal
+}
+
+export function useValueWithDependencies<T>(value: T, dependencies: React.DependencyList): DependencyState<T> {
+  const [state, setState] = React.useState<DependencyState<T>>(() => ({dependencies, signal: 0, value}))
+
+  if (!areDependenciesEqual(state.dependencies, dependencies)) {
+    const nextState = {
+      dependencies,
+      signal: state.signal + 1,
+      value,
+    }
+    setState(nextState)
+    return nextState
+  }
+
+  return state
+}

packages/react/src/hooks/useFocusTrap.ts

@@ -2,6 +2,7 @@ import React from 'react'
 import {focusTrap} from '@primer/behaviors'
 import {useProvidedRefOrCreate} from './useProvidedRefOrCreate'
 import {useOnOutsideClick} from './useOnOutsideClick'
+import {useValueWithDependencies} from './useDependencies'
 
 export interface FocusTrapHookSettings {
   /**
@@ -52,59 +53,53 @@ export function useFocusTrap(
   settings?: FocusTrapHookSettings,
   dependencies: React.DependencyList = [],
 ): {containerRef: React.RefObject<HTMLElement | null>; initialFocusRef: React.RefObject<HTMLElement | null>} {
-  const [outsideClicked, setOutsideClicked] = React.useState(false)
   const containerRef = useProvidedRefOrCreate(settings?.containerRef)
   const initialFocusRef = useProvidedRefOrCreate(settings?.initialFocusRef)
   const disabled = settings?.disabled
+  const {signal: settingsSignal, value: focusTrapSettings} = useValueWithDependencies(settings, [
+    disabled,
+    ...dependencies,
+  ])
   const abortController = React.useRef<AbortController>()
   const previousFocusedElement = React.useRef<Element | null>(null)
-
-  // If we are enabling a focus trap and haven't already stored the previously focused element
-  // go ahead an do that so we can restore later when the trap is disabled.
-  // eslint-disable-next-line react-hooks/refs
-  if (!previousFocusedElement.current && !disabled) {
-    previousFocusedElement.current = document.activeElement
-  }
+  const skipRestoreFocusRef = React.useRef(false)
 
   // This function removes the event listeners that enable the focus trap and restores focus
   // to the previously-focused element (if necessary).
-  function disableTrap() {
+  const disableTrap = React.useCallback(() => {
     abortController.current?.abort()
-    if (settings?.allowOutsideClick && outsideClicked) {
+    if (focusTrapSettings?.allowOutsideClick && skipRestoreFocusRef.current) {
       return
     }
-    if (settings?.returnFocusRef && settings.returnFocusRef.current instanceof HTMLElement) {
-      settings.returnFocusRef.current.focus()
-    } else if (settings?.restoreFocusOnCleanUp && previousFocusedElement.current instanceof HTMLElement) {
+    if (focusTrapSettings?.returnFocusRef && focusTrapSettings.returnFocusRef.current instanceof HTMLElement) {
+      focusTrapSettings.returnFocusRef.current.focus()
+    } else if (focusTrapSettings?.restoreFocusOnCleanUp && previousFocusedElement.current instanceof HTMLElement) {
       previousFocusedElement.current.focus()
       previousFocusedElement.current = null
     }
-  }
+  }, [focusTrapSettings])
 
-  React.useEffect(
-    () => {
-      if (containerRef.current instanceof HTMLElement) {
-        if (!disabled) {
-          abortController.current = focusTrap(containerRef.current, initialFocusRef.current ?? undefined)
-          return () => {
-            disableTrap()
-          }
-        } else {
+  React.useEffect(() => {
+    if (containerRef.current instanceof HTMLElement) {
+      if (!disabled) {
+        // If we are enabling a focus trap and haven't already stored the previously focused element
+        // go ahead an do that so we can restore later when the trap is disabled.
+        previousFocusedElement.current ??= document.activeElement
+        abortController.current = focusTrap(containerRef.current, initialFocusRef.current ?? undefined)
+        return () => {
           disableTrap()
         }
+      } else {
+        disableTrap()
       }
-    },
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-    [containerRef, initialFocusRef, disabled, ...dependencies],
-  )
+    }
+  }, [containerRef, initialFocusRef, disabled, settingsSignal, disableTrap])
+
   useOnOutsideClick({
     containerRef: containerRef as React.RefObject<HTMLDivElement>,
     onClickOutside: () => {
-      setOutsideClicked(true)
       if (settings?.allowOutsideClick) {
-        // eslint-disable-next-line react-hooks/immutability
-        if (settings.returnFocusRef) settings.returnFocusRef = undefined
-        settings.restoreFocusOnCleanUp = false
+        skipRestoreFocusRef.current = true
         abortController.current?.abort()
       }
     },

packages/react/src/hooks/useFocusZone.ts

@@ -2,6 +2,7 @@ import React, {useEffect} from 'react'
 import {focusZone} from '@primer/behaviors'
 import type {FocusZoneSettings} from '@primer/behaviors'
 import {useProvidedRefOrCreate} from './useProvidedRefOrCreate'
+import {useValueWithDependencies} from './useDependencies'
 export {FocusKeys} from '@primer/behaviors'
 export type {Direction} from '@primer/behaviors'
 
@@ -45,32 +46,32 @@ export function useFocusZone(
       : settings.activeDescendantFocus
   const activeDescendantControlRef = useProvidedRefOrCreate(passedActiveDescendantRef)
   const disabled = settings.disabled
+  const {signal: settingsSignal, value: focusZoneSettings} = useValueWithDependencies(settings, [
+    disabled,
+    ...dependencies,
+  ])
   const abortController = React.useRef<AbortController>()
 
-  useEffect(
-    () => {
-      if (
-        containerRef.current instanceof HTMLElement &&
-        // eslint-disable-next-line react-you-might-not-need-an-effect/no-event-handler
-        (!useActiveDescendant || activeDescendantControlRef.current instanceof HTMLElement)
-      ) {
-        if (!disabled) {
-          const vanillaSettings: FocusZoneSettings = {
-            ...settings,
-            activeDescendantControl: activeDescendantControlRef.current ?? undefined,
-          }
-          abortController.current = focusZone(containerRef.current, vanillaSettings)
-          return () => {
-            abortController.current?.abort()
-          }
-        } else {
+  useEffect(() => {
+    if (
+      containerRef.current instanceof HTMLElement &&
+      // eslint-disable-next-line react-you-might-not-need-an-effect/no-event-handler
+      (!useActiveDescendant || activeDescendantControlRef.current instanceof HTMLElement)
+    ) {
+      if (!disabled) {
+        const vanillaSettings: FocusZoneSettings = {
+          ...focusZoneSettings,
+          activeDescendantControl: activeDescendantControlRef.current ?? undefined,
+        }
+        abortController.current = focusZone(containerRef.current, vanillaSettings)
+        return () => {
           abortController.current?.abort()
         }
+      } else {
+        abortController.current?.abort()
       }
-    },
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-    [disabled, ...dependencies],
-  )
+    }
+  }, [activeDescendantControlRef, containerRef, disabled, focusZoneSettings, settingsSignal, useActiveDescendant])
 
   return {containerRef, activeDescendantControlRef}
 }

packages/react/src/hooks/useMenuInitialFocus.ts

@@ -72,7 +72,6 @@ export const useMenuInitialFocus = (
     },
     // we don't want containerRef in dependencies
     // because re-renders to containerRef while it's open should not fire initialMenuFocus
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-    [open, openingGesture, anchorRef],
+    [open, openingGesture, anchorRef, containerRef],
   )
 }

packages/react/src/hooks/useOnEscapePress.ts

@@ -1,4 +1,5 @@
 import {useEffect, useCallback, useMemo} from 'react'
+import {useValueWithDependencies} from './useDependencies'
 
 /**
  * Calls all handlers in reverse order
@@ -54,8 +55,7 @@ export const useOnEscapePress = (
   onEscape: (e: KeyboardEvent) => void,
   callbackDependencies: React.DependencyList = [onEscape],
 ): void => {
-  // eslint-disable-next-line react-hooks/exhaustive-deps, react-hooks/use-memo
-  const escapeCallback = useCallback(onEscape, callbackDependencies)
+  const {value: escapeCallback} = useValueWithDependencies(onEscape, callbackDependencies)
 
   const handler = useCallback<KeyboardEventCallback>(
     event => {