Fix React StrictMode NumberField, ColorField, ActionGroup

packages/@react-aria/actiongroup/src/useActionGroupItem.ts

@@ -11,9 +11,9 @@
  */
 
 import {DOMAttributes, FocusableElement} from '@react-types/shared';
-import {Key, RefObject, useEffect, useRef} from 'react';
+import {Key, RefObject, useEffect} from 'react';
 import {ListState} from '@react-stately/list';
-import {mergeProps} from '@react-aria/utils';
+import {mergeProps, useEffectEvent} from '@react-aria/utils';
 import {PressProps} from '@react-aria/interactions';
 
 export interface AriaActionGroupItemProps {
@@ -43,18 +43,18 @@ export function useActionGroupItem<T>(props: AriaActionGroupItemProps, state: Li
   }
 
   let isFocused = props.key === state.selectionManager.focusedKey;
-  let lastRender = useRef({isFocused, state});
-  lastRender.current = {isFocused, state};
+  let onRemovedWithFocus = useEffectEvent(() => {
+    if (isFocused) {
+      state.selectionManager.setFocusedKey(null);
+    }
+  });
 
   // If the focused item is removed from the DOM, reset the focused key to null.
-  // eslint-disable-next-line arrow-body-style
   useEffect(() => {
     return () => {
-      if (lastRender.current.isFocused) {
-        lastRender.current.state.selectionManager.setFocusedKey(null);
-      }
+      onRemovedWithFocus();
     };
-  }, []);
+  }, [onRemovedWithFocus]);
 
   return {
     buttonProps: mergeProps(buttonProps, {

packages/@react-aria/spinbutton/src/useSpinButton.ts

@@ -15,8 +15,8 @@ import {AriaButtonProps} from '@react-types/button';
 import {DOMAttributes, InputBase, RangeInputBase, Validation, ValueBase} from '@react-types/shared';
 // @ts-ignore
 import intlMessages from '../intl/*.json';
-import {useCallback, useEffect, useRef} from 'react';
-import {useGlobalListeners} from '@react-aria/utils';
+import {useCallback, useEffect, useMemo, useRef} from 'react';
+import {useEffectEvent, useGlobalListeners} from '@react-aria/utils';
 import {useLocalizedStringFormatter} from '@react-aria/i18n';
 
 
@@ -39,7 +39,6 @@ export interface SpinbuttonAria {
 export function useSpinButton(
   props: SpinButtonProps
 ): SpinbuttonAria {
-  const _async = useRef<number>();
   let {
     value,
     textValue,
@@ -55,18 +54,17 @@ export function useSpinButton(
     onDecrementToMin,
     onIncrementToMax
   } = props;
-  const stringFormatter = useLocalizedStringFormatter(intlMessages);
-  const propsRef = useRef(props);
-  propsRef.current = props;
 
-  const clearAsync = () => clearTimeout(_async.current);
+  const stringFormatter = useLocalizedStringFormatter(intlMessages);
 
-  // eslint-disable-next-line arrow-body-style
+  const _async = useRef<number>();
+  const clearAsync = useCallback(() => clearTimeout(_async.current), [_async]);
+  // only run on unmount
   useEffect(() => {
     return () => clearAsync();
-  }, []);
+  }, [clearAsync]);
 
-  let onKeyDown = (e) => {
+  let onKeyDown = useCallback((e) => {
     if (e.ctrlKey || e.metaKey || e.shiftKey || e.altKey || isReadOnly) {
       return;
     }
@@ -113,22 +111,24 @@ export function useSpinButton(
         }
         break;
     }
-  };
+  }, [isReadOnly, onIncrementPage, onIncrement, onDecrementPage, onDecrement, onDecrementToMin, onIncrementToMax]);
 
   let isFocused = useRef(false);
-  let onFocus = () => {
+  let onFocus = useCallback(() => {
     isFocused.current = true;
-  };
+  }, [isFocused]);
 
-  let onBlur = () => {
+  let onBlur = useCallback(() => {
     isFocused.current = false;
-  };
+  }, [isFocused]);
 
   // Replace Unicode hyphen-minus (U+002D) with minus sign (U+2212).
   // This ensures that macOS VoiceOver announces it as "minus" even with other characters between the minus sign
   // and the number (e.g. currency symbol). Otherwise it announces nothing because it assumes the character is a hyphen.
   // In addition, replace the empty string with the word "Empty" so that iOS VoiceOver does not read "50%" for an empty field.
-  textValue = textValue === '' ? stringFormatter.format('Empty') : (textValue || `${value}`).replace('-', '\u2212');
+  textValue = useMemo(
+    () => textValue === '' ? stringFormatter.format('Empty') : (textValue || `${value}`).replace('-', '\u2212')
+  , [textValue, stringFormatter, value]);
 
   useEffect(() => {
     if (isFocused.current) {
@@ -137,45 +137,37 @@ export function useSpinButton(
     }
   }, [textValue]);
 
-  const onIncrementPressStart = useCallback(
-    (initialStepDelay: number) => {
-      clearAsync();
-      propsRef.current.onIncrement();
-      // Start spinning after initial delay
-      _async.current = window.setTimeout(
-        () => {
-          if (isNaN(maxValue) || isNaN(value) || value < maxValue) {
-            onIncrementPressStart(60);
-          }
-        },
-        initialStepDelay
-      );
-    },
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-    [onIncrement, maxValue, value]
-  );
-
-  const onDecrementPressStart = useCallback(
-    (initialStepDelay: number) => {
-      clearAsync();
-      propsRef.current.onDecrement();
-      // Start spinning after initial delay
-      _async.current = window.setTimeout(
-        () => {
-          if (isNaN(minValue) || isNaN(value) || value > minValue) {
-            onDecrementPressStart(60);
-          }
-        },
-        initialStepDelay
-      );
-    },
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-    [onDecrement, minValue, value]
-  );
+  let onIncrementPressStart = useEffectEvent((initialStepDelay: number) => {
+    clearAsync();
+    onIncrement();
+    // Start spinning after initial delay
+    _async.current = window.setTimeout(
+      () => {
+        if (isNaN(maxValue) || isNaN(value) || value < maxValue) {
+          onIncrementPressStart(60);
+        }
+      },
+      initialStepDelay
+    );
+  });
+
+  let onDecrementPressStart = useEffectEvent((initialStepDelay: number) => {
+    clearAsync();
+    onDecrement();
+    // Start spinning after initial delay
+    _async.current = window.setTimeout(
+      () => {
+        if (isNaN(minValue) || isNaN(value) || value > minValue) {
+          onDecrementPressStart(60);
+        }
+      },
+      initialStepDelay
+    );
+  });
 
-  let cancelContextMenu = (e) => {
+  let cancelContextMenu = useCallback((e) => {
     e.preventDefault();
-  };
+  }, []);
 
   let {addGlobalListener, removeAllGlobalListeners} = useGlobalListeners();
 
@@ -194,26 +186,26 @@ export function useSpinButton(
       onBlur
     },
     incrementButtonProps: {
-      onPressStart: () => {
+      onPressStart: useCallback(() => {
         onIncrementPressStart(400);
         addGlobalListener(window, 'contextmenu', cancelContextMenu);
-      },
-      onPressEnd: () => {
+      }, [onIncrementPressStart, addGlobalListener, cancelContextMenu]),
+      onPressEnd: useCallback(() => {
         clearAsync();
         removeAllGlobalListeners();
-      },
+      }, [clearAsync, removeAllGlobalListeners]),
       onFocus,
       onBlur
     },
     decrementButtonProps: {
-      onPressStart: () => {
+      onPressStart: useCallback(() => {
         onDecrementPressStart(400);
         addGlobalListener(window, 'contextmenu', cancelContextMenu);
-      },
-      onPressEnd: () => {
+      }, [onDecrementPressStart, addGlobalListener, cancelContextMenu]),
+      onPressEnd: useCallback(() => {
         clearAsync();
         removeAllGlobalListeners();
-      },
+      }, [clearAsync, removeAllGlobalListeners]),
       onFocus,
       onBlur
     }

packages/@react-aria/utils/src/useEffectEvent.ts

@@ -13,13 +13,13 @@
 import {useCallback, useRef} from 'react';
 import {useLayoutEffect} from './useLayoutEffect';
 
-export function useEffectEvent(fn) {
+export function useEffectEvent<T extends Function>(fn: T): T {
   const ref = useRef(null);
   useLayoutEffect(() => {
     ref.current = fn;
   }, [fn]);
-  return useCallback((...args) => {
+  return useCallback<any>((...args) => {
     const f = ref.current;
     return f(...args);
-  }, []);
+  }, []) as T;
 }

packages/@react-spectrum/actiongroup/test/ActionGroup.test.js

@@ -772,6 +772,41 @@ describe('ActionGroup', function () {
       expect(buttons[1]).toHaveAttribute('tabIndex', '0');
     });
 
+    it('moves focus if the focused button was removed', function () {
+      let onAction = jest.fn();
+      let tree = render(
+        <Provider theme={theme}>
+          <ActionGroup onAction={onAction}>
+            <Item key="one">One</Item>
+            <Item key="two">Two</Item>
+            <Item key="three">Three</Item>
+            <Item key="four">Four</Item>
+          </ActionGroup>
+        </Provider>
+      );
+
+      let actiongroup = tree.getByRole('toolbar');
+      let buttons = within(actiongroup).getAllByRole('button');
+      expect(buttons[0]).toHaveAttribute('tabIndex', '0');
+      expect(buttons[1]).toHaveAttribute('tabIndex', '0');
+
+      act(() => buttons[2].focus());
+      tree.rerender(
+        <Provider theme={theme}>
+          <ActionGroup onAction={onAction}>
+            <Item key="one">One</Item>
+            <Item key="two">Two</Item>
+            <Item key="four">Four</Item>
+          </ActionGroup>
+        </Provider>
+      );
+      actiongroup = tree.getByRole('toolbar');
+      buttons = within(actiongroup).getAllByRole('button');
+      expect(buttons[0]).toHaveAttribute('tabIndex', '0');
+      expect(buttons[1]).toHaveAttribute('tabIndex', '0');
+      expect(buttons[2]).toHaveAttribute('tabIndex', '0');
+    });
+
     it('passes aria labeling props through to menu button if it is the only child', function () {
       jest.spyOn(HTMLElement.prototype, 'offsetWidth', 'get').mockImplementation(function () {
         if (this instanceof HTMLButtonElement) {

packages/@react-spectrum/numberfield/test/NumberField.test.js

@@ -280,6 +280,7 @@ describe('NumberField', function () {
     act(() => {textField.blur();});
     expect(onChangeSpy).toHaveBeenLastCalledWith(5);
     expect(onChangeSpy).toHaveBeenCalledTimes(2);
+    expect(textField).toHaveAttribute('value', '5');
     triggerPress(incrementButton);
     expect(onChangeSpy).toHaveBeenLastCalledWith(10);
     expect(onChangeSpy).toHaveBeenCalledTimes(3);
@@ -881,41 +882,29 @@ describe('NumberField', function () {
     act(() => {textField.focus();});
     textField.setSelectionRange(2, 3);
     userEvent.type(textField, '{backspace}');
-    expect(announce).toHaveBeenCalledTimes(2);
-    expect(announce).toHaveBeenLastCalledWith('−$0.00', 'assertive');
     textField.setSelectionRange(2, 2);
     typeText(textField, '1');
-    expect(announce).toHaveBeenCalledTimes(3);
-    expect(announce).toHaveBeenLastCalledWith('−$1.00', 'assertive');
     textField.setSelectionRange(3, 3);
     typeText(textField, '8');
     expect(textField).toHaveAttribute('value', '($18.00)');
     act(() => {textField.blur();});
     expect(textField).toHaveAttribute('value', '($18.00)');
     expect(onChangeSpy).toHaveBeenCalledTimes(2);
     expect(onChangeSpy).toHaveBeenLastCalledWith(-18);
-    expect(announce).toHaveBeenCalledTimes(4);
-    expect(announce).toHaveBeenLastCalledWith('−$18.00', 'assertive');
 
     act(() => {textField.focus();});
     textField.setSelectionRange(7, 8);
     userEvent.type(textField, '{backspace}');
     expect(textField).toHaveAttribute('value', '($18.00');
-    expect(announce).toHaveBeenCalledTimes(5);
-    expect(announce).toHaveBeenLastCalledWith('$18.00', 'assertive');
     act(() => {textField.blur();});
     expect(textField).toHaveAttribute('value', '$18.00');
     expect(onChangeSpy).toHaveBeenCalledTimes(3);
     expect(onChangeSpy).toHaveBeenLastCalledWith(18);
 
     act(() => {textField.focus();});
     userEvent.clear(textField);
-    expect(announce).toHaveBeenCalledTimes(6);
-    expect(announce).toHaveBeenLastCalledWith('Empty', 'assertive');
     typeText(textField, '($32)');
     expect(textField).toHaveAttribute('value', '($32)');
-    expect(announce).toHaveBeenCalledTimes(9);
-    expect(announce).toHaveBeenLastCalledWith('−$32.00', 'assertive');
     act(() => {textField.blur();});
     expect(textField).toHaveAttribute('value', '($32.00)');
     expect(onChangeSpy).toHaveBeenCalledTimes(4);
@@ -929,10 +918,9 @@ describe('NumberField', function () {
     let {textField} = renderNumberField({onChange: onChangeSpy, formatOptions: {style: 'currency', currency: 'USD', currencySign: 'accounting'}}, {locale: 'ar-AE'});
 
     act(() => {textField.focus();});
-    userEvent.type(textField, '(10)');
+    typeText(textField, '(10)');
     expect(textField).toHaveAttribute('value', '(10)');
-    expect(announce).toHaveBeenCalledTimes(3);
-    expect(announce).toHaveBeenLastCalledWith('؜−١٠٫٠٠ US$', 'assertive');
+    expect(announce).toHaveBeenCalledTimes(0);
     act(() => {textField.blur();});
     expect(textField).toHaveAttribute('value', '(US$10.00)');
     expect(onChangeSpy).toHaveBeenCalledTimes(1);
@@ -962,17 +950,11 @@ describe('NumberField', function () {
     });
     textField.setSelectionRange(1, 2);
     userEvent.type(textField, '{backspace}');
-    expect(announce).toHaveBeenCalledTimes(2);
-    expect(announce).toHaveBeenLastCalledWith('−0,00 $', 'assertive');
     textField.setSelectionRange(1, 1);
     typeText(textField, '1');
-    expect(announce).toHaveBeenCalledTimes(3);
-    expect(announce).toHaveBeenLastCalledWith('−1,00 $', 'assertive');
     textField.setSelectionRange(2, 2);
     typeText(textField, '8');
     expect(textField).toHaveAttribute('value', '-18,00 $');
-    expect(announce).toHaveBeenCalledTimes(4);
-    expect(announce).toHaveBeenLastCalledWith('−18,00 $', 'assertive');
     act(() => {
       textField.blur();
     });