chore: Update eslint plugin react hooks followup (#9054)

* chore: update eslint plugin react hooks

* fix all lint errors

* Add lines for other config items

* Revert "fix all lint errors"

This reverts commit 7b96414.

* fix all rules of hooks and exhaustive dependencies

* enable all rules we are already passing

* fix event order and cleanup

* fix lint

* turn on and fix static-components

* turn on and fix set-state-in-effect

* turn on and fix purity

* fix lower react version tests

* turn on globals and fix errors

* move event listener attachment to an effect

* convert remaining useElementTypes

* fix read from ref in render

Author: snowystinger

eslint.config.mjs

@@ -234,14 +234,14 @@ export default [{
         'react-hooks/error-boundaries': ERROR,
         'react-hooks/component-hook-factories': ERROR,
         'react-hooks/gating': ERROR,
-        // 'react-hooks/globals': ERROR,
+        'react-hooks/globals': ERROR,
         // 'react-hooks/immutability': ERROR,
-        // 'react-hooks/preserve-manual-memoization': ERROR,
-        // 'react-hooks/purity': ERROR,
-        // 'react-hooks/refs': ERROR,
-        // 'react-hooks/set-state-in-effect': ERROR,
+        // 'react-hooks/preserve-manual-memoization': ERROR, // No idea how to turn this one on yet
+        'react-hooks/purity': ERROR,
+        // 'react-hooks/refs': ERROR, // can't turn on until https://github.com/facebook/react/issues/34775 is fixed
+        'react-hooks/set-state-in-effect': ERROR,
         'react-hooks/set-state-in-render': ERROR,
-        // 'react-hooks/static-components': ERROR,
+        'react-hooks/static-components': ERROR,
         'react-hooks/unsupported-syntax': WARN,
         'react-hooks/use-memo': ERROR,
         'react-hooks/incompatible-library': WARN,

packages/@react-aria/dnd/stories/Reorderable.tsx

@@ -65,6 +65,7 @@ export function ReorderableGridExample(props: any): JSX.Element {
   );
 }
 
+let randomDragTypeReorderExample = `keys-${Math.random().toString(36).slice(2)}`;
 function ReorderableGrid(props) {
   let ref = React.useRef<HTMLDivElement>(null);
   let state = useListState(props);
@@ -91,7 +92,7 @@ function ReorderableGrid(props) {
   });
 
   // Use a random drag type so the items can only be reordered within this list and not dragged elsewhere.
-  let dragType = React.useMemo(() => `keys-${Math.random().toString(36).slice(2)}`, []);
+  let dragType = React.useMemo(() => randomDragTypeReorderExample, []);
   let preview = useRef(null);
   let dragState = useDraggableCollectionState({
     collection: gridState.collection,

packages/@react-aria/dnd/test/dnd.test.js

@@ -17,7 +17,7 @@ import {CUSTOM_DRAG_TYPE} from '../src/constants';
 import {DataTransfer, DataTransferItem, DragEvent, FileSystemDirectoryEntry, FileSystemFileEntry} from './mocks';
 import {Draggable, Droppable} from './examples';
 import {DragTypes} from '../src/utils';
-import React from 'react';
+import React, {useEffect} from 'react';
 import userEvent from '@testing-library/user-event';
 
 function pointerEvent(type, opts) {
@@ -195,13 +195,13 @@ describe('useDrag and useDrop', function () {
       let draggable = tree.getByText('Drag me');
       let droppable = tree.getByText('Drop here');
       expect(droppable).toHaveAttribute('data-droptarget', 'false');
-      
+
       let dataTransfer = new DataTransfer();
       fireEvent(draggable, new DragEvent('dragstart', {dataTransfer, clientX: 0, clientY: 0}));
       act(() => jest.runAllTimers());
       expect(draggable).toHaveAttribute('data-dragging', 'true');
       expect(droppable).toHaveAttribute('data-droptarget', 'false');
-      
+
       expect(onDragStart).toHaveBeenCalledTimes(1);
       expect(onDragMove).not.toHaveBeenCalled();
       expect(onDragEnd).not.toHaveBeenCalled();
@@ -2574,7 +2574,9 @@ describe('useDrag and useDrop', function () {
       let setShowTarget2;
       let Test = () => {
         let [showTarget2, _setShowTarget2] = React.useState(false);
-        setShowTarget2 = _setShowTarget2;
+        useEffect(() => {
+          setShowTarget2 = _setShowTarget2;
+        }, [_setShowTarget2]);
         return (<>
           <Draggable />
           <Droppable />
@@ -2635,7 +2637,9 @@ describe('useDrag and useDrop', function () {
       let setShowTarget2;
       let Test = () => {
         let [showTarget2, _setShowTarget2] = React.useState(true);
-        setShowTarget2 = _setShowTarget2;
+        useEffect(() => {
+          setShowTarget2 = _setShowTarget2;
+        }, [_setShowTarget2]);
         return (<>
           <Draggable />
           <Droppable />

packages/@react-aria/menu/src/useSafelyMouseToSubmenu.ts

@@ -1,7 +1,7 @@
 
 import {RefObject} from '@react-types/shared';
 import {useEffect, useRef, useState} from 'react';
-import {useEffectEvent, useResizeObserver} from '@react-aria/utils';
+import {useEffectEvent, useLayoutEffect, useResizeObserver} from '@react-aria/utils';
 import {useInteractionModality} from '@react-aria/interactions';
 
 interface SafelyMouseToSubmenuOptions {
@@ -67,7 +67,7 @@ export function useSafelyMouseToSubmenu(options: SafelyMouseToSubmenuOptions): v
     }
   }, [menuRef, preventPointerEvents]);
 
-  useEffect(() => {
+  useLayoutEffect(() => {
     let submenu = submenuRef.current;
     let menu = menuRef.current;
 

packages/@react-aria/toast/src/useToast.ts

@@ -12,7 +12,7 @@
 
 import {AriaButtonProps} from '@react-types/button';
 import {AriaLabelingProps, DOMAttributes, FocusableElement, RefObject} from '@react-types/shared';
-import {filterDOMProps, useId, useSlotId} from '@react-aria/utils';
+import {filterDOMProps, useId, useLayoutEffect, useSlotId} from '@react-aria/utils';
 // @ts-ignore
 import intlMessages from '../intl/*.json';
 import {QueuedToast, ToastState} from '@react-stately/toast';
@@ -68,7 +68,7 @@ export function useToast<T>(props: AriaToastProps<T>, state: ToastState<T>, ref:
   // Originally was tied to animationStart/End via https://github.com/adobe/react-spectrum/pull/6223/commits/e22e319df64958e822ab7cd9685e96818cae9ba5
   // but toasts don't always have animations.
   let [isVisible, setIsVisible] = useState(false);
-  useEffect(() => {
+  useLayoutEffect(() => {
     setIsVisible(true);
   }, []);
 

packages/@react-aria/utils/test/mergeRefs.test.tsx

@@ -11,7 +11,7 @@
  */
 
 import {mergeRefs} from '../';
-import React, {useCallback, useRef} from 'react';
+import React, {useCallback, useEffect, useRef} from 'react';
 import {render} from '@react-spectrum/test-utils-internal';
 
 describe('mergeRefs', () => {
@@ -20,16 +20,21 @@ describe('mergeRefs', () => {
     let ref2;
 
     const TextField = (props) => {
-      ref1 = useRef(null);
-      ref2 = useRef(null);
+      let internalRef1 = useRef(null);
+      let internalRef2 = useRef(null);
+      useEffect(() => {
+        ref1 = internalRef1;
+        ref2 = internalRef2;
+      }, [internalRef1, internalRef2]);
 
-      const ref = mergeRefs(ref1, ref2);
+      const ref = mergeRefs(internalRef1, internalRef2);
       return <input {...props} ref={ref} />;
     };
 
     render(<TextField foo="foo" />);
 
     expect(ref1.current).toBe(ref2.current);
+    expect(ref1.current).not.toBeNull();
   });
 
   if (parseInt(React.version.split('.')[0], 10) >= 19) {
@@ -40,14 +45,18 @@ describe('mergeRefs', () => {
       let target = null;
 
       const TextField = (props) => {
-        ref1 = useRef(null);
-        ref2 = useRef(null);
+        let internalRef1 = useRef(null);
+        let internalRef2 = useRef(null);
+        useEffect(() => {
+          ref1 = internalRef1;
+          ref2 = internalRef2;
+        }, [internalRef1, internalRef2]);
         let ref3 = useCallback((node) => {
           target = node;
           return cleanUp;
         }, []);
 
-        const ref = mergeRefs(ref1, ref2, ref3);
+        const ref = mergeRefs(internalRef1, internalRef2, ref3);
         return <input {...props} ref={ref} />;
       };
 

packages/@react-spectrum/autocomplete/src/SearchAutocomplete.tsx

@@ -288,7 +288,6 @@ function ForwardSearchAutocompleteInput<T>(props: SearchAutocompleteInputProps<T
       }
     } else if (!isLoading) {
       // If loading is no longer happening, clear any timers and hide the loading circle
-      setShowLoading(false);
       if (timeout.current != null) {
         clearTimeout(timeout.current);
         timeout.current = null;
@@ -297,6 +296,11 @@ function ForwardSearchAutocompleteInput<T>(props: SearchAutocompleteInputProps<T
 
     lastInputValue.current = inputValue;
   }, [isLoading, showLoading, inputValue]);
+  let [prevIsLoading, setPrevIsLoading] = useState(isLoading);
+  if (prevIsLoading !== isLoading && !isLoading) {
+    setShowLoading(false);
+    setPrevIsLoading(isLoading);
+  }
 
   return (
     (<FocusRing

packages/@react-spectrum/breadcrumbs/test/Breadcrumbs.test.js

@@ -14,7 +14,7 @@ import {act, pointerMap, render, within} from '@react-spectrum/test-utils-intern
 import {Breadcrumbs} from '../';
 import {Item} from '@react-stately/collections';
 import {Provider} from '@react-spectrum/provider';
-import React, {useRef} from 'react';
+import React, {createRef, forwardRef} from 'react';
 import {theme} from '@react-spectrum/theme-default';
 import userEvent from '@testing-library/user-event';
 
@@ -93,16 +93,15 @@ describe('Breadcrumbs', function () {
   });
 
   it('Should handle forward ref', function () {
-    let ref;
-    let Component = () => {
-      ref = useRef();
+    let ref = createRef();
+    let Component = forwardRef((props, forwardedRef) => {
       return (
-        <Breadcrumbs ref={ref} aria-label="breadcrumbs-test">
+        <Breadcrumbs ref={forwardedRef} aria-label="breadcrumbs-test">
           <Item>Folder 1</Item>
         </Breadcrumbs>
       );
-    };
-    let {getByLabelText} = render(<Component />);
+    });
+    let {getByLabelText} = render(<Component ref={ref} />);
     let breadcrumb = getByLabelText('breadcrumbs-test');
     expect(breadcrumb).toBe(ref.current.UNSAFE_getDOMNode());
   });

packages/@react-spectrum/color/src/ColorWheel.tsx

@@ -14,14 +14,14 @@ import {classNames, dimensionValue, useFocusableRef, useStyleProps} from '@react
 import {ColorThumb} from './ColorThumb';
 import {ColorWheelContext, useContextProps} from 'react-aria-components';
 import {FocusableRef} from '@react-types/shared';
-import React, {useCallback, useEffect, useRef, useState} from 'react';
+import React, {useCallback, useRef, useState} from 'react';
 import {SpectrumColorWheelProps} from '@react-types/color';
 import styles from '@adobe/spectrum-css-temp/components/colorwheel/vars.css';
 import {useColorWheel} from '@react-aria/color';
 import {useColorWheelState} from '@react-stately/color';
 import {useFocusRing} from '@react-aria/focus';
+import {useLayoutEffect, useResizeObserver} from '@react-aria/utils';
 import {useProviderProps} from '@react-spectrum/provider';
-import {useResizeObserver} from '@react-aria/utils';
 
 const WHEEL_THICKNESS = 24;
 
@@ -53,7 +53,7 @@ export const ColorWheel = React.forwardRef(function ColorWheel(props: SpectrumCo
     }
   }, [containerRef, setWheelRadius, setWheelThickness]);
 
-  useEffect(() => {
+  useLayoutEffect(() => {
     // the size observer's fallback to the window resize event doesn't fire on mount
     if (wheelRadius === 0) {
       resizeHandler();

packages/@react-spectrum/color/stories/ColorSwatchPicker.stories.tsx

@@ -48,10 +48,12 @@ export const Default: ColorSwatchPickerStory = (args) => (
   </ColorSwatchPicker>
 );
 
+let randomColors = Array.from(Array(24)).map(() => {
+  return `#${Math.floor(Math.random() * 0xffffff).toString(16).padStart(6, '0')}`;
+});
 export const ManySwatches: ColorSwatchPickerStory = (args) => (
   <ColorSwatchPicker {...args} maxWidth="size-3000">
-    {Array.from(Array(24)).map(() => {
-      let color = `#${Math.floor(Math.random() * 0xffffff).toString(16).padStart(6, '0')}`;
+    {randomColors.map((color) => {
       return <ColorSwatch key={color} color={color} />;
     })}
   </ColorSwatchPicker>