From 5d3e2f355c57326fbba96ebc2d0d5ea7929a6633 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Fri, 10 Oct 2025 14:58:20 +1100 Subject: [PATCH 01/26] chore: update eslint plugin react hooks --- eslint.config.mjs | 5 ++--- package.json | 2 +- yarn.lock | 48 ++++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index 92f49f59acc..f8883204b14 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -5,7 +5,6 @@ import reactHooks from "eslint-plugin-react-hooks"; import jest from "eslint-plugin-jest"; import monorepo from "@jdb8/eslint-plugin-monorepo"; import * as rspRules from "eslint-plugin-rsp-rules"; -import { fixupPluginRules } from "@eslint/compat"; import globals from "globals"; import babelParser from "@babel/eslint-parser"; import typescriptEslint from "@typescript-eslint/eslint-plugin"; @@ -67,7 +66,7 @@ export default [{ react, rulesdir, "jsx-a11y": jsxA11Y, - "react-hooks": fixupPluginRules(reactHooks), + "react-hooks": reactHooks, jest, monorepo, "rsp-rules": rspRules, @@ -332,7 +331,7 @@ export default [{ react, rulesdir, "jsx-a11y": jsxA11Y, - "react-hooks": fixupPluginRules(reactHooks), + "react-hooks": reactHooks, jest, "@typescript-eslint": typescriptEslint, monorepo, diff --git a/package.json b/package.json index d7a0bf1dfc0..641e5e1438a 100644 --- a/package.json +++ b/package.json @@ -160,7 +160,7 @@ "eslint-plugin-jsdoc": "^50.4.1", "eslint-plugin-jsx-a11y": "^6.10.0", "eslint-plugin-react": "^7.37.1", - "eslint-plugin-react-hooks": "^5.0.0", + "eslint-plugin-react-hooks": "^7.0.0", "eslint-plugin-rulesdir": "^0.2.2", "fast-check": "^2.19.0", "fast-glob": "^3.1.0", diff --git a/yarn.lock b/yarn.lock index 4a08c636dbb..5509a123729 100644 --- a/yarn.lock +++ b/yarn.lock @@ -16036,12 +16036,18 @@ __metadata: languageName: node linkType: hard -"eslint-plugin-react-hooks@npm:^5.0.0": - version: 5.0.0 - resolution: "eslint-plugin-react-hooks@npm:5.0.0" +"eslint-plugin-react-hooks@npm:^7.0.0": + version: 7.0.0 + resolution: "eslint-plugin-react-hooks@npm:7.0.0" + dependencies: + "@babel/core": "npm:^7.24.4" + "@babel/parser": "npm:^7.24.4" + hermes-parser: "npm:^0.25.1" + zod: "npm:^3.22.4 || ^4.0.0" + zod-validation-error: "npm:^3.0.3 || ^4.0.0" peerDependencies: eslint: ^3.0.0 || ^4.0.0 || ^5.0.0 || ^6.0.0 || ^7.0.0 || ^8.0.0-0 || ^9.0.0 - checksum: 10c0/bcb74b421f32e4203a7100405b57aab85526be4461e5a1da01bc537969a30012d2ee209a2c2a6cac543833a27188ce1e6ad71e4628d0bb4a2e5365cad86c5002 + checksum: 10c0/911c9efdd9b102ce2eabac247dff8c217ecb8d6972aaf3b7eecfb1cfc293d4d902766355993ff7a37a33c0abde3e76971f43bc1c8ff36d6c123310e5680d0423 languageName: node linkType: hard @@ -18586,6 +18592,22 @@ __metadata: languageName: node linkType: hard +"hermes-estree@npm:0.25.1": + version: 0.25.1 + resolution: "hermes-estree@npm:0.25.1" + checksum: 10c0/48be3b2fa37a0cbc77a112a89096fa212f25d06de92781b163d67853d210a8a5c3784fac23d7d48335058f7ed283115c87b4332c2a2abaaccc76d0ead1a282ac + languageName: node + linkType: hard + +"hermes-parser@npm:^0.25.1": + version: 0.25.1 + resolution: "hermes-parser@npm:0.25.1" + dependencies: + hermes-estree: "npm:0.25.1" + checksum: 10c0/3abaa4c6f1bcc25273f267297a89a4904963ea29af19b8e4f6eabe04f1c2c7e9abd7bfc4730ddb1d58f2ea04b6fee74053d8bddb5656ec6ebf6c79cc8d14202c + languageName: node + linkType: hard + "hex-rgb@npm:^4.1.0": version: 4.3.0 resolution: "hex-rgb@npm:4.3.0" @@ -26634,7 +26656,7 @@ __metadata: eslint-plugin-jsdoc: "npm:^50.4.1" eslint-plugin-jsx-a11y: "npm:^6.10.0" eslint-plugin-react: "npm:^7.37.1" - eslint-plugin-react-hooks: "npm:^5.0.0" + eslint-plugin-react-hooks: "npm:^7.0.0" eslint-plugin-rulesdir: "npm:^0.2.2" fast-check: "npm:^2.19.0" fast-glob: "npm:^3.1.0" @@ -32204,6 +32226,22 @@ __metadata: languageName: node linkType: hard +"zod-validation-error@npm:^3.0.3 || ^4.0.0": + version: 4.0.2 + resolution: "zod-validation-error@npm:4.0.2" + peerDependencies: + zod: ^3.25.0 || ^4.0.0 + checksum: 10c0/0ccfec48c46de1be440b719cd02044d4abb89ed0e14c13e637cd55bf29102f67ccdba373f25def0fc7130e5f15025be4d557a7edcc95d5a3811599aade689e1b + languageName: node + linkType: hard + +"zod@npm:^3.22.4 || ^4.0.0": + version: 4.1.12 + resolution: "zod@npm:4.1.12" + checksum: 10c0/b64c1feb19e99d77075261eaf613e0b2be4dfcd3551eff65ad8b4f2a079b61e379854d066f7d447491fcf193f45babd8095551a9d47973d30b46b6d8e2c46774 + languageName: node + linkType: hard + "zod@npm:^3.23.8": version: 3.25.76 resolution: "zod@npm:3.25.76" From 7b9641413c49717246bd6f664763cde4a4ecd8c2 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Fri, 10 Oct 2025 15:32:57 +1100 Subject: [PATCH 02/26] fix all lint errors --- .../autocomplete/src/useAutocomplete.ts | 10 +++++----- .../grid/src/useGridSelectionAnnouncement.ts | 4 ++-- packages/@react-aria/interactions/src/useMove.ts | 6 +++--- .../@react-aria/interactions/src/usePress.ts | 16 ++++++++-------- packages/@react-aria/interactions/src/utils.ts | 4 ++-- .../@react-aria/menu/src/useSubmenuTrigger.ts | 6 +++--- .../selection/src/useSelectableCollection.ts | 6 +++--- .../@react-aria/spinbutton/src/useSpinButton.ts | 6 +++--- .../table/src/useTableColumnResize.ts | 8 ++++---- packages/@react-aria/toast/src/useToastRegion.ts | 4 ++-- packages/@react-aria/utils/src/index.ts | 2 +- packages/@react-aria/utils/src/useEffectEvent.ts | 4 ++++ packages/@react-aria/utils/src/useValueEffect.ts | 6 +++--- .../@react-aria/virtualizer/src/ScrollView.tsx | 4 ++-- packages/@react-spectrum/s2/src/Tabs.tsx | 8 ++++---- packages/@react-spectrum/s2/src/TagGroup.tsx | 6 +++--- 16 files changed, 52 insertions(+), 48 deletions(-) diff --git a/packages/@react-aria/autocomplete/src/useAutocomplete.ts b/packages/@react-aria/autocomplete/src/useAutocomplete.ts index 313615d5977..6e0892f210c 100644 --- a/packages/@react-aria/autocomplete/src/useAutocomplete.ts +++ b/packages/@react-aria/autocomplete/src/useAutocomplete.ts @@ -13,7 +13,7 @@ import {AriaLabelingProps, BaseEvent, DOMProps, FocusableElement, FocusEvents, KeyboardEvents, Node, RefObject, ValueBase} from '@react-types/shared'; import {AriaTextFieldProps} from '@react-aria/textfield'; import {AutocompleteProps, AutocompleteState} from '@react-stately/autocomplete'; -import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, getActiveElement, getOwnerDocument, isAndroid, isCtrlKeyPressed, isIOS, mergeProps, mergeRefs, useEffectEvent, useEvent, useId, useLabels, useObjectRef} from '@react-aria/utils'; +import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, getActiveElement, getOwnerDocument, isAndroid, isCtrlKeyPressed, isIOS, mergeProps, mergeRefs, useEvent, useId, useLabels, useObjectRef, useStableCallback} from '@react-aria/utils'; import {dispatchVirtualBlur, dispatchVirtualFocus, getVirtuallyFocusedElement, moveVirtualFocus} from '@react-aria/focus'; import {getInteractionModality} from '@react-aria/interactions'; // @ts-ignore @@ -106,7 +106,7 @@ export function useAutocomplete(props: AriaAutocompleteOptions, state: Aut return () => clearTimeout(timeout.current); }, []); - let updateActiveDescendant = useEffectEvent((e: Event) => { + let updateActiveDescendant = useStableCallback((e: Event) => { // Ensure input is focused if the user clicks on the collection directly. if (!e.isTrusted && shouldUseVirtualFocus && inputRef.current && getActiveElement(getOwnerDocument(inputRef.current)) !== inputRef.current) { inputRef.current.focus(); @@ -165,7 +165,7 @@ export function useAutocomplete(props: AriaAutocompleteOptions, state: Aut // Make sure to memo so that React doesn't keep registering a new event listeners on every rerender of the wrapped collection let mergedCollectionRef = useObjectRef(useMemo(() => mergeRefs(collectionRef, callbackRef), [collectionRef, callbackRef])); - let focusFirstItem = useEffectEvent(() => { + let focusFirstItem = useStableCallback(() => { delayNextActiveDescendant.current = true; collectionRef.current?.dispatchEvent( new CustomEvent(FOCUS_EVENT, { @@ -178,7 +178,7 @@ export function useAutocomplete(props: AriaAutocompleteOptions, state: Aut ); }); - let clearVirtualFocus = useEffectEvent((clearFocusKey?: boolean) => { + let clearVirtualFocus = useStableCallback((clearFocusKey?: boolean) => { moveVirtualFocus(getActiveElement()); queuedActiveDescendant.current = null; state.setFocusedNodeId(null); @@ -321,7 +321,7 @@ export function useAutocomplete(props: AriaAutocompleteOptions, state: Aut } }; - let onKeyUpCapture = useEffectEvent((e) => { + let onKeyUpCapture = useStableCallback((e) => { // Dispatch simulated key up events for things like triggering links in listbox // Make sure to stop the propagation of the input keyup event so that the simulated keyup/down pair // is detected by usePress instead of the original keyup originating from the input diff --git a/packages/@react-aria/grid/src/useGridSelectionAnnouncement.ts b/packages/@react-aria/grid/src/useGridSelectionAnnouncement.ts index c3d84564c32..16df3d2ce7d 100644 --- a/packages/@react-aria/grid/src/useGridSelectionAnnouncement.ts +++ b/packages/@react-aria/grid/src/useGridSelectionAnnouncement.ts @@ -15,9 +15,9 @@ import {Collection, Key, Node, Selection} from '@react-types/shared'; // @ts-ignore import intlMessages from '../intl/*.json'; import {SelectionManager} from '@react-stately/selection'; -import {useEffectEvent, useUpdateEffect} from '@react-aria/utils'; import {useLocalizedStringFormatter} from '@react-aria/i18n'; import {useRef} from 'react'; +import {useStableCallback, useUpdateEffect} from '@react-aria/utils'; export interface GridSelectionAnnouncementProps { /** @@ -46,7 +46,7 @@ export function useGridSelectionAnnouncement(props: GridSelectionAnnouncement // We do this using an ARIA live region. let selection = state.selectionManager.rawSelection; let lastSelection = useRef(selection); - let announceSelectionChange = useEffectEvent(() => { + let announceSelectionChange = useStableCallback(() => { if (!state.selectionManager.isFocused || selection === lastSelection.current) { lastSelection.current = selection; diff --git a/packages/@react-aria/interactions/src/useMove.ts b/packages/@react-aria/interactions/src/useMove.ts index f7f32acfdbb..a004288541e 100644 --- a/packages/@react-aria/interactions/src/useMove.ts +++ b/packages/@react-aria/interactions/src/useMove.ts @@ -13,7 +13,7 @@ import {disableTextSelection, restoreTextSelection} from './textSelection'; import {DOMAttributes, MoveEvents, PointerType} from '@react-types/shared'; import React, {useMemo, useRef} from 'react'; -import {useEffectEvent, useGlobalListeners} from '@react-aria/utils'; +import {useGlobalListeners, useStableCallback} from '@react-aria/utils'; export interface MoveResult { /** Props to spread on the target element. */ @@ -43,7 +43,7 @@ export function useMove(props: MoveEvents): MoveResult { let {addGlobalListener, removeGlobalListener} = useGlobalListeners(); - let move = useEffectEvent((originalEvent: EventBase, pointerType: PointerType, deltaX: number, deltaY: number) => { + let move = useStableCallback((originalEvent: EventBase, pointerType: PointerType, deltaX: number, deltaY: number) => { if (deltaX === 0 && deltaY === 0) { return; } @@ -71,7 +71,7 @@ export function useMove(props: MoveEvents): MoveResult { }); }); - let end = useEffectEvent((originalEvent: EventBase, pointerType: PointerType) => { + let end = useStableCallback((originalEvent: EventBase, pointerType: PointerType) => { restoreTextSelection(); if (state.current.didMove) { onMoveEnd?.({ diff --git a/packages/@react-aria/interactions/src/usePress.ts b/packages/@react-aria/interactions/src/usePress.ts index 42576b36095..779e3091a10 100644 --- a/packages/@react-aria/interactions/src/usePress.ts +++ b/packages/@react-aria/interactions/src/usePress.ts @@ -27,8 +27,8 @@ import { mergeProps, nodeContains, openLink, - useEffectEvent, useGlobalListeners, + useStableCallback, useSyncRef } from '@react-aria/utils'; import {createSyntheticEvent, preventFocus, setEventTarget} from './utils'; @@ -197,7 +197,7 @@ export function usePress(props: PressHookProps): PressResult { let {addGlobalListener, removeAllGlobalListeners} = useGlobalListeners(); - let triggerPressStart = useEffectEvent((originalEvent: EventBase, pointerType: PointerType) => { + let triggerPressStart = useStableCallback((originalEvent: EventBase, pointerType: PointerType) => { let state = ref.current; if (isDisabled || state.didFirePressStart) { return false; @@ -221,7 +221,7 @@ export function usePress(props: PressHookProps): PressResult { return shouldStopPropagation; }); - let triggerPressEnd = useEffectEvent((originalEvent: EventBase, pointerType: PointerType, wasPressed = true) => { + let triggerPressEnd = useStableCallback((originalEvent: EventBase, pointerType: PointerType, wasPressed = true) => { let state = ref.current; if (!state.didFirePressStart) { return false; @@ -253,7 +253,7 @@ export function usePress(props: PressHookProps): PressResult { return shouldStopPropagation; }); - let triggerPressUp = useEffectEvent((originalEvent: EventBase, pointerType: PointerType) => { + let triggerPressUp = useStableCallback((originalEvent: EventBase, pointerType: PointerType) => { let state = ref.current; if (isDisabled) { return false; @@ -270,7 +270,7 @@ export function usePress(props: PressHookProps): PressResult { return true; }); - let cancel = useEffectEvent((e: EventBase) => { + let cancel = useStableCallback((e: EventBase) => { let state = ref.current; if (state.isPressed && state.target) { if (state.didFirePressStart && state.pointerType != null) { @@ -291,13 +291,13 @@ export function usePress(props: PressHookProps): PressResult { } }); - let cancelOnPointerExit = useEffectEvent((e: EventBase) => { + let cancelOnPointerExit = useStableCallback((e: EventBase) => { if (shouldCancelOnPointerExit) { cancel(e); } }); - let triggerClick = useEffectEvent((e: RMouseEvent) => { + let triggerClick = useStableCallback((e: RMouseEvent) => { if (isDisabled) { return; } @@ -305,7 +305,7 @@ export function usePress(props: PressHookProps): PressResult { onClick?.(e); }); - let triggerSyntheticClick = useEffectEvent((e: KeyboardEvent | TouchEvent, target: FocusableElement) => { + let triggerSyntheticClick = useStableCallback((e: KeyboardEvent | TouchEvent, target: FocusableElement) => { if (isDisabled) { return; } diff --git a/packages/@react-aria/interactions/src/utils.ts b/packages/@react-aria/interactions/src/utils.ts index f31da638bc9..78185e63f3b 100644 --- a/packages/@react-aria/interactions/src/utils.ts +++ b/packages/@react-aria/interactions/src/utils.ts @@ -11,7 +11,7 @@ */ import {FocusableElement} from '@react-types/shared'; -import {focusWithoutScrolling, getOwnerWindow, isFocusable, useEffectEvent, useLayoutEffect} from '@react-aria/utils'; +import {focusWithoutScrolling, getOwnerWindow, isFocusable, useLayoutEffect, useStableCallback} from '@react-aria/utils'; import {FocusEvent as ReactFocusEvent, SyntheticEvent, useCallback, useRef} from 'react'; // Turn a native event into a React synthetic event. @@ -48,7 +48,7 @@ export function useSyntheticBlurEvent(onBlur: }; }, []); - let dispatchBlur = useEffectEvent((e: ReactFocusEvent) => { + let dispatchBlur = useStableCallback((e: ReactFocusEvent) => { onBlur?.(e); }); diff --git a/packages/@react-aria/menu/src/useSubmenuTrigger.ts b/packages/@react-aria/menu/src/useSubmenuTrigger.ts index 590ce43a213..2cdd991188a 100644 --- a/packages/@react-aria/menu/src/useSubmenuTrigger.ts +++ b/packages/@react-aria/menu/src/useSubmenuTrigger.ts @@ -14,7 +14,7 @@ import {AriaMenuItemProps} from './useMenuItem'; import {AriaMenuOptions} from './useMenu'; import type {AriaPopoverProps, OverlayProps} from '@react-aria/overlays'; import {FocusableElement, FocusStrategy, KeyboardEvent, Node, PressEvent, RefObject} from '@react-types/shared'; -import {focusWithoutScrolling, useEffectEvent, useEvent, useId, useLayoutEffect} from '@react-aria/utils'; +import {focusWithoutScrolling, useEvent, useId, useLayoutEffect, useStableCallback} from '@react-aria/utils'; import type {SubmenuTriggerState} from '@react-stately/menu'; import {useCallback, useRef} from 'react'; import {useLocale} from '@react-aria/i18n'; @@ -81,12 +81,12 @@ export function useSubmenuTrigger(props: AriaSubmenuTriggerProps, state: Subm } }, [openTimeout]); - let onSubmenuOpen = useEffectEvent((focusStrategy?: FocusStrategy) => { + let onSubmenuOpen = useStableCallback((focusStrategy?: FocusStrategy) => { cancelOpenTimeout(); state.open(focusStrategy); }); - let onSubmenuClose = useEffectEvent(() => { + let onSubmenuClose = useStableCallback(() => { cancelOpenTimeout(); state.close(); }); diff --git a/packages/@react-aria/selection/src/useSelectableCollection.ts b/packages/@react-aria/selection/src/useSelectableCollection.ts index 4259dc51132..72c0275697f 100644 --- a/packages/@react-aria/selection/src/useSelectableCollection.ts +++ b/packages/@react-aria/selection/src/useSelectableCollection.ts @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, focusWithoutScrolling, getActiveElement, isCtrlKeyPressed, isTabbable, mergeProps, scrollIntoView, scrollIntoViewport, useEffectEvent, useEvent, useRouter, useUpdateLayoutEffect} from '@react-aria/utils'; +import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, focusWithoutScrolling, getActiveElement, isCtrlKeyPressed, isTabbable, mergeProps, scrollIntoView, scrollIntoViewport, useEvent, useRouter, useStableCallback, useUpdateLayoutEffect} from '@react-aria/utils'; import {dispatchVirtualFocus, getFocusableTreeWalker, moveVirtualFocus} from '@react-aria/focus'; import {DOMAttributes, FocusableElement, FocusStrategy, Key, KeyboardDelegate, RefObject} from '@react-types/shared'; import {flushSync} from 'react-dom'; @@ -416,7 +416,7 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions } }); - let updateActiveDescendant = useEffectEvent(() => { + let updateActiveDescendant = useStableCallback(() => { let keyToFocus = delegate.getFirstKey?.() ?? null; // If no focusable items exist in the list, make sure to clear any activedescendant that may still exist and move focus back to @@ -447,7 +447,7 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions }, [manager.collection, updateActiveDescendant]); - let resetFocusFirstFlag = useEffectEvent(() => { + let resetFocusFirstFlag = useStableCallback(() => { // If user causes the focused key to change in any other way, clear shouldVirtualFocusFirst so we don't // accidentally move focus from under them. Skip this if the collection was empty because we might be in a load // state and will still want to focus the first item after load diff --git a/packages/@react-aria/spinbutton/src/useSpinButton.ts b/packages/@react-aria/spinbutton/src/useSpinButton.ts index dada685076b..3115972a17f 100644 --- a/packages/@react-aria/spinbutton/src/useSpinButton.ts +++ b/packages/@react-aria/spinbutton/src/useSpinButton.ts @@ -16,7 +16,7 @@ import {DOMAttributes, InputBase, RangeInputBase, Validation, ValueBase} from '@ // @ts-ignore import intlMessages from '../intl/*.json'; import {useCallback, useEffect, useRef} from 'react'; -import {useEffectEvent, useGlobalListeners} from '@react-aria/utils'; +import {useGlobalListeners, useStableCallback} from '@react-aria/utils'; import {useLocalizedStringFormatter} from '@react-aria/i18n'; @@ -153,7 +153,7 @@ export function useSpinButton( prevTouchPosition.current = touchPosition; }, []); - const onIncrementPressStart = useEffectEvent( + const onIncrementPressStart = useStableCallback( (initialStepDelay: number) => { clearAsync(); isSpinning.current = true; @@ -170,7 +170,7 @@ export function useSpinButton( } ); - const onDecrementPressStart = useEffectEvent( + const onDecrementPressStart = useStableCallback( (initialStepDelay: number) => { clearAsync(); isSpinning.current = true; diff --git a/packages/@react-aria/table/src/useTableColumnResize.ts b/packages/@react-aria/table/src/useTableColumnResize.ts index 0a8c956ab13..af10e371157 100644 --- a/packages/@react-aria/table/src/useTableColumnResize.ts +++ b/packages/@react-aria/table/src/useTableColumnResize.ts @@ -18,7 +18,7 @@ import {getColumnHeaderId} from './utils'; import {GridNode} from '@react-types/grid'; // @ts-ignore import intlMessages from '../intl/*.json'; -import {mergeProps, useDescription, useEffectEvent, useId} from '@react-aria/utils'; +import {mergeProps, useDescription, useId, useStableCallback} from '@react-aria/utils'; import {TableColumnResizeState} from '@react-stately/table'; import {useLocale, useLocalizedStringFormatter} from '@react-aria/i18n'; import {useVisuallyHidden} from '@react-aria/visually-hidden'; @@ -88,7 +88,7 @@ export function useTableColumnResize(props: AriaTableColumnResizeProps, st } }); - let startResize = useEffectEvent((item) => { + let startResize = useStableCallback((item) => { if (!isResizingRef.current) { lastSize.current = state.updateResizedColumns(item.key, state.getColumnWidth(item.key)); state.startResize(item.key); @@ -98,13 +98,13 @@ export function useTableColumnResize(props: AriaTableColumnResizeProps, st isResizingRef.current = true; }); - let resize = useEffectEvent((item, newWidth) => { + let resize = useStableCallback((item, newWidth) => { let sizes = state.updateResizedColumns(item.key, newWidth); onResize?.(sizes); lastSize.current = sizes; }); - let endResize = useEffectEvent((item) => { + let endResize = useStableCallback((item) => { if (isResizingRef.current) { if (lastSize.current == null) { lastSize.current = state.updateResizedColumns(item.key, state.getColumnWidth(item.key)); diff --git a/packages/@react-aria/toast/src/useToastRegion.ts b/packages/@react-aria/toast/src/useToastRegion.ts index 04d6a8dceed..3c5939ff0cc 100644 --- a/packages/@react-aria/toast/src/useToastRegion.ts +++ b/packages/@react-aria/toast/src/useToastRegion.ts @@ -11,7 +11,7 @@ */ import {AriaLabelingProps, DOMAttributes, FocusableElement, RefObject} from '@react-types/shared'; -import {focusWithoutScrolling, mergeProps, useEffectEvent, useLayoutEffect} from '@react-aria/utils'; +import {focusWithoutScrolling, mergeProps, useLayoutEffect, useStableCallback} from '@react-aria/utils'; import {getInteractionModality, useFocusWithin, useHover} from '@react-aria/interactions'; // @ts-ignore import intlMessages from '../intl/*.json'; @@ -46,7 +46,7 @@ export function useToastRegion(props: AriaToastRegionProps, state: ToastState let isHovered = useRef(false); let isFocused = useRef(false); - let updateTimers = useEffectEvent(() => { + let updateTimers = useStableCallback(() => { if (isHovered.current || isFocused.current) { state.pauseAll(); } else { diff --git a/packages/@react-aria/utils/src/index.ts b/packages/@react-aria/utils/src/index.ts index 9da3461dd5b..36e1c5dc003 100644 --- a/packages/@react-aria/utils/src/index.ts +++ b/packages/@react-aria/utils/src/index.ts @@ -41,7 +41,7 @@ export {useValueEffect} from './useValueEffect'; export {scrollIntoView, scrollIntoViewport} from './scrollIntoView'; export {clamp, snapValueToStep} from '@react-stately/utils'; export {isVirtualClick, isVirtualPointerEvent} from './isVirtualEvent'; -export {useEffectEvent} from './useEffectEvent'; +export {useEffectEvent, useStableCallback} from './useEffectEvent'; export {useDeepMemo} from './useDeepMemo'; export {useFormReset} from './useFormReset'; export {useLoadMore} from './useLoadMore'; diff --git a/packages/@react-aria/utils/src/useEffectEvent.ts b/packages/@react-aria/utils/src/useEffectEvent.ts index 20b89d03038..a3569af2494 100644 --- a/packages/@react-aria/utils/src/useEffectEvent.ts +++ b/packages/@react-aria/utils/src/useEffectEvent.ts @@ -28,3 +28,7 @@ export function useEffectEvent(fn?: T): T { return f?.(...args); }, []); } + +export function useStableCallback(fn?: T): T { + return useEffectEvent(fn); +} diff --git a/packages/@react-aria/utils/src/useValueEffect.ts b/packages/@react-aria/utils/src/useValueEffect.ts index a8c90397aea..f83b1f9e3fb 100644 --- a/packages/@react-aria/utils/src/useValueEffect.ts +++ b/packages/@react-aria/utils/src/useValueEffect.ts @@ -11,7 +11,7 @@ */ import {Dispatch, MutableRefObject, useRef, useState} from 'react'; -import {useEffectEvent, useLayoutEffect} from './'; +import {useLayoutEffect, useStableCallback} from './'; type SetValueAction = (prev: S) => Generator; @@ -25,7 +25,7 @@ export function useValueEffect(defaultValue: S | (() => S)): [S, Dispatch { + let nextRef = useStableCallback(() => { if (!effect.current) { return; } @@ -55,7 +55,7 @@ export function useValueEffect(defaultValue: S | (() => S)): [S, Dispatch { + let queue = useStableCallback(fn => { effect.current = fn(value); nextRef(); }); diff --git a/packages/@react-aria/virtualizer/src/ScrollView.tsx b/packages/@react-aria/virtualizer/src/ScrollView.tsx index 7c4988a5b3e..efaf24c4252 100644 --- a/packages/@react-aria/virtualizer/src/ScrollView.tsx +++ b/packages/@react-aria/virtualizer/src/ScrollView.tsx @@ -25,7 +25,7 @@ import React, { useState } from 'react'; import {Rect, Size} from '@react-stately/virtualizer'; -import {useEffectEvent, useEvent, useLayoutEffect, useObjectRef, useResizeObserver} from '@react-aria/utils'; +import {useEvent, useLayoutEffect, useObjectRef, useResizeObserver, useStableCallback} from '@react-aria/utils'; import {useLocale} from '@react-aria/i18n'; interface ScrollViewProps extends HTMLAttributes { @@ -157,7 +157,7 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject { + let updateSize = useStableCallback((flush: typeof flushSync) => { let dom = ref.current; if (!dom || isUpdatingSize.current) { return; diff --git a/packages/@react-spectrum/s2/src/Tabs.tsx b/packages/@react-spectrum/s2/src/Tabs.tsx index 329a6284daa..cc252b41e0b 100644 --- a/packages/@react-spectrum/s2/src/Tabs.tsx +++ b/packages/@react-spectrum/s2/src/Tabs.tsx @@ -33,7 +33,7 @@ import {CollectionBuilder} from '@react-aria/collections'; import {createContext, forwardRef, ReactNode, useCallback, useContext, useEffect, useMemo, useRef, useState} from 'react'; import {getAllowedOverrides, StyleProps, StylesPropWithHeight, UnsafeStyles} from './style-utils' with {type: 'macro'}; import {IconContext} from './Icon'; -import {inertValue, useEffectEvent, useId, useLabels, useLayoutEffect, useResizeObserver} from '@react-aria/utils'; +import {inertValue, useId, useLabels, useLayoutEffect, useResizeObserver, useStableCallback} from '@react-aria/utils'; import {Picker, PickerItem} from './TabsPicker'; import {Text, TextContext} from './Content'; import {useControlledState} from '@react-stately/utils'; @@ -216,7 +216,7 @@ export function TabList(props: TabListProps): ReactNode | n if (showTabs) { return ; } - + return (
{listRef &&
@@ -628,7 +628,7 @@ let CollapsingTabs = ({collection, containerRef, ...props}: {collection: Collect let children = useMemo(() => [...collection], [collection]); let listRef = useRef(null); - let updateOverflow = useEffectEvent(() => { + let updateOverflow = useStableCallback(() => { if (orientation === 'vertical' || !listRef.current || !containerRef?.current) { return; } @@ -664,7 +664,7 @@ let CollapsingTabs = ({collection, containerRef, ...props}: {collection: Collect useEffect(() => { // Recalculate visible tags when fonts are loaded. document.fonts?.ready.then(() => updateOverflow()); - // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); let menuId = useId(); diff --git a/packages/@react-spectrum/s2/src/TagGroup.tsx b/packages/@react-spectrum/s2/src/TagGroup.tsx index 2beaaa2bcf0..4dd33259ae1 100644 --- a/packages/@react-spectrum/s2/src/TagGroup.tsx +++ b/packages/@react-spectrum/s2/src/TagGroup.tsx @@ -41,7 +41,7 @@ import {FormContext, useFormProps} from './Form'; import {forwardRefType} from './types'; import {IconContext} from './Icon'; import {ImageContext} from './Image'; -import {inertValue, useEffectEvent, useId, useLayoutEffect, useResizeObserver} from '@react-aria/utils'; +import {inertValue, useId, useLayoutEffect, useResizeObserver, useStableCallback} from '@react-aria/utils'; // @ts-ignore import intlMessages from '../intl/*.json'; import {pressScale} from './pressScale'; @@ -146,7 +146,7 @@ function TagGroupInner({ [collection, tagState.visibleTagCount, isCollapsed] ); - let updateVisibleTagCount = useEffectEvent(() => { + let updateVisibleTagCount = useStableCallback(() => { if (maxRows == null) { setTagState({visibleTagCount: collection.size, showCollapseButton: false}); } @@ -229,7 +229,7 @@ function TagGroupInner({ useEffect(() => { // Recalculate visible tags when fonts are loaded. document.fonts?.ready.then(() => updateVisibleTagCount()); - // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); let handlePressCollapse = () => { From 7f79dd9c834e271b26d1f9ca8c011749455300f7 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Mon, 13 Oct 2025 11:21:41 +1100 Subject: [PATCH 03/26] Add lines for other config items --- eslint.config.mjs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/eslint.config.mjs b/eslint.config.mjs index f8883204b14..417342a1567 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -224,8 +224,28 @@ export default [{ "react/jsx-boolean-value": ERROR, "react/jsx-first-prop-new-line": [ERROR, "multiline"], "react/self-closing-comp": ERROR, + + // Core hooks rules "react-hooks/rules-of-hooks": ERROR, // https://github.com/facebook/react/blob/main/packages/eslint-plugin-react-hooks/CHANGELOG.md "react-hooks/exhaustive-deps": WARN, + + // React Compiler rules + // 'react-hooks/config': ERROR, + // 'react-hooks/error-boundaries': ERROR, + // 'react-hooks/component-hook-factories': ERROR, + // 'react-hooks/gating': 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/set-state-in-render': ERROR, + // 'react-hooks/static-components': ERROR, + // 'react-hooks/unsupported-syntax': WARN, + // 'react-hooks/use-memo': ERROR, + // 'react-hooks/incompatible-library': WARN, + "rsp-rules/no-react-key": [ERROR], "rsp-rules/sort-imports": [ERROR], "rulesdir/imports": [ERROR], From 1208e74b9723bcbbef4e70340870820025d1447d Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Mon, 13 Oct 2025 11:22:24 +1100 Subject: [PATCH 04/26] Revert "fix all lint errors" This reverts commit 7b9641413c49717246bd6f664763cde4a4ecd8c2. --- .../autocomplete/src/useAutocomplete.ts | 10 +++++----- .../grid/src/useGridSelectionAnnouncement.ts | 4 ++-- packages/@react-aria/interactions/src/useMove.ts | 6 +++--- .../@react-aria/interactions/src/usePress.ts | 16 ++++++++-------- packages/@react-aria/interactions/src/utils.ts | 4 ++-- .../@react-aria/menu/src/useSubmenuTrigger.ts | 6 +++--- .../selection/src/useSelectableCollection.ts | 6 +++--- .../@react-aria/spinbutton/src/useSpinButton.ts | 6 +++--- .../table/src/useTableColumnResize.ts | 8 ++++---- packages/@react-aria/toast/src/useToastRegion.ts | 4 ++-- packages/@react-aria/utils/src/index.ts | 2 +- packages/@react-aria/utils/src/useEffectEvent.ts | 4 ---- packages/@react-aria/utils/src/useValueEffect.ts | 6 +++--- .../@react-aria/virtualizer/src/ScrollView.tsx | 4 ++-- packages/@react-spectrum/s2/src/Tabs.tsx | 8 ++++---- packages/@react-spectrum/s2/src/TagGroup.tsx | 6 +++--- 16 files changed, 48 insertions(+), 52 deletions(-) diff --git a/packages/@react-aria/autocomplete/src/useAutocomplete.ts b/packages/@react-aria/autocomplete/src/useAutocomplete.ts index 6e0892f210c..313615d5977 100644 --- a/packages/@react-aria/autocomplete/src/useAutocomplete.ts +++ b/packages/@react-aria/autocomplete/src/useAutocomplete.ts @@ -13,7 +13,7 @@ import {AriaLabelingProps, BaseEvent, DOMProps, FocusableElement, FocusEvents, KeyboardEvents, Node, RefObject, ValueBase} from '@react-types/shared'; import {AriaTextFieldProps} from '@react-aria/textfield'; import {AutocompleteProps, AutocompleteState} from '@react-stately/autocomplete'; -import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, getActiveElement, getOwnerDocument, isAndroid, isCtrlKeyPressed, isIOS, mergeProps, mergeRefs, useEvent, useId, useLabels, useObjectRef, useStableCallback} from '@react-aria/utils'; +import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, getActiveElement, getOwnerDocument, isAndroid, isCtrlKeyPressed, isIOS, mergeProps, mergeRefs, useEffectEvent, useEvent, useId, useLabels, useObjectRef} from '@react-aria/utils'; import {dispatchVirtualBlur, dispatchVirtualFocus, getVirtuallyFocusedElement, moveVirtualFocus} from '@react-aria/focus'; import {getInteractionModality} from '@react-aria/interactions'; // @ts-ignore @@ -106,7 +106,7 @@ export function useAutocomplete(props: AriaAutocompleteOptions, state: Aut return () => clearTimeout(timeout.current); }, []); - let updateActiveDescendant = useStableCallback((e: Event) => { + let updateActiveDescendant = useEffectEvent((e: Event) => { // Ensure input is focused if the user clicks on the collection directly. if (!e.isTrusted && shouldUseVirtualFocus && inputRef.current && getActiveElement(getOwnerDocument(inputRef.current)) !== inputRef.current) { inputRef.current.focus(); @@ -165,7 +165,7 @@ export function useAutocomplete(props: AriaAutocompleteOptions, state: Aut // Make sure to memo so that React doesn't keep registering a new event listeners on every rerender of the wrapped collection let mergedCollectionRef = useObjectRef(useMemo(() => mergeRefs(collectionRef, callbackRef), [collectionRef, callbackRef])); - let focusFirstItem = useStableCallback(() => { + let focusFirstItem = useEffectEvent(() => { delayNextActiveDescendant.current = true; collectionRef.current?.dispatchEvent( new CustomEvent(FOCUS_EVENT, { @@ -178,7 +178,7 @@ export function useAutocomplete(props: AriaAutocompleteOptions, state: Aut ); }); - let clearVirtualFocus = useStableCallback((clearFocusKey?: boolean) => { + let clearVirtualFocus = useEffectEvent((clearFocusKey?: boolean) => { moveVirtualFocus(getActiveElement()); queuedActiveDescendant.current = null; state.setFocusedNodeId(null); @@ -321,7 +321,7 @@ export function useAutocomplete(props: AriaAutocompleteOptions, state: Aut } }; - let onKeyUpCapture = useStableCallback((e) => { + let onKeyUpCapture = useEffectEvent((e) => { // Dispatch simulated key up events for things like triggering links in listbox // Make sure to stop the propagation of the input keyup event so that the simulated keyup/down pair // is detected by usePress instead of the original keyup originating from the input diff --git a/packages/@react-aria/grid/src/useGridSelectionAnnouncement.ts b/packages/@react-aria/grid/src/useGridSelectionAnnouncement.ts index 16df3d2ce7d..c3d84564c32 100644 --- a/packages/@react-aria/grid/src/useGridSelectionAnnouncement.ts +++ b/packages/@react-aria/grid/src/useGridSelectionAnnouncement.ts @@ -15,9 +15,9 @@ import {Collection, Key, Node, Selection} from '@react-types/shared'; // @ts-ignore import intlMessages from '../intl/*.json'; import {SelectionManager} from '@react-stately/selection'; +import {useEffectEvent, useUpdateEffect} from '@react-aria/utils'; import {useLocalizedStringFormatter} from '@react-aria/i18n'; import {useRef} from 'react'; -import {useStableCallback, useUpdateEffect} from '@react-aria/utils'; export interface GridSelectionAnnouncementProps { /** @@ -46,7 +46,7 @@ export function useGridSelectionAnnouncement(props: GridSelectionAnnouncement // We do this using an ARIA live region. let selection = state.selectionManager.rawSelection; let lastSelection = useRef(selection); - let announceSelectionChange = useStableCallback(() => { + let announceSelectionChange = useEffectEvent(() => { if (!state.selectionManager.isFocused || selection === lastSelection.current) { lastSelection.current = selection; diff --git a/packages/@react-aria/interactions/src/useMove.ts b/packages/@react-aria/interactions/src/useMove.ts index a004288541e..f7f32acfdbb 100644 --- a/packages/@react-aria/interactions/src/useMove.ts +++ b/packages/@react-aria/interactions/src/useMove.ts @@ -13,7 +13,7 @@ import {disableTextSelection, restoreTextSelection} from './textSelection'; import {DOMAttributes, MoveEvents, PointerType} from '@react-types/shared'; import React, {useMemo, useRef} from 'react'; -import {useGlobalListeners, useStableCallback} from '@react-aria/utils'; +import {useEffectEvent, useGlobalListeners} from '@react-aria/utils'; export interface MoveResult { /** Props to spread on the target element. */ @@ -43,7 +43,7 @@ export function useMove(props: MoveEvents): MoveResult { let {addGlobalListener, removeGlobalListener} = useGlobalListeners(); - let move = useStableCallback((originalEvent: EventBase, pointerType: PointerType, deltaX: number, deltaY: number) => { + let move = useEffectEvent((originalEvent: EventBase, pointerType: PointerType, deltaX: number, deltaY: number) => { if (deltaX === 0 && deltaY === 0) { return; } @@ -71,7 +71,7 @@ export function useMove(props: MoveEvents): MoveResult { }); }); - let end = useStableCallback((originalEvent: EventBase, pointerType: PointerType) => { + let end = useEffectEvent((originalEvent: EventBase, pointerType: PointerType) => { restoreTextSelection(); if (state.current.didMove) { onMoveEnd?.({ diff --git a/packages/@react-aria/interactions/src/usePress.ts b/packages/@react-aria/interactions/src/usePress.ts index 779e3091a10..42576b36095 100644 --- a/packages/@react-aria/interactions/src/usePress.ts +++ b/packages/@react-aria/interactions/src/usePress.ts @@ -27,8 +27,8 @@ import { mergeProps, nodeContains, openLink, + useEffectEvent, useGlobalListeners, - useStableCallback, useSyncRef } from '@react-aria/utils'; import {createSyntheticEvent, preventFocus, setEventTarget} from './utils'; @@ -197,7 +197,7 @@ export function usePress(props: PressHookProps): PressResult { let {addGlobalListener, removeAllGlobalListeners} = useGlobalListeners(); - let triggerPressStart = useStableCallback((originalEvent: EventBase, pointerType: PointerType) => { + let triggerPressStart = useEffectEvent((originalEvent: EventBase, pointerType: PointerType) => { let state = ref.current; if (isDisabled || state.didFirePressStart) { return false; @@ -221,7 +221,7 @@ export function usePress(props: PressHookProps): PressResult { return shouldStopPropagation; }); - let triggerPressEnd = useStableCallback((originalEvent: EventBase, pointerType: PointerType, wasPressed = true) => { + let triggerPressEnd = useEffectEvent((originalEvent: EventBase, pointerType: PointerType, wasPressed = true) => { let state = ref.current; if (!state.didFirePressStart) { return false; @@ -253,7 +253,7 @@ export function usePress(props: PressHookProps): PressResult { return shouldStopPropagation; }); - let triggerPressUp = useStableCallback((originalEvent: EventBase, pointerType: PointerType) => { + let triggerPressUp = useEffectEvent((originalEvent: EventBase, pointerType: PointerType) => { let state = ref.current; if (isDisabled) { return false; @@ -270,7 +270,7 @@ export function usePress(props: PressHookProps): PressResult { return true; }); - let cancel = useStableCallback((e: EventBase) => { + let cancel = useEffectEvent((e: EventBase) => { let state = ref.current; if (state.isPressed && state.target) { if (state.didFirePressStart && state.pointerType != null) { @@ -291,13 +291,13 @@ export function usePress(props: PressHookProps): PressResult { } }); - let cancelOnPointerExit = useStableCallback((e: EventBase) => { + let cancelOnPointerExit = useEffectEvent((e: EventBase) => { if (shouldCancelOnPointerExit) { cancel(e); } }); - let triggerClick = useStableCallback((e: RMouseEvent) => { + let triggerClick = useEffectEvent((e: RMouseEvent) => { if (isDisabled) { return; } @@ -305,7 +305,7 @@ export function usePress(props: PressHookProps): PressResult { onClick?.(e); }); - let triggerSyntheticClick = useStableCallback((e: KeyboardEvent | TouchEvent, target: FocusableElement) => { + let triggerSyntheticClick = useEffectEvent((e: KeyboardEvent | TouchEvent, target: FocusableElement) => { if (isDisabled) { return; } diff --git a/packages/@react-aria/interactions/src/utils.ts b/packages/@react-aria/interactions/src/utils.ts index 78185e63f3b..f31da638bc9 100644 --- a/packages/@react-aria/interactions/src/utils.ts +++ b/packages/@react-aria/interactions/src/utils.ts @@ -11,7 +11,7 @@ */ import {FocusableElement} from '@react-types/shared'; -import {focusWithoutScrolling, getOwnerWindow, isFocusable, useLayoutEffect, useStableCallback} from '@react-aria/utils'; +import {focusWithoutScrolling, getOwnerWindow, isFocusable, useEffectEvent, useLayoutEffect} from '@react-aria/utils'; import {FocusEvent as ReactFocusEvent, SyntheticEvent, useCallback, useRef} from 'react'; // Turn a native event into a React synthetic event. @@ -48,7 +48,7 @@ export function useSyntheticBlurEvent(onBlur: }; }, []); - let dispatchBlur = useStableCallback((e: ReactFocusEvent) => { + let dispatchBlur = useEffectEvent((e: ReactFocusEvent) => { onBlur?.(e); }); diff --git a/packages/@react-aria/menu/src/useSubmenuTrigger.ts b/packages/@react-aria/menu/src/useSubmenuTrigger.ts index 2cdd991188a..590ce43a213 100644 --- a/packages/@react-aria/menu/src/useSubmenuTrigger.ts +++ b/packages/@react-aria/menu/src/useSubmenuTrigger.ts @@ -14,7 +14,7 @@ import {AriaMenuItemProps} from './useMenuItem'; import {AriaMenuOptions} from './useMenu'; import type {AriaPopoverProps, OverlayProps} from '@react-aria/overlays'; import {FocusableElement, FocusStrategy, KeyboardEvent, Node, PressEvent, RefObject} from '@react-types/shared'; -import {focusWithoutScrolling, useEvent, useId, useLayoutEffect, useStableCallback} from '@react-aria/utils'; +import {focusWithoutScrolling, useEffectEvent, useEvent, useId, useLayoutEffect} from '@react-aria/utils'; import type {SubmenuTriggerState} from '@react-stately/menu'; import {useCallback, useRef} from 'react'; import {useLocale} from '@react-aria/i18n'; @@ -81,12 +81,12 @@ export function useSubmenuTrigger(props: AriaSubmenuTriggerProps, state: Subm } }, [openTimeout]); - let onSubmenuOpen = useStableCallback((focusStrategy?: FocusStrategy) => { + let onSubmenuOpen = useEffectEvent((focusStrategy?: FocusStrategy) => { cancelOpenTimeout(); state.open(focusStrategy); }); - let onSubmenuClose = useStableCallback(() => { + let onSubmenuClose = useEffectEvent(() => { cancelOpenTimeout(); state.close(); }); diff --git a/packages/@react-aria/selection/src/useSelectableCollection.ts b/packages/@react-aria/selection/src/useSelectableCollection.ts index 72c0275697f..4259dc51132 100644 --- a/packages/@react-aria/selection/src/useSelectableCollection.ts +++ b/packages/@react-aria/selection/src/useSelectableCollection.ts @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, focusWithoutScrolling, getActiveElement, isCtrlKeyPressed, isTabbable, mergeProps, scrollIntoView, scrollIntoViewport, useEvent, useRouter, useStableCallback, useUpdateLayoutEffect} from '@react-aria/utils'; +import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, focusWithoutScrolling, getActiveElement, isCtrlKeyPressed, isTabbable, mergeProps, scrollIntoView, scrollIntoViewport, useEffectEvent, useEvent, useRouter, useUpdateLayoutEffect} from '@react-aria/utils'; import {dispatchVirtualFocus, getFocusableTreeWalker, moveVirtualFocus} from '@react-aria/focus'; import {DOMAttributes, FocusableElement, FocusStrategy, Key, KeyboardDelegate, RefObject} from '@react-types/shared'; import {flushSync} from 'react-dom'; @@ -416,7 +416,7 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions } }); - let updateActiveDescendant = useStableCallback(() => { + let updateActiveDescendant = useEffectEvent(() => { let keyToFocus = delegate.getFirstKey?.() ?? null; // If no focusable items exist in the list, make sure to clear any activedescendant that may still exist and move focus back to @@ -447,7 +447,7 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions }, [manager.collection, updateActiveDescendant]); - let resetFocusFirstFlag = useStableCallback(() => { + let resetFocusFirstFlag = useEffectEvent(() => { // If user causes the focused key to change in any other way, clear shouldVirtualFocusFirst so we don't // accidentally move focus from under them. Skip this if the collection was empty because we might be in a load // state and will still want to focus the first item after load diff --git a/packages/@react-aria/spinbutton/src/useSpinButton.ts b/packages/@react-aria/spinbutton/src/useSpinButton.ts index 3115972a17f..dada685076b 100644 --- a/packages/@react-aria/spinbutton/src/useSpinButton.ts +++ b/packages/@react-aria/spinbutton/src/useSpinButton.ts @@ -16,7 +16,7 @@ import {DOMAttributes, InputBase, RangeInputBase, Validation, ValueBase} from '@ // @ts-ignore import intlMessages from '../intl/*.json'; import {useCallback, useEffect, useRef} from 'react'; -import {useGlobalListeners, useStableCallback} from '@react-aria/utils'; +import {useEffectEvent, useGlobalListeners} from '@react-aria/utils'; import {useLocalizedStringFormatter} from '@react-aria/i18n'; @@ -153,7 +153,7 @@ export function useSpinButton( prevTouchPosition.current = touchPosition; }, []); - const onIncrementPressStart = useStableCallback( + const onIncrementPressStart = useEffectEvent( (initialStepDelay: number) => { clearAsync(); isSpinning.current = true; @@ -170,7 +170,7 @@ export function useSpinButton( } ); - const onDecrementPressStart = useStableCallback( + const onDecrementPressStart = useEffectEvent( (initialStepDelay: number) => { clearAsync(); isSpinning.current = true; diff --git a/packages/@react-aria/table/src/useTableColumnResize.ts b/packages/@react-aria/table/src/useTableColumnResize.ts index af10e371157..0a8c956ab13 100644 --- a/packages/@react-aria/table/src/useTableColumnResize.ts +++ b/packages/@react-aria/table/src/useTableColumnResize.ts @@ -18,7 +18,7 @@ import {getColumnHeaderId} from './utils'; import {GridNode} from '@react-types/grid'; // @ts-ignore import intlMessages from '../intl/*.json'; -import {mergeProps, useDescription, useId, useStableCallback} from '@react-aria/utils'; +import {mergeProps, useDescription, useEffectEvent, useId} from '@react-aria/utils'; import {TableColumnResizeState} from '@react-stately/table'; import {useLocale, useLocalizedStringFormatter} from '@react-aria/i18n'; import {useVisuallyHidden} from '@react-aria/visually-hidden'; @@ -88,7 +88,7 @@ export function useTableColumnResize(props: AriaTableColumnResizeProps, st } }); - let startResize = useStableCallback((item) => { + let startResize = useEffectEvent((item) => { if (!isResizingRef.current) { lastSize.current = state.updateResizedColumns(item.key, state.getColumnWidth(item.key)); state.startResize(item.key); @@ -98,13 +98,13 @@ export function useTableColumnResize(props: AriaTableColumnResizeProps, st isResizingRef.current = true; }); - let resize = useStableCallback((item, newWidth) => { + let resize = useEffectEvent((item, newWidth) => { let sizes = state.updateResizedColumns(item.key, newWidth); onResize?.(sizes); lastSize.current = sizes; }); - let endResize = useStableCallback((item) => { + let endResize = useEffectEvent((item) => { if (isResizingRef.current) { if (lastSize.current == null) { lastSize.current = state.updateResizedColumns(item.key, state.getColumnWidth(item.key)); diff --git a/packages/@react-aria/toast/src/useToastRegion.ts b/packages/@react-aria/toast/src/useToastRegion.ts index 3c5939ff0cc..04d6a8dceed 100644 --- a/packages/@react-aria/toast/src/useToastRegion.ts +++ b/packages/@react-aria/toast/src/useToastRegion.ts @@ -11,7 +11,7 @@ */ import {AriaLabelingProps, DOMAttributes, FocusableElement, RefObject} from '@react-types/shared'; -import {focusWithoutScrolling, mergeProps, useLayoutEffect, useStableCallback} from '@react-aria/utils'; +import {focusWithoutScrolling, mergeProps, useEffectEvent, useLayoutEffect} from '@react-aria/utils'; import {getInteractionModality, useFocusWithin, useHover} from '@react-aria/interactions'; // @ts-ignore import intlMessages from '../intl/*.json'; @@ -46,7 +46,7 @@ export function useToastRegion(props: AriaToastRegionProps, state: ToastState let isHovered = useRef(false); let isFocused = useRef(false); - let updateTimers = useStableCallback(() => { + let updateTimers = useEffectEvent(() => { if (isHovered.current || isFocused.current) { state.pauseAll(); } else { diff --git a/packages/@react-aria/utils/src/index.ts b/packages/@react-aria/utils/src/index.ts index 36e1c5dc003..9da3461dd5b 100644 --- a/packages/@react-aria/utils/src/index.ts +++ b/packages/@react-aria/utils/src/index.ts @@ -41,7 +41,7 @@ export {useValueEffect} from './useValueEffect'; export {scrollIntoView, scrollIntoViewport} from './scrollIntoView'; export {clamp, snapValueToStep} from '@react-stately/utils'; export {isVirtualClick, isVirtualPointerEvent} from './isVirtualEvent'; -export {useEffectEvent, useStableCallback} from './useEffectEvent'; +export {useEffectEvent} from './useEffectEvent'; export {useDeepMemo} from './useDeepMemo'; export {useFormReset} from './useFormReset'; export {useLoadMore} from './useLoadMore'; diff --git a/packages/@react-aria/utils/src/useEffectEvent.ts b/packages/@react-aria/utils/src/useEffectEvent.ts index a3569af2494..20b89d03038 100644 --- a/packages/@react-aria/utils/src/useEffectEvent.ts +++ b/packages/@react-aria/utils/src/useEffectEvent.ts @@ -28,7 +28,3 @@ export function useEffectEvent(fn?: T): T { return f?.(...args); }, []); } - -export function useStableCallback(fn?: T): T { - return useEffectEvent(fn); -} diff --git a/packages/@react-aria/utils/src/useValueEffect.ts b/packages/@react-aria/utils/src/useValueEffect.ts index f83b1f9e3fb..a8c90397aea 100644 --- a/packages/@react-aria/utils/src/useValueEffect.ts +++ b/packages/@react-aria/utils/src/useValueEffect.ts @@ -11,7 +11,7 @@ */ import {Dispatch, MutableRefObject, useRef, useState} from 'react'; -import {useLayoutEffect, useStableCallback} from './'; +import {useEffectEvent, useLayoutEffect} from './'; type SetValueAction = (prev: S) => Generator; @@ -25,7 +25,7 @@ export function useValueEffect(defaultValue: S | (() => S)): [S, Dispatch { + let nextRef = useEffectEvent(() => { if (!effect.current) { return; } @@ -55,7 +55,7 @@ export function useValueEffect(defaultValue: S | (() => S)): [S, Dispatch { + let queue = useEffectEvent(fn => { effect.current = fn(value); nextRef(); }); diff --git a/packages/@react-aria/virtualizer/src/ScrollView.tsx b/packages/@react-aria/virtualizer/src/ScrollView.tsx index efaf24c4252..7c4988a5b3e 100644 --- a/packages/@react-aria/virtualizer/src/ScrollView.tsx +++ b/packages/@react-aria/virtualizer/src/ScrollView.tsx @@ -25,7 +25,7 @@ import React, { useState } from 'react'; import {Rect, Size} from '@react-stately/virtualizer'; -import {useEvent, useLayoutEffect, useObjectRef, useResizeObserver, useStableCallback} from '@react-aria/utils'; +import {useEffectEvent, useEvent, useLayoutEffect, useObjectRef, useResizeObserver} from '@react-aria/utils'; import {useLocale} from '@react-aria/i18n'; interface ScrollViewProps extends HTMLAttributes { @@ -157,7 +157,7 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject { + let updateSize = useEffectEvent((flush: typeof flushSync) => { let dom = ref.current; if (!dom || isUpdatingSize.current) { return; diff --git a/packages/@react-spectrum/s2/src/Tabs.tsx b/packages/@react-spectrum/s2/src/Tabs.tsx index cc252b41e0b..329a6284daa 100644 --- a/packages/@react-spectrum/s2/src/Tabs.tsx +++ b/packages/@react-spectrum/s2/src/Tabs.tsx @@ -33,7 +33,7 @@ import {CollectionBuilder} from '@react-aria/collections'; import {createContext, forwardRef, ReactNode, useCallback, useContext, useEffect, useMemo, useRef, useState} from 'react'; import {getAllowedOverrides, StyleProps, StylesPropWithHeight, UnsafeStyles} from './style-utils' with {type: 'macro'}; import {IconContext} from './Icon'; -import {inertValue, useId, useLabels, useLayoutEffect, useResizeObserver, useStableCallback} from '@react-aria/utils'; +import {inertValue, useEffectEvent, useId, useLabels, useLayoutEffect, useResizeObserver} from '@react-aria/utils'; import {Picker, PickerItem} from './TabsPicker'; import {Text, TextContext} from './Content'; import {useControlledState} from '@react-stately/utils'; @@ -216,7 +216,7 @@ export function TabList(props: TabListProps): ReactNode | n if (showTabs) { return ; } - + return (
{listRef &&
@@ -628,7 +628,7 @@ let CollapsingTabs = ({collection, containerRef, ...props}: {collection: Collect let children = useMemo(() => [...collection], [collection]); let listRef = useRef(null); - let updateOverflow = useStableCallback(() => { + let updateOverflow = useEffectEvent(() => { if (orientation === 'vertical' || !listRef.current || !containerRef?.current) { return; } @@ -664,7 +664,7 @@ let CollapsingTabs = ({collection, containerRef, ...props}: {collection: Collect useEffect(() => { // Recalculate visible tags when fonts are loaded. document.fonts?.ready.then(() => updateOverflow()); - + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); let menuId = useId(); diff --git a/packages/@react-spectrum/s2/src/TagGroup.tsx b/packages/@react-spectrum/s2/src/TagGroup.tsx index 4dd33259ae1..2beaaa2bcf0 100644 --- a/packages/@react-spectrum/s2/src/TagGroup.tsx +++ b/packages/@react-spectrum/s2/src/TagGroup.tsx @@ -41,7 +41,7 @@ import {FormContext, useFormProps} from './Form'; import {forwardRefType} from './types'; import {IconContext} from './Icon'; import {ImageContext} from './Image'; -import {inertValue, useId, useLayoutEffect, useResizeObserver, useStableCallback} from '@react-aria/utils'; +import {inertValue, useEffectEvent, useId, useLayoutEffect, useResizeObserver} from '@react-aria/utils'; // @ts-ignore import intlMessages from '../intl/*.json'; import {pressScale} from './pressScale'; @@ -146,7 +146,7 @@ function TagGroupInner({ [collection, tagState.visibleTagCount, isCollapsed] ); - let updateVisibleTagCount = useStableCallback(() => { + let updateVisibleTagCount = useEffectEvent(() => { if (maxRows == null) { setTagState({visibleTagCount: collection.size, showCollapseButton: false}); } @@ -229,7 +229,7 @@ function TagGroupInner({ useEffect(() => { // Recalculate visible tags when fonts are loaded. document.fonts?.ready.then(() => updateVisibleTagCount()); - + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); let handlePressCollapse = () => { From 3f9b6c9bb77e7cdbc534cd96423c818a2f18a807 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Tue, 14 Oct 2025 16:18:02 +1100 Subject: [PATCH 05/26] fix all rules of hooks and exhaustive dependencies --- .../actiongroup/src/useActionGroupItem.ts | 2 +- .../autocomplete/src/useAutocomplete.ts | 14 +- packages/@react-aria/dnd/src/useClipboard.ts | 2 +- packages/@react-aria/dnd/src/useDrop.ts | 2 +- .../@react-aria/form/src/useFormValidation.ts | 2 +- .../grid/src/useGridSelectionAnnouncement.ts | 20 +- .../interactions/src/useInteractOutside.ts | 2 +- .../@react-aria/interactions/src/useMove.ts | 128 +++---- .../@react-aria/interactions/src/usePress.ts | 313 ++++++++++-------- .../@react-aria/interactions/src/utils.ts | 10 +- .../menu/src/useSafelyMouseToSubmenu.ts | 2 +- .../@react-aria/menu/src/useSubmenuTrigger.ts | 10 +- .../selection/src/useSelectableCollection.ts | 59 ++-- .../spinbutton/src/useSpinButton.ts | 47 ++- .../table/src/useTableColumnResize.ts | 50 +-- .../textfield/src/useFormattedTextField.ts | 2 +- .../@react-aria/toast/src/useToastRegion.ts | 10 +- packages/@react-aria/utils/src/useEvent.ts | 2 +- .../@react-aria/utils/src/useFormReset.ts | 2 +- .../utils/src/useLoadMoreSentinel.ts | 2 +- .../utils/src/useResizeObserver.ts | 12 +- .../@react-aria/utils/src/useUpdateEffect.ts | 8 +- .../@react-aria/utils/src/useValueEffect.ts | 26 +- .../virtualizer/src/ScrollView.tsx | 11 +- packages/@react-spectrum/s2/src/CardView.tsx | 11 +- packages/@react-spectrum/s2/src/CoachMark.tsx | 1 + packages/@react-spectrum/s2/src/Skeleton.tsx | 2 +- packages/@react-spectrum/s2/src/Tabs.tsx | 20 +- packages/@react-spectrum/s2/src/TagGroup.tsx | 13 +- .../table/stories/Table.stories.tsx | 7 +- .../dev/s2-docs/src/VisualExampleClient.tsx | 1 + .../src/HiddenDateInput.tsx | 10 +- .../react-aria-components/test/Button.test.js | 1 + 33 files changed, 442 insertions(+), 362 deletions(-) diff --git a/packages/@react-aria/actiongroup/src/useActionGroupItem.ts b/packages/@react-aria/actiongroup/src/useActionGroupItem.ts index 9f33010a1eb..5cb90c01764 100644 --- a/packages/@react-aria/actiongroup/src/useActionGroupItem.ts +++ b/packages/@react-aria/actiongroup/src/useActionGroupItem.ts @@ -54,7 +54,7 @@ export function useActionGroupItem(props: AriaActionGroupItemProps, state: Li return () => { onRemovedWithFocus(); }; - }, [onRemovedWithFocus]); + }, []); return { buttonProps: mergeProps(buttonProps, { diff --git a/packages/@react-aria/autocomplete/src/useAutocomplete.ts b/packages/@react-aria/autocomplete/src/useAutocomplete.ts index 313615d5977..f2a70b86032 100644 --- a/packages/@react-aria/autocomplete/src/useAutocomplete.ts +++ b/packages/@react-aria/autocomplete/src/useAutocomplete.ts @@ -106,7 +106,7 @@ export function useAutocomplete(props: AriaAutocompleteOptions, state: Aut return () => clearTimeout(timeout.current); }, []); - let updateActiveDescendant = useEffectEvent((e: Event) => { + let updateActiveDescendant = useCallback((e: Event) => { // Ensure input is focused if the user clicks on the collection directly. if (!e.isTrusted && shouldUseVirtualFocus && inputRef.current && getActiveElement(getOwnerDocument(inputRef.current)) !== inputRef.current) { inputRef.current.focus(); @@ -138,7 +138,7 @@ export function useAutocomplete(props: AriaAutocompleteOptions, state: Aut } delayNextActiveDescendant.current = false; - }); + }, [shouldUseVirtualFocus, inputRef, collectionRef, state]); let callbackRef = useCallback((collectionNode) => { if (collectionNode != null) { @@ -165,7 +165,7 @@ export function useAutocomplete(props: AriaAutocompleteOptions, state: Aut // Make sure to memo so that React doesn't keep registering a new event listeners on every rerender of the wrapped collection let mergedCollectionRef = useObjectRef(useMemo(() => mergeRefs(collectionRef, callbackRef), [collectionRef, callbackRef])); - let focusFirstItem = useEffectEvent(() => { + let focusFirstItem = useCallback(() => { delayNextActiveDescendant.current = true; collectionRef.current?.dispatchEvent( new CustomEvent(FOCUS_EVENT, { @@ -176,9 +176,9 @@ export function useAutocomplete(props: AriaAutocompleteOptions, state: Aut } }) ); - }); + }, [collectionRef]); - let clearVirtualFocus = useEffectEvent((clearFocusKey?: boolean) => { + let clearVirtualFocus = useCallback((clearFocusKey?: boolean) => { moveVirtualFocus(getActiveElement()); queuedActiveDescendant.current = null; state.setFocusedNodeId(null); @@ -192,7 +192,7 @@ export function useAutocomplete(props: AriaAutocompleteOptions, state: Aut clearTimeout(timeout.current); delayNextActiveDescendant.current = false; collectionRef.current?.dispatchEvent(clearFocusEvent); - }); + }, [collectionRef, state]); let lastInputType = useRef(''); useEvent(inputRef, 'input', e => { @@ -346,7 +346,7 @@ export function useAutocomplete(props: AriaAutocompleteOptions, state: Aut return () => { document.removeEventListener('keyup', onKeyUpCapture, true); }; - }, [onKeyUpCapture]); + }, []); let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-aria/autocomplete'); let collectionProps = useLabels({ diff --git a/packages/@react-aria/dnd/src/useClipboard.ts b/packages/@react-aria/dnd/src/useClipboard.ts index 53f23ee148b..fb883fa598e 100644 --- a/packages/@react-aria/dnd/src/useClipboard.ts +++ b/packages/@react-aria/dnd/src/useClipboard.ts @@ -143,7 +143,7 @@ export function useClipboard(options: ClipboardProps): ClipboardResult { addGlobalEventListener('beforepaste', onBeforePaste), addGlobalEventListener('paste', onPaste) ); - }, [isDisabled, onBeforeCopy, onCopy, onBeforeCut, onCut, onBeforePaste, onPaste]); + }, [isDisabled]); return { clipboardProps: focusProps diff --git a/packages/@react-aria/dnd/src/useDrop.ts b/packages/@react-aria/dnd/src/useDrop.ts index 09985b2d7a5..31b2204546b 100644 --- a/packages/@react-aria/dnd/src/useDrop.ts +++ b/packages/@react-aria/dnd/src/useDrop.ts @@ -339,7 +339,7 @@ export function useDrop(options: DropOptions): DropResult { onDrop: onKeyboardDrop, onDropActivate }); - }, [isDisabled, ref, getDropOperationKeyboard, onDropEnter, onDropExit, onKeyboardDrop, onDropActivate]); + }, [isDisabled, ref]); let {dropProps} = useVirtualDrop(); if (isDisabled) { diff --git a/packages/@react-aria/form/src/useFormValidation.ts b/packages/@react-aria/form/src/useFormValidation.ts index 034814fd6f2..b4699bf8046 100644 --- a/packages/@react-aria/form/src/useFormValidation.ts +++ b/packages/@react-aria/form/src/useFormValidation.ts @@ -112,7 +112,7 @@ export function useFormValidation(props: FormValidationProps, state: FormV form.reset = reset; } }; - }, [ref, onInvalid, onChange, onReset, validationBehavior]); + }, [ref, validationBehavior]); } function getValidity(input: ValidatableElement) { diff --git a/packages/@react-aria/grid/src/useGridSelectionAnnouncement.ts b/packages/@react-aria/grid/src/useGridSelectionAnnouncement.ts index c3d84564c32..5d129aedba9 100644 --- a/packages/@react-aria/grid/src/useGridSelectionAnnouncement.ts +++ b/packages/@react-aria/grid/src/useGridSelectionAnnouncement.ts @@ -15,9 +15,9 @@ import {Collection, Key, Node, Selection} from '@react-types/shared'; // @ts-ignore import intlMessages from '../intl/*.json'; import {SelectionManager} from '@react-stately/selection'; -import {useEffectEvent, useUpdateEffect} from '@react-aria/utils'; +import {useCallback, useRef} from 'react'; import {useLocalizedStringFormatter} from '@react-aria/i18n'; -import {useRef} from 'react'; +import {useUpdateEffect} from '@react-aria/utils'; export interface GridSelectionAnnouncementProps { /** @@ -46,7 +46,7 @@ export function useGridSelectionAnnouncement(props: GridSelectionAnnouncement // We do this using an ARIA live region. let selection = state.selectionManager.rawSelection; let lastSelection = useRef(selection); - let announceSelectionChange = useEffectEvent(() => { + let announceSelectionChange = useCallback(() => { if (!state.selectionManager.isFocused || selection === lastSelection.current) { lastSelection.current = selection; @@ -101,8 +101,18 @@ export function useGridSelectionAnnouncement(props: GridSelectionAnnouncement } lastSelection.current = selection; - }); - + }, [ + selection, + state.selectionManager.selectedKeys, + state.selectionManager.isFocused, + state.selectionManager.selectionBehavior, + state.selectionManager.selectionMode, + state.collection, + getRowText, + stringFormatter + ]); + + // useUpdateEffect will handle using useEffectEvent, no need to stabilize anything on this end useUpdateEffect(() => { if (state.selectionManager.isFocused) { announceSelectionChange(); diff --git a/packages/@react-aria/interactions/src/useInteractOutside.ts b/packages/@react-aria/interactions/src/useInteractOutside.ts index 94c1e65a46c..9f413630ca3 100644 --- a/packages/@react-aria/interactions/src/useInteractOutside.ts +++ b/packages/@react-aria/interactions/src/useInteractOutside.ts @@ -111,7 +111,7 @@ export function useInteractOutside(props: InteractOutsideProps): void { documentObject.removeEventListener('touchend', onTouchEnd, true); }; } - }, [ref, isDisabled, onPointerDown, triggerInteractOutside]); + }, [ref, isDisabled]); } function isValidEvent(event, ref) { diff --git a/packages/@react-aria/interactions/src/useMove.ts b/packages/@react-aria/interactions/src/useMove.ts index f7f32acfdbb..24584260cfb 100644 --- a/packages/@react-aria/interactions/src/useMove.ts +++ b/packages/@react-aria/interactions/src/useMove.ts @@ -12,7 +12,7 @@ import {disableTextSelection, restoreTextSelection} from './textSelection'; import {DOMAttributes, MoveEvents, PointerType} from '@react-types/shared'; -import React, {useMemo, useRef} from 'react'; +import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; import {useEffectEvent, useGlobalListeners} from '@react-aria/utils'; export interface MoveResult { @@ -43,7 +43,7 @@ export function useMove(props: MoveEvents): MoveResult { let {addGlobalListener, removeGlobalListener} = useGlobalListeners(); - let move = useEffectEvent((originalEvent: EventBase, pointerType: PointerType, deltaX: number, deltaY: number) => { + let move = useCallback((originalEvent: EventBase, pointerType: PointerType, deltaX: number, deltaY: number) => { if (deltaX === 0 && deltaY === 0) { return; } @@ -69,9 +69,10 @@ export function useMove(props: MoveEvents): MoveResult { ctrlKey: originalEvent.ctrlKey, altKey: originalEvent.altKey }); - }); + }, [onMoveStart, onMove, state]); + let moveEvent = useEffectEvent(move); - let end = useEffectEvent((originalEvent: EventBase, pointerType: PointerType) => { + let end = useCallback((originalEvent: EventBase, pointerType: PointerType) => { restoreTextSelection(); if (state.current.didMove) { onMoveEnd?.({ @@ -83,57 +84,97 @@ export function useMove(props: MoveEvents): MoveResult { altKey: originalEvent.altKey }); } - }); + }, [onMoveEnd, state]); + let endEvent = useEffectEvent(end); - let moveProps = useMemo(() => { - let moveProps: DOMAttributes = {}; + let [pointerDown, setPointerDown] = useState<'pointer' | 'mouse' | 'touch' | null>(null); + useEffect(() => { + if (pointerDown === 'pointer') { + let onPointerMove = (e: PointerEvent) => { + if (e.pointerId === state.current.id) { + let pointerType = (e.pointerType || 'mouse') as PointerType; - let start = () => { - disableTextSelection(); - state.current.didMove = false; - }; + // Problems with PointerEvent#movementX/movementY: + // 1. it is always 0 on macOS Safari. + // 2. On Chrome Android, it's scaled by devicePixelRatio, but not on Chrome macOS + moveEvent(e, pointerType, e.pageX - (state.current.lastPosition?.pageX ?? 0), e.pageY - (state.current.lastPosition?.pageY ?? 0)); + state.current.lastPosition = {pageX: e.pageX, pageY: e.pageY}; + } + }; - if (typeof PointerEvent === 'undefined' && process.env.NODE_ENV === 'test') { + let onPointerUp = (e: PointerEvent) => { + if (e.pointerId === state.current.id) { + let pointerType = (e.pointerType || 'mouse') as PointerType; + endEvent(e, pointerType); + state.current.id = null; + removeGlobalListener(window, 'pointermove', onPointerMove, false); + removeGlobalListener(window, 'pointerup', onPointerUp, false); + removeGlobalListener(window, 'pointercancel', onPointerUp, false); + setPointerDown(null); + } + }; + addGlobalListener(window, 'pointermove', onPointerMove, false); + addGlobalListener(window, 'pointerup', onPointerUp, false); + addGlobalListener(window, 'pointercancel', onPointerUp, false); + } else if (pointerDown === 'mouse' && process.env.NODE_ENV === 'test') { let onMouseMove = (e: MouseEvent) => { if (e.button === 0) { - move(e, 'mouse', e.pageX - (state.current.lastPosition?.pageX ?? 0), e.pageY - (state.current.lastPosition?.pageY ?? 0)); + moveEvent(e, 'mouse', e.pageX - (state.current.lastPosition?.pageX ?? 0), e.pageY - (state.current.lastPosition?.pageY ?? 0)); state.current.lastPosition = {pageX: e.pageX, pageY: e.pageY}; } }; let onMouseUp = (e: MouseEvent) => { if (e.button === 0) { - end(e, 'mouse'); + endEvent(e, 'mouse'); removeGlobalListener(window, 'mousemove', onMouseMove, false); removeGlobalListener(window, 'mouseup', onMouseUp, false); + setPointerDown(null); } }; - moveProps.onMouseDown = (e: React.MouseEvent) => { - if (e.button === 0) { - start(); - e.stopPropagation(); - e.preventDefault(); - state.current.lastPosition = {pageX: e.pageX, pageY: e.pageY}; - addGlobalListener(window, 'mousemove', onMouseMove, false); - addGlobalListener(window, 'mouseup', onMouseUp, false); - } - }; - + addGlobalListener(window, 'mousemove', onMouseMove, false); + addGlobalListener(window, 'mouseup', onMouseUp, false); + } else if (pointerDown === 'touch' && process.env.NODE_ENV === 'test') { let onTouchMove = (e: TouchEvent) => { let touch = [...e.changedTouches].findIndex(({identifier}) => identifier === state.current.id); if (touch >= 0) { let {pageX, pageY} = e.changedTouches[touch]; - move(e, 'touch', pageX - (state.current.lastPosition?.pageX ?? 0), pageY - (state.current.lastPosition?.pageY ?? 0)); + moveEvent(e, 'touch', pageX - (state.current.lastPosition?.pageX ?? 0), pageY - (state.current.lastPosition?.pageY ?? 0)); state.current.lastPosition = {pageX, pageY}; } }; let onTouchEnd = (e: TouchEvent) => { let touch = [...e.changedTouches].findIndex(({identifier}) => identifier === state.current.id); if (touch >= 0) { - end(e, 'touch'); + endEvent(e, 'touch'); state.current.id = null; removeGlobalListener(window, 'touchmove', onTouchMove); removeGlobalListener(window, 'touchend', onTouchEnd); removeGlobalListener(window, 'touchcancel', onTouchEnd); + setPointerDown(null); + } + }; + addGlobalListener(window, 'touchmove', onTouchMove, false); + addGlobalListener(window, 'touchend', onTouchEnd, false); + addGlobalListener(window, 'touchcancel', onTouchEnd, false); + } + }, [pointerDown, addGlobalListener, removeGlobalListener]); + + let moveProps = useMemo(() => { + let moveProps: DOMAttributes = {}; + + let start = () => { + disableTextSelection(); + state.current.didMove = false; + }; + + if (typeof PointerEvent === 'undefined' && process.env.NODE_ENV === 'test') { + moveProps.onMouseDown = (e: React.MouseEvent) => { + if (e.button === 0) { + start(); + e.stopPropagation(); + e.preventDefault(); + state.current.lastPosition = {pageX: e.pageX, pageY: e.pageY}; + setPointerDown('mouse'); } }; moveProps.onTouchStart = (e: React.TouchEvent) => { @@ -147,34 +188,9 @@ export function useMove(props: MoveEvents): MoveResult { e.preventDefault(); state.current.lastPosition = {pageX, pageY}; state.current.id = identifier; - addGlobalListener(window, 'touchmove', onTouchMove, false); - addGlobalListener(window, 'touchend', onTouchEnd, false); - addGlobalListener(window, 'touchcancel', onTouchEnd, false); + setPointerDown('touch'); }; } else { - let onPointerMove = (e: PointerEvent) => { - if (e.pointerId === state.current.id) { - let pointerType = (e.pointerType || 'mouse') as PointerType; - - // Problems with PointerEvent#movementX/movementY: - // 1. it is always 0 on macOS Safari. - // 2. On Chrome Android, it's scaled by devicePixelRatio, but not on Chrome macOS - move(e, pointerType, e.pageX - (state.current.lastPosition?.pageX ?? 0), e.pageY - (state.current.lastPosition?.pageY ?? 0)); - state.current.lastPosition = {pageX: e.pageX, pageY: e.pageY}; - } - }; - - let onPointerUp = (e: PointerEvent) => { - if (e.pointerId === state.current.id) { - let pointerType = (e.pointerType || 'mouse') as PointerType; - end(e, pointerType); - state.current.id = null; - removeGlobalListener(window, 'pointermove', onPointerMove, false); - removeGlobalListener(window, 'pointerup', onPointerUp, false); - removeGlobalListener(window, 'pointercancel', onPointerUp, false); - } - }; - moveProps.onPointerDown = (e: React.PointerEvent) => { if (e.button === 0 && state.current.id == null) { start(); @@ -182,9 +198,7 @@ export function useMove(props: MoveEvents): MoveResult { e.preventDefault(); state.current.lastPosition = {pageX: e.pageX, pageY: e.pageY}; state.current.id = e.pointerId; - addGlobalListener(window, 'pointermove', onPointerMove, false); - addGlobalListener(window, 'pointerup', onPointerUp, false); - addGlobalListener(window, 'pointercancel', onPointerUp, false); + setPointerDown('pointer'); } }; } @@ -225,7 +239,7 @@ export function useMove(props: MoveEvents): MoveResult { }; return moveProps; - }, [state, addGlobalListener, removeGlobalListener, move, end]); + }, [state, move, end]); return {moveProps}; } diff --git a/packages/@react-aria/interactions/src/usePress.ts b/packages/@react-aria/interactions/src/usePress.ts index 42576b36095..fce29504349 100644 --- a/packages/@react-aria/interactions/src/usePress.ts +++ b/packages/@react-aria/interactions/src/usePress.ts @@ -36,7 +36,7 @@ import {disableTextSelection, restoreTextSelection} from './textSelection'; import {DOMAttributes, FocusableElement, PressEvent as IPressEvent, PointerType, PressEvents, RefObject} from '@react-types/shared'; import {flushSync} from 'react-dom'; import {PressResponderContext} from './context'; -import {MouseEvent as RMouseEvent, TouchEvent as RTouchEvent, useContext, useEffect, useMemo, useRef, useState} from 'react'; +import {MouseEvent as RMouseEvent, TouchEvent as RTouchEvent, useCallback, useContext, useEffect, useMemo, useRef, useState} from 'react'; export interface PressProps extends PressEvents { /** Whether the target is in a controlled press state (e.g. an overlay it triggers is open). */ @@ -197,7 +197,7 @@ export function usePress(props: PressHookProps): PressResult { let {addGlobalListener, removeAllGlobalListeners} = useGlobalListeners(); - let triggerPressStart = useEffectEvent((originalEvent: EventBase, pointerType: PointerType) => { + let triggerPressStart = useCallback((originalEvent: EventBase, pointerType: PointerType) => { let state = ref.current; if (isDisabled || state.didFirePressStart) { return false; @@ -219,9 +219,9 @@ export function usePress(props: PressHookProps): PressResult { state.didFirePressStart = true; setPressed(true); return shouldStopPropagation; - }); + }, [isDisabled, onPressStart, onPressChange]); - let triggerPressEnd = useEffectEvent((originalEvent: EventBase, pointerType: PointerType, wasPressed = true) => { + let triggerPressEnd = useCallback((originalEvent: EventBase, pointerType: PointerType, wasPressed = true) => { let state = ref.current; if (!state.didFirePressStart) { return false; @@ -251,9 +251,10 @@ export function usePress(props: PressHookProps): PressResult { state.isTriggeringEvent = false; return shouldStopPropagation; - }); + }, [isDisabled, onPressEnd, onPressChange, onPress]); + let triggerPressEndEvent = useEffectEvent(triggerPressEnd); - let triggerPressUp = useEffectEvent((originalEvent: EventBase, pointerType: PointerType) => { + let triggerPressUp = useCallback((originalEvent: EventBase, pointerType: PointerType) => { let state = ref.current; if (isDisabled) { return false; @@ -268,15 +269,17 @@ export function usePress(props: PressHookProps): PressResult { } return true; - }); + }, [isDisabled, onPressUp]); + let triggerPressUpEvent = useEffectEvent(triggerPressUp); - let cancel = useEffectEvent((e: EventBase) => { + let cancel = useCallback((e: EventBase) => { let state = ref.current; if (state.isPressed && state.target) { if (state.didFirePressStart && state.pointerType != null) { triggerPressEnd(createEvent(state.target, e), state.pointerType, false); } state.isPressed = false; + setIsPointerPressed(null); state.isOverTarget = false; state.activePointerId = null; state.pointerType = null; @@ -289,23 +292,24 @@ export function usePress(props: PressHookProps): PressResult { } state.disposables = []; } - }); + }, [allowTextSelectionOnPress, removeAllGlobalListeners, triggerPressEnd]); + let cancelEvent = useEffectEvent(cancel); - let cancelOnPointerExit = useEffectEvent((e: EventBase) => { + let cancelOnPointerExit = useCallback((e: EventBase) => { if (shouldCancelOnPointerExit) { cancel(e); } - }); + }, [shouldCancelOnPointerExit, cancel]); - let triggerClick = useEffectEvent((e: RMouseEvent) => { + let triggerClick = useCallback((e: RMouseEvent) => { if (isDisabled) { return; } onClick?.(e); - }); + }, [isDisabled, onClick]); - let triggerSyntheticClick = useEffectEvent((e: KeyboardEvent | TouchEvent, target: FocusableElement) => { + let triggerSyntheticClick = useCallback((e: KeyboardEvent | TouchEvent, target: FocusableElement) => { if (isDisabled) { return; } @@ -320,7 +324,150 @@ export function usePress(props: PressHookProps): PressResult { setEventTarget(event, target); onClick(createSyntheticEvent(event)); } - }); + }, [isDisabled, onClick]); + let triggerSyntheticClickEvent = useEffectEvent(triggerSyntheticClick); + + let [isElemKeyPressed, setIsElemKeyPressed] = useState(false); + useEffect(() => { + let state = ref.current; + if (isElemKeyPressed) { + let onKeyUp = (e: KeyboardEvent) => { + if (state.isPressed && state.target && isValidKeyboardEvent(e, state.target)) { + if (shouldPreventDefaultKeyboard(getEventTarget(e), e.key)) { + e.preventDefault(); + } + + let target = getEventTarget(e); + let wasPressed = nodeContains(state.target, getEventTarget(e)); + triggerPressEndEvent(createEvent(state.target, e), 'keyboard', wasPressed); + if (wasPressed) { + triggerSyntheticClickEvent(e, state.target); + } + removeAllGlobalListeners(); + + // If a link was triggered with a key other than Enter, open the URL ourselves. + // This means the link has a role override, and the default browser behavior + // only applies when using the Enter key. + if (e.key !== 'Enter' && isHTMLAnchorLink(state.target) && nodeContains(state.target, target) && !e[LINK_CLICKED]) { + // Store a hidden property on the event so we only trigger link click once, + // even if there are multiple usePress instances attached to the element. + e[LINK_CLICKED] = true; + openLink(state.target, e, false); + } + + state.isPressed = false; + setIsElemKeyPressed(false); + state.metaKeyEvents?.delete(e.key); + } else if (e.key === 'Meta' && state.metaKeyEvents?.size) { + // If we recorded keydown events that occurred while the Meta key was pressed, + // and those haven't received keyup events already, fire keyup events ourselves. + // See comment above for more info about the macOS bug causing this. + let events = state.metaKeyEvents; + state.metaKeyEvents = undefined; + for (let event of events.values()) { + state.target?.dispatchEvent(new KeyboardEvent('keyup', event)); + } + } + }; + // Focus may move before the key up event, so register the event on the document + // instead of the same element where the key down event occurred. Make it capturing so that it will trigger + // before stopPropagation from useKeyboard on a child element may happen and thus we can still call triggerPress for the parent element. + let originalTarget = state.target; + let pressUp = (e) => { + if (originalTarget && isValidKeyboardEvent(e, originalTarget) && !e.repeat && nodeContains(originalTarget, getEventTarget(e)) && state.target) { + triggerPressUpEvent(createEvent(state.target, e), 'keyboard'); + } + }; + addGlobalListener(getOwnerDocument(state.target), 'keyup', chain(pressUp, onKeyUp), true); + } + }, [isElemKeyPressed, addGlobalListener, removeAllGlobalListeners]); + + let [isPointerPressed, setIsPointerPressed] = useState<'pointer' | 'mouse' | 'touch' | null>(null); + useEffect(() => { + let state = ref.current; + if (isPointerPressed === 'pointer') { + let onPointerUp = (e: PointerEvent) => { + if (e.pointerId === state.activePointerId && state.isPressed && e.button === 0 && state.target) { + if (nodeContains(state.target, getEventTarget(e)) && state.pointerType != null) { + // Wait for onClick to fire onPress. This avoids browser issues when the DOM + // is mutated between onPointerUp and onClick, and is more compatible with third party libraries. + // https://github.com/adobe/react-spectrum/issues/1513 + // https://issues.chromium.org/issues/40732224 + // However, iOS and Android do not focus or fire onClick after a long press. + // We work around this by triggering a click ourselves after a timeout. + // This timeout is canceled during the click event in case the real one fires first. + // The timeout must be at least 32ms, because Safari on iOS delays the click event on + // non-form elements without certain ARIA roles (for hover emulation). + // https://github.com/WebKit/WebKit/blob/dccfae42bb29bd4bdef052e469f604a9387241c0/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm#L875-L892 + let clicked = false; + let timeout = setTimeout(() => { + if (state.isPressed && state.target instanceof HTMLElement) { + if (clicked) { + cancelEvent(e); + } else { + focusWithoutScrolling(state.target); + state.target.click(); + } + } + }, 80); + // Use a capturing listener to track if a click occurred. + // If stopPropagation is called it may never reach our handler. + addGlobalListener(e.currentTarget as Document, 'click', () => clicked = true, true); + state.disposables.push(() => clearTimeout(timeout)); + } else { + cancelEvent(e); + } + + // Ignore subsequent onPointerLeave event before onClick on touch devices. + state.isOverTarget = false; + } + }; + + let onPointerCancel = (e: PointerEvent) => { + cancelEvent(e); + }; + + addGlobalListener(getOwnerDocument(state.target), 'pointerup', onPointerUp, false); + addGlobalListener(getOwnerDocument(state.target), 'pointercancel', onPointerCancel, false); + } else if (isPointerPressed === 'mouse' && process.env.NODE_ENV === 'test') { + let onMouseUp = (e: MouseEvent) => { + // Only handle left clicks + if (e.button !== 0) { + return; + } + + if (state.ignoreEmulatedMouseEvents) { + state.ignoreEmulatedMouseEvents = false; + return; + } + + if (state.target && state.target.contains(e.target as Element) && state.pointerType != null) { + // Wait for onClick to fire onPress. This avoids browser issues when the DOM + // is mutated between onMouseUp and onClick, and is more compatible with third party libraries. + } else { + cancelEvent(e); + } + + state.isOverTarget = false; + }; + + addGlobalListener(getOwnerDocument(state.target), 'mouseup', onMouseUp, false); + } else if (isPointerPressed === 'touch' && process.env.NODE_ENV === 'test') { + let onScroll = (e: Event) => { + if (state.isPressed && nodeContains(getEventTarget(e), state.target)) { + cancelEvent({ + currentTarget: state.target, + shiftKey: false, + ctrlKey: false, + metaKey: false, + altKey: false + }); + } + }; + + addGlobalListener(getOwnerWindow(state.target), 'scroll', onScroll, true); + } + }, [isPointerPressed, addGlobalListener]); let pressProps = useMemo(() => { let state = ref.current; @@ -338,20 +485,9 @@ export function usePress(props: PressHookProps): PressResult { if (!state.isPressed && !e.repeat) { state.target = e.currentTarget; state.isPressed = true; + setIsElemKeyPressed(true); state.pointerType = 'keyboard'; shouldStopPropagation = triggerPressStart(e, 'keyboard'); - - // Focus may move before the key up event, so register the event on the document - // instead of the same element where the key down event occurred. Make it capturing so that it will trigger - // before stopPropagation from useKeyboard on a child element may happen and thus we can still call triggerPress for the parent element. - let originalTarget = e.currentTarget; - let pressUp = (e) => { - if (isValidKeyboardEvent(e, originalTarget) && !e.repeat && nodeContains(originalTarget, getEventTarget(e)) && state.target) { - triggerPressUp(createEvent(state.target, e), 'keyboard'); - } - }; - - addGlobalListener(getOwnerDocument(e.currentTarget), 'keyup', chain(pressUp, onKeyUp), true); } if (shouldStopPropagation) { @@ -409,44 +545,6 @@ export function usePress(props: PressHookProps): PressResult { } }; - let onKeyUp = (e: KeyboardEvent) => { - if (state.isPressed && state.target && isValidKeyboardEvent(e, state.target)) { - if (shouldPreventDefaultKeyboard(getEventTarget(e), e.key)) { - e.preventDefault(); - } - - let target = getEventTarget(e); - let wasPressed = nodeContains(state.target, getEventTarget(e)); - triggerPressEnd(createEvent(state.target, e), 'keyboard', wasPressed); - if (wasPressed) { - triggerSyntheticClick(e, state.target); - } - removeAllGlobalListeners(); - - // If a link was triggered with a key other than Enter, open the URL ourselves. - // This means the link has a role override, and the default browser behavior - // only applies when using the Enter key. - if (e.key !== 'Enter' && isHTMLAnchorLink(state.target) && nodeContains(state.target, target) && !e[LINK_CLICKED]) { - // Store a hidden property on the event so we only trigger link click once, - // even if there are multiple usePress instances attached to the element. - e[LINK_CLICKED] = true; - openLink(state.target, e, false); - } - - state.isPressed = false; - state.metaKeyEvents?.delete(e.key); - } else if (e.key === 'Meta' && state.metaKeyEvents?.size) { - // If we recorded keydown events that occurred while the Meta key was pressed, - // and those haven't received keyup events already, fire keyup events ourselves. - // See comment above for more info about the macOS bug causing this. - let events = state.metaKeyEvents; - state.metaKeyEvents = undefined; - for (let event of events.values()) { - state.target?.dispatchEvent(new KeyboardEvent('keyup', event)); - } - } - }; - if (typeof PointerEvent !== 'undefined') { pressProps.onPointerDown = (e) => { // Only handle left clicks, and ignore events that bubbled through portals. @@ -468,6 +566,7 @@ export function usePress(props: PressHookProps): PressResult { let shouldStopPropagation = true; if (!state.isPressed) { state.isPressed = true; + setIsPointerPressed('pointer'); state.isOverTarget = true; state.activePointerId = e.pointerId; state.target = e.currentTarget as FocusableElement; @@ -484,9 +583,6 @@ export function usePress(props: PressHookProps): PressResult { if ('releasePointerCapture' in target) { target.releasePointerCapture(e.pointerId); } - - addGlobalListener(getOwnerDocument(e.currentTarget), 'pointerup', onPointerUp, false); - addGlobalListener(getOwnerDocument(e.currentTarget), 'pointercancel', onPointerCancel, false); } if (shouldStopPropagation) { @@ -538,46 +634,6 @@ export function usePress(props: PressHookProps): PressResult { } }; - let onPointerUp = (e: PointerEvent) => { - if (e.pointerId === state.activePointerId && state.isPressed && e.button === 0 && state.target) { - if (nodeContains(state.target, getEventTarget(e)) && state.pointerType != null) { - // Wait for onClick to fire onPress. This avoids browser issues when the DOM - // is mutated between onPointerUp and onClick, and is more compatible with third party libraries. - // https://github.com/adobe/react-spectrum/issues/1513 - // https://issues.chromium.org/issues/40732224 - // However, iOS and Android do not focus or fire onClick after a long press. - // We work around this by triggering a click ourselves after a timeout. - // This timeout is canceled during the click event in case the real one fires first. - // The timeout must be at least 32ms, because Safari on iOS delays the click event on - // non-form elements without certain ARIA roles (for hover emulation). - // https://github.com/WebKit/WebKit/blob/dccfae42bb29bd4bdef052e469f604a9387241c0/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm#L875-L892 - let clicked = false; - let timeout = setTimeout(() => { - if (state.isPressed && state.target instanceof HTMLElement) { - if (clicked) { - cancel(e); - } else { - focusWithoutScrolling(state.target); - state.target.click(); - } - } - }, 80); - // Use a capturing listener to track if a click occurred. - // If stopPropagation is called it may never reach our handler. - addGlobalListener(e.currentTarget as Document, 'click', () => clicked = true, true); - state.disposables.push(() => clearTimeout(timeout)); - } else { - cancel(e); - } - - // Ignore subsequent onPointerLeave event before onClick on touch devices. - state.isOverTarget = false; - } - }; - - let onPointerCancel = (e: PointerEvent) => { - cancel(e); - }; pressProps.onDragStart = (e) => { if (!nodeContains(e.currentTarget, getEventTarget(e.nativeEvent))) { @@ -603,6 +659,7 @@ export function usePress(props: PressHookProps): PressResult { } state.isPressed = true; + setIsPointerPressed('mouse'); state.isOverTarget = true; state.target = e.currentTarget; state.pointerType = isVirtualClick(e.nativeEvent) ? 'virtual' : 'mouse'; @@ -619,8 +676,6 @@ export function usePress(props: PressHookProps): PressResult { state.disposables.push(dispose); } } - - addGlobalListener(getOwnerDocument(e.currentTarget), 'mouseup', onMouseUp, false); }; pressProps.onMouseEnter = (e) => { @@ -666,27 +721,6 @@ export function usePress(props: PressHookProps): PressResult { } }; - let onMouseUp = (e: MouseEvent) => { - // Only handle left clicks - if (e.button !== 0) { - return; - } - - if (state.ignoreEmulatedMouseEvents) { - state.ignoreEmulatedMouseEvents = false; - return; - } - - if (state.target && state.target.contains(e.target as Element) && state.pointerType != null) { - // Wait for onClick to fire onPress. This avoids browser issues when the DOM - // is mutated between onMouseUp and onClick, and is more compatible with third party libraries. - } else { - cancel(e); - } - - state.isOverTarget = false; - }; - pressProps.onTouchStart = (e) => { if (!nodeContains(e.currentTarget, getEventTarget(e.nativeEvent))) { return; @@ -700,6 +734,7 @@ export function usePress(props: PressHookProps): PressResult { state.ignoreEmulatedMouseEvents = true; state.isOverTarget = true; state.isPressed = true; + setIsPointerPressed('touch'); state.target = e.currentTarget; state.pointerType = 'touch'; @@ -711,8 +746,6 @@ export function usePress(props: PressHookProps): PressResult { if (shouldStopPropagation) { e.stopPropagation(); } - - addGlobalListener(getOwnerWindow(e.currentTarget), 'scroll', onScroll, true); }; pressProps.onTouchMove = (e) => { @@ -768,6 +801,7 @@ export function usePress(props: PressHookProps): PressResult { } state.isPressed = false; + setIsPointerPressed(null); state.activePointerId = null; state.isOverTarget = false; state.ignoreEmulatedMouseEvents = true; @@ -788,18 +822,6 @@ export function usePress(props: PressHookProps): PressResult { } }; - let onScroll = (e: Event) => { - if (state.isPressed && nodeContains(getEventTarget(e), state.target)) { - cancel({ - currentTarget: state.target, - shiftKey: false, - ctrlKey: false, - metaKey: false, - altKey: false - }); - } - }; - pressProps.onDragStart = (e) => { if (!nodeContains(e.currentTarget, getEventTarget(e.nativeEvent))) { return; @@ -811,7 +833,6 @@ export function usePress(props: PressHookProps): PressResult { return pressProps; }, [ - addGlobalListener, isDisabled, preventFocusOnPress, removeAllGlobalListeners, diff --git a/packages/@react-aria/interactions/src/utils.ts b/packages/@react-aria/interactions/src/utils.ts index f31da638bc9..46321a1d4a8 100644 --- a/packages/@react-aria/interactions/src/utils.ts +++ b/packages/@react-aria/interactions/src/utils.ts @@ -11,7 +11,7 @@ */ import {FocusableElement} from '@react-types/shared'; -import {focusWithoutScrolling, getOwnerWindow, isFocusable, useEffectEvent, useLayoutEffect} from '@react-aria/utils'; +import {focusWithoutScrolling, getOwnerWindow, isFocusable, useLayoutEffect} from '@react-aria/utils'; import {FocusEvent as ReactFocusEvent, SyntheticEvent, useCallback, useRef} from 'react'; // Turn a native event into a React synthetic event. @@ -48,10 +48,6 @@ export function useSyntheticBlurEvent(onBlur: }; }, []); - let dispatchBlur = useEffectEvent((e: ReactFocusEvent) => { - onBlur?.(e); - }); - // This function is called during a React onFocus event. return useCallback((e: ReactFocusEvent) => { // React does not fire onBlur when an element is disabled. https://github.com/facebook/react/issues/9142 @@ -73,7 +69,7 @@ export function useSyntheticBlurEvent(onBlur: if (target.disabled) { // For backward compatibility, dispatch a (fake) React synthetic event. let event = createSyntheticEvent>(e); - dispatchBlur(event); + onBlur?.(event); } // We no longer need the MutationObserver once the target is blurred. @@ -96,7 +92,7 @@ export function useSyntheticBlurEvent(onBlur: stateRef.current.observer.observe(target, {attributes: true, attributeFilter: ['disabled']}); } - }, [dispatchBlur]); + }, [onBlur]); } export let ignoreFocusEvent = false; diff --git a/packages/@react-aria/menu/src/useSafelyMouseToSubmenu.ts b/packages/@react-aria/menu/src/useSafelyMouseToSubmenu.ts index 1a43971bbe4..f36eb66261e 100644 --- a/packages/@react-aria/menu/src/useSafelyMouseToSubmenu.ts +++ b/packages/@react-aria/menu/src/useSafelyMouseToSubmenu.ts @@ -174,5 +174,5 @@ export function useSafelyMouseToSubmenu(options: SafelyMouseToSubmenuOptions): v movementsTowardsSubmenuCount.current = ALLOWED_INVALID_MOVEMENTS; }; - }, [isDisabled, isOpen, menuRef, modality, setPreventPointerEvents, onPointerDown, submenuRef]); + }, [isDisabled, isOpen, menuRef, modality, setPreventPointerEvents, submenuRef]); } diff --git a/packages/@react-aria/menu/src/useSubmenuTrigger.ts b/packages/@react-aria/menu/src/useSubmenuTrigger.ts index 590ce43a213..eed89771089 100644 --- a/packages/@react-aria/menu/src/useSubmenuTrigger.ts +++ b/packages/@react-aria/menu/src/useSubmenuTrigger.ts @@ -14,7 +14,7 @@ import {AriaMenuItemProps} from './useMenuItem'; import {AriaMenuOptions} from './useMenu'; import type {AriaPopoverProps, OverlayProps} from '@react-aria/overlays'; import {FocusableElement, FocusStrategy, KeyboardEvent, Node, PressEvent, RefObject} from '@react-types/shared'; -import {focusWithoutScrolling, useEffectEvent, useEvent, useId, useLayoutEffect} from '@react-aria/utils'; +import {focusWithoutScrolling, useEvent, useId, useLayoutEffect} from '@react-aria/utils'; import type {SubmenuTriggerState} from '@react-stately/menu'; import {useCallback, useRef} from 'react'; import {useLocale} from '@react-aria/i18n'; @@ -81,15 +81,15 @@ export function useSubmenuTrigger(props: AriaSubmenuTriggerProps, state: Subm } }, [openTimeout]); - let onSubmenuOpen = useEffectEvent((focusStrategy?: FocusStrategy) => { + let onSubmenuOpen = useCallback((focusStrategy?: FocusStrategy) => { cancelOpenTimeout(); state.open(focusStrategy); - }); + }, [state, cancelOpenTimeout]); - let onSubmenuClose = useEffectEvent(() => { + let onSubmenuClose = useCallback(() => { cancelOpenTimeout(); state.close(); - }); + }, [state, cancelOpenTimeout]); useLayoutEffect(() => { return () => { diff --git a/packages/@react-aria/selection/src/useSelectableCollection.ts b/packages/@react-aria/selection/src/useSelectableCollection.ts index 4259dc51132..f44cb0a123d 100644 --- a/packages/@react-aria/selection/src/useSelectableCollection.ts +++ b/packages/@react-aria/selection/src/useSelectableCollection.ts @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, focusWithoutScrolling, getActiveElement, isCtrlKeyPressed, isTabbable, mergeProps, scrollIntoView, scrollIntoViewport, useEffectEvent, useEvent, useRouter, useUpdateLayoutEffect} from '@react-aria/utils'; +import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, focusWithoutScrolling, getActiveElement, isCtrlKeyPressed, isTabbable, mergeProps, scrollIntoView, scrollIntoViewport, useEvent, useRouter, useUpdateLayoutEffect} from '@react-aria/utils'; import {dispatchVirtualFocus, getFocusableTreeWalker, moveVirtualFocus} from '@react-aria/focus'; import {DOMAttributes, FocusableElement, FocusStrategy, Key, KeyboardDelegate, RefObject} from '@react-types/shared'; import {flushSync} from 'react-dom'; @@ -416,49 +416,42 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions } }); - let updateActiveDescendant = useEffectEvent(() => { - let keyToFocus = delegate.getFirstKey?.() ?? null; - - // If no focusable items exist in the list, make sure to clear any activedescendant that may still exist and move focus back to - // the original active element (e.g. the autocomplete input) - if (keyToFocus == null) { - let previousActiveElement = getActiveElement(); - moveVirtualFocus(ref.current); - dispatchVirtualFocus(previousActiveElement!, null); - - // If there wasn't a focusable key but the collection had items, then that means we aren't in an intermediate load state and all keys are disabled. - // Reset shouldVirtualFocusFirst so that we don't erronously autofocus an item when the collection is filtered again. - if (manager.collection.size > 0) { + // update active descendant + useUpdateLayoutEffect(() => { + if (shouldVirtualFocusFirst.current) { + let keyToFocus = delegate.getFirstKey?.() ?? null; + + // If no focusable items exist in the list, make sure to clear any activedescendant that may still exist and move focus back to + // the original active element (e.g. the autocomplete input) + if (keyToFocus == null) { + let previousActiveElement = getActiveElement(); + moveVirtualFocus(ref.current); + dispatchVirtualFocus(previousActiveElement!, null); + + // If there wasn't a focusable key but the collection had items, then that means we aren't in an intermediate load state and all keys are disabled. + // Reset shouldVirtualFocusFirst so that we don't erronously autofocus an item when the collection is filtered again. + if (manager.collection.size > 0) { + shouldVirtualFocusFirst.current = false; + } + } else { + manager.setFocusedKey(keyToFocus); + // Only set shouldVirtualFocusFirst to false if we've successfully set the first key as the focused key + // If there wasn't a key to focus, we might be in a temporary loading state so we'll want to still focus the first key + // after the collection updates after load shouldVirtualFocusFirst.current = false; } - } else { - manager.setFocusedKey(keyToFocus); - // Only set shouldVirtualFocusFirst to false if we've successfully set the first key as the focused key - // If there wasn't a key to focus, we might be in a temporary loading state so we'll want to still focus the first key - // after the collection updates after load - shouldVirtualFocusFirst.current = false; } - }); + }, [manager.collection]); + // reset focus first flag useUpdateLayoutEffect(() => { - if (shouldVirtualFocusFirst.current) { - updateActiveDescendant(); - } - - }, [manager.collection, updateActiveDescendant]); - - let resetFocusFirstFlag = useEffectEvent(() => { // If user causes the focused key to change in any other way, clear shouldVirtualFocusFirst so we don't // accidentally move focus from under them. Skip this if the collection was empty because we might be in a load // state and will still want to focus the first item after load if (manager.collection.size > 0) { shouldVirtualFocusFirst.current = false; } - }); - - useUpdateLayoutEffect(() => { - resetFocusFirstFlag(); - }, [manager.focusedKey, resetFocusFirstFlag]); + }, [manager.focusedKey]); useEvent(ref, CLEAR_FOCUS_EVENT, !shouldUseVirtualFocus ? undefined : (e: any) => { e.stopPropagation(); diff --git a/packages/@react-aria/spinbutton/src/useSpinButton.ts b/packages/@react-aria/spinbutton/src/useSpinButton.ts index dada685076b..de772fa5106 100644 --- a/packages/@react-aria/spinbutton/src/useSpinButton.ts +++ b/packages/@react-aria/spinbutton/src/useSpinButton.ts @@ -15,7 +15,7 @@ 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 {useCallback, useEffect, useRef, useState} from 'react'; import {useEffectEvent, useGlobalListeners} from '@react-aria/utils'; import {useLocalizedStringFormatter} from '@react-aria/i18n'; @@ -64,7 +64,6 @@ export function useSpinButton( isSpinning.current = false; }; - useEffect(() => { return () => clearAsync(); }, []); @@ -199,6 +198,24 @@ export function useSpinButton( // an increment or decrement. let isUp = useRef(false); + let [isIncrementPressed, setIsIncrementPressed] = useState<'touch' | 'mouse' | null>(null); + useEffect(() => { + if (isIncrementPressed === 'touch') { + onIncrementPressStart(60); + } else if (isIncrementPressed) { + onIncrementPressStart(400); + } + }, [isIncrementPressed]); + + let [isDecrementPressed, setIsDecrementPressed] = useState<'touch' | 'mouse' | null>(null); + useEffect(() => { + if (isDecrementPressed === 'touch') { + onDecrementPressStart(60); + } else if (isDecrementPressed) { + onDecrementPressStart(400); + } + }, [isDecrementPressed]); + return { spinButtonProps: { role: 'spinbutton', @@ -216,19 +233,19 @@ export function useSpinButton( incrementButtonProps: { onPressStart: (e) => { if (e.pointerType !== 'touch') { - onIncrementPressStart(400); + setIsIncrementPressed('mouse'); } else { if (_async.current) { clearAsync(); } - // For touch users, don't trigger an increment on press start, we'll wait for the press end to trigger it if - // the control isn't spinning. - _async.current = window.setTimeout(() => { - onIncrementPressStart(60); - }, 600); addGlobalListener(window, 'touchmove', onTouchMove, {capture: true}); isUp.current = false; + // For touch users, don't trigger a decrement on press start, we'll wait for the press end to trigger it if + // the control isn't spinning. + _async.current = window.setTimeout(() => { + setIsIncrementPressed('touch'); + }, 600); } addGlobalListener(window, 'contextmenu', cancelContextMenu); }, @@ -239,6 +256,7 @@ export function useSpinButton( prevTouchPosition.current = null; clearAsync(); removeAllGlobalListeners(); + setIsIncrementPressed(null); }, onPressEnd: (e) => { if (e.pointerType === 'touch') { @@ -247,6 +265,7 @@ export function useSpinButton( } } isUp.current = false; + setIsIncrementPressed(null); }, onFocus, onBlur @@ -254,19 +273,19 @@ export function useSpinButton( decrementButtonProps: { onPressStart: (e) => { if (e.pointerType !== 'touch') { - onDecrementPressStart(400); + setIsDecrementPressed('mouse'); } else { if (_async.current) { clearAsync(); } + + addGlobalListener(window, 'touchmove', onTouchMove, {capture: true}); + isUp.current = false; // For touch users, don't trigger a decrement on press start, we'll wait for the press end to trigger it if // the control isn't spinning. _async.current = window.setTimeout(() => { - onDecrementPressStart(60); + setIsDecrementPressed('touch'); }, 600); - - addGlobalListener(window, 'touchmove', onTouchMove, {capture: true}); - isUp.current = false; } }, onPressUp: (e) => { @@ -276,6 +295,7 @@ export function useSpinButton( prevTouchPosition.current = null; clearAsync(); removeAllGlobalListeners(); + setIsDecrementPressed(null); }, onPressEnd: (e) => { if (e.pointerType === 'touch') { @@ -284,6 +304,7 @@ export function useSpinButton( } } isUp.current = false; + setIsDecrementPressed(null); }, onFocus, onBlur diff --git a/packages/@react-aria/table/src/useTableColumnResize.ts b/packages/@react-aria/table/src/useTableColumnResize.ts index 0a8c956ab13..7d02274c946 100644 --- a/packages/@react-aria/table/src/useTableColumnResize.ts +++ b/packages/@react-aria/table/src/useTableColumnResize.ts @@ -70,25 +70,8 @@ export function useTableColumnResize(props: AriaTableColumnResizeProps, st let editModeEnabled = state.tableState.isKeyboardNavigationDisabled; let {direction} = useLocale(); - let {keyboardProps} = useKeyboard({ - onKeyDown: (e) => { - if (editModeEnabled) { - if (e.key === 'Escape' || e.key === 'Enter' || e.key === ' ' || e.key === 'Tab') { - e.preventDefault(); - endResize(item); - } - } else { - // Continue propagation on keydown events so they still bubbles to useSelectableCollection and are handled there - e.continuePropagation(); - if (e.key === 'Enter') { - startResize(item); - } - } - } - }); - - let startResize = useEffectEvent((item) => { + let startResize = useCallback((item) => { if (!isResizingRef.current) { lastSize.current = state.updateResizedColumns(item.key, state.getColumnWidth(item.key)); state.startResize(item.key); @@ -96,15 +79,15 @@ export function useTableColumnResize(props: AriaTableColumnResizeProps, st onResizeStart?.(lastSize.current); } isResizingRef.current = true; - }); + }, [state, onResizeStart]); - let resize = useEffectEvent((item, newWidth) => { + let resize = useCallback((item, newWidth) => { let sizes = state.updateResizedColumns(item.key, newWidth); onResize?.(sizes); lastSize.current = sizes; - }); + }, [state, onResize]); - let endResize = useEffectEvent((item) => { + let endResize = useCallback((item) => { if (isResizingRef.current) { if (lastSize.current == null) { lastSize.current = state.updateResizedColumns(item.key, state.getColumnWidth(item.key)); @@ -121,6 +104,24 @@ export function useTableColumnResize(props: AriaTableColumnResizeProps, st } } lastSize.current = null; + }, [state, triggerRef, onResizeEnd]); + + let {keyboardProps} = useKeyboard({ + onKeyDown: (e) => { + if (editModeEnabled) { + if (e.key === 'Escape' || e.key === 'Enter' || e.key === ' ' || e.key === 'Tab') { + e.preventDefault(); + endResize(item); + } + } else { + // Continue propagation on keydown events so they still bubbles to useSelectableCollection and are handled there + e.continuePropagation(); + + if (e.key === 'Enter') { + startResize(item); + } + } + } }); const columnResizeWidthRef = useRef(0); @@ -194,10 +195,11 @@ export function useTableColumnResize(props: AriaTableColumnResizeProps, st let resizingColumn = state.resizingColumn; let prevResizingColumn = useRef(null); + let startResizeEvent = useEffectEvent(startResize); useEffect(() => { if (prevResizingColumn.current !== resizingColumn && resizingColumn != null && resizingColumn === item.key) { wasFocusedOnResizeStart.current = document.activeElement === ref.current; - startResize(item); + startResizeEvent(item); // Delay focusing input until Android Chrome's delayed click after touchend happens: https://bugs.chromium.org/p/chromium/issues/detail?id=1150073 let timeout = setTimeout(() => focusInput(), 0); // VoiceOver on iOS has problems focusing the input from a menu. @@ -208,7 +210,7 @@ export function useTableColumnResize(props: AriaTableColumnResizeProps, st }; } prevResizingColumn.current = resizingColumn; - }, [resizingColumn, item, focusInput, ref, startResize]); + }, [resizingColumn, item, focusInput, ref]); let onChange = (e: ChangeEvent) => { let currentWidth = state.getColumnWidth(item.key); diff --git a/packages/@react-aria/textfield/src/useFormattedTextField.ts b/packages/@react-aria/textfield/src/useFormattedTextField.ts index 878deb9c841..e0d866fd016 100644 --- a/packages/@react-aria/textfield/src/useFormattedTextField.ts +++ b/packages/@react-aria/textfield/src/useFormattedTextField.ts @@ -104,7 +104,7 @@ export function useFormattedTextField(props: AriaTextFieldProps, state: Formatte return () => { input.removeEventListener('beforeinput', onBeforeInputFallback, false); }; - }, [inputRef, onBeforeInputFallback]); + }, [inputRef]); let onBeforeInput = !supportsNativeBeforeInputEvent() ? e => { diff --git a/packages/@react-aria/toast/src/useToastRegion.ts b/packages/@react-aria/toast/src/useToastRegion.ts index 04d6a8dceed..1c6c3dbaf84 100644 --- a/packages/@react-aria/toast/src/useToastRegion.ts +++ b/packages/@react-aria/toast/src/useToastRegion.ts @@ -11,12 +11,12 @@ */ import {AriaLabelingProps, DOMAttributes, FocusableElement, RefObject} from '@react-types/shared'; -import {focusWithoutScrolling, mergeProps, useEffectEvent, useLayoutEffect} from '@react-aria/utils'; +import {focusWithoutScrolling, mergeProps, useLayoutEffect} from '@react-aria/utils'; import {getInteractionModality, useFocusWithin, useHover} from '@react-aria/interactions'; // @ts-ignore import intlMessages from '../intl/*.json'; import {ToastState} from '@react-stately/toast'; -import {useEffect, useRef} from 'react'; +import {useCallback, useEffect, useRef} from 'react'; import {useLandmark} from '@react-aria/landmark'; import {useLocalizedStringFormatter} from '@react-aria/i18n'; @@ -46,13 +46,13 @@ export function useToastRegion(props: AriaToastRegionProps, state: ToastState let isHovered = useRef(false); let isFocused = useRef(false); - let updateTimers = useEffectEvent(() => { + let updateTimers = useCallback(() => { if (isHovered.current || isFocused.current) { state.pauseAll(); } else { state.resumeAll(); } - }); + }, [state]); let {hoverProps} = useHover({ onHoverStart: () => { @@ -133,7 +133,7 @@ export function useToastRegion(props: AriaToastRegionProps, state: ToastState } prevVisibleToasts.current = state.visibleToasts; - }, [state.visibleToasts, ref, updateTimers]); + }, [state.visibleToasts, ref]); let lastFocused = useRef(null); let {focusWithinProps} = useFocusWithin({ diff --git a/packages/@react-aria/utils/src/useEvent.ts b/packages/@react-aria/utils/src/useEvent.ts index a56710b906c..1dd35499847 100644 --- a/packages/@react-aria/utils/src/useEvent.ts +++ b/packages/@react-aria/utils/src/useEvent.ts @@ -33,5 +33,5 @@ export function useEvent( return () => { element.removeEventListener(event, handleEvent as EventListener, options); }; - }, [ref, event, options, isDisabled, handleEvent]); + }, [ref, event, options, isDisabled]); } diff --git a/packages/@react-aria/utils/src/useFormReset.ts b/packages/@react-aria/utils/src/useFormReset.ts index 749c5e38788..c37eb67cc50 100644 --- a/packages/@react-aria/utils/src/useFormReset.ts +++ b/packages/@react-aria/utils/src/useFormReset.ts @@ -32,5 +32,5 @@ export function useFormReset( return () => { form?.removeEventListener('reset', handleReset); }; - }, [ref, handleReset]); + }, [ref]); } diff --git a/packages/@react-aria/utils/src/useLoadMoreSentinel.ts b/packages/@react-aria/utils/src/useLoadMoreSentinel.ts index 2d02f2ca101..457124261b4 100644 --- a/packages/@react-aria/utils/src/useLoadMoreSentinel.ts +++ b/packages/@react-aria/utils/src/useLoadMoreSentinel.ts @@ -59,5 +59,5 @@ export function useLoadMoreSentinel(props: LoadMoreSentinelProps, ref: RefObject sentinelObserver.current.disconnect(); } }; - }, [collection, triggerLoadMore, ref, scrollOffset]); + }, [collection, ref, scrollOffset]); } diff --git a/packages/@react-aria/utils/src/useResizeObserver.ts b/packages/@react-aria/utils/src/useResizeObserver.ts index 32d9e8a4e4b..ab0a1d5ae3c 100644 --- a/packages/@react-aria/utils/src/useResizeObserver.ts +++ b/packages/@react-aria/utils/src/useResizeObserver.ts @@ -1,6 +1,7 @@ import {RefObject} from '@react-types/shared'; import {useEffect} from 'react'; +import {useEffectEvent} from './useEffectEvent'; function hasResizeObserver() { return typeof window.ResizeObserver !== 'undefined'; @@ -13,7 +14,10 @@ type useResizeObserverOptionsType = { } export function useResizeObserver(options: useResizeObserverOptionsType): void { + // Only call onResize from inside the effect, otherwise we'll void our assumption that + // useEffectEvents are safe to pass in. const {ref, box, onResize} = options; + let onResizeEvent = useEffectEvent(onResize); useEffect(() => { let element = ref?.current; @@ -22,9 +26,9 @@ export function useResizeObserver(options: useResizeObserverO } if (!hasResizeObserver()) { - window.addEventListener('resize', onResize, false); + window.addEventListener('resize', onResizeEvent, false); return () => { - window.removeEventListener('resize', onResize, false); + window.removeEventListener('resize', onResizeEvent, false); }; } else { @@ -33,7 +37,7 @@ export function useResizeObserver(options: useResizeObserverO return; } - onResize(); + onResizeEvent(); }); resizeObserverInstance.observe(element, {box}); @@ -44,5 +48,5 @@ export function useResizeObserver(options: useResizeObserverO }; } - }, [onResize, ref, box]); + }, [ref, box]); } diff --git a/packages/@react-aria/utils/src/useUpdateEffect.ts b/packages/@react-aria/utils/src/useUpdateEffect.ts index d486e61baa6..e1aff96e964 100644 --- a/packages/@react-aria/utils/src/useUpdateEffect.ts +++ b/packages/@react-aria/utils/src/useUpdateEffect.ts @@ -11,11 +11,13 @@ */ import {EffectCallback, useEffect, useRef} from 'react'; +import {useEffectEvent} from './useEffectEvent'; // Like useEffect, but only called for updates after the initial render. -export function useUpdateEffect(effect: EffectCallback, dependencies: any[]): void { +export function useUpdateEffect(cb: EffectCallback, dependencies: any[]): void { const isInitialMount = useRef(true); const lastDeps = useRef(null); + let cbEvent = useEffectEvent(cb); useEffect(() => { isInitialMount.current = true; @@ -29,9 +31,9 @@ export function useUpdateEffect(effect: EffectCallback, dependencies: any[]): vo if (isInitialMount.current) { isInitialMount.current = false; } else if (!prevDeps || dependencies.some((dep, i) => !Object.is(dep, prevDeps[i]))) { - effect(); + cbEvent(); } lastDeps.current = dependencies; - // eslint-disable-next-line react-hooks/exhaustive-deps + // eslint-disable-next-line react-hooks/exhaustive-deps }, dependencies); } diff --git a/packages/@react-aria/utils/src/useValueEffect.ts b/packages/@react-aria/utils/src/useValueEffect.ts index a8c90397aea..f22ee14fb05 100644 --- a/packages/@react-aria/utils/src/useValueEffect.ts +++ b/packages/@react-aria/utils/src/useValueEffect.ts @@ -10,8 +10,8 @@ * governing permissions and limitations under the License. */ -import {Dispatch, MutableRefObject, useRef, useState} from 'react'; -import {useEffectEvent, useLayoutEffect} from './'; +import {Dispatch, RefObject, useCallback, useRef, useState} from 'react'; +import {useLayoutEffect} from './'; type SetValueAction = (prev: S) => Generator; @@ -21,11 +21,14 @@ type SetValueAction = (prev: S) => Generator; // written linearly. export function useValueEffect(defaultValue: S | (() => S)): [S, Dispatch>] { let [value, setValue] = useState(defaultValue); - let effect: MutableRefObject | null> = useRef | null>(null); + // Keep an up to date copy of value in a ref so we can access the current value in the generator. + // This allows us to maintain a stable queue function. + let currValue = useRef(value); + let effect: RefObject | null> = useRef | null>(null); // Store the function in a ref so we can always access the current version // which has the proper `value` in scope. - let nextRef = useEffectEvent(() => { + let nextRef = useRef(() => { if (!effect.current) { return; } @@ -41,24 +44,25 @@ export function useValueEffect(defaultValue: S | (() => S)): [S, Dispatch { + currValue.current = value; // If there is an effect currently running, continue to the next yield. if (effect.current) { - nextRef(); + nextRef.current(); } }); - let queue = useEffectEvent(fn => { - effect.current = fn(value); - nextRef(); - }); + let queue = useCallback(fn => { + effect.current = fn(currValue.current); + nextRef.current(); + }, [nextRef]); return [value, queue]; } diff --git a/packages/@react-aria/virtualizer/src/ScrollView.tsx b/packages/@react-aria/virtualizer/src/ScrollView.tsx index 7c4988a5b3e..b57efc4f19e 100644 --- a/packages/@react-aria/virtualizer/src/ScrollView.tsx +++ b/packages/@react-aria/virtualizer/src/ScrollView.tsx @@ -157,7 +157,7 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject { + let updateSize = useCallback((flush: typeof flushSync) => { let dom = ref.current; if (!dom || isUpdatingSize.current) { return; @@ -197,11 +197,14 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject(null); let [update, setUpdate] = useState({}); + // We only contain a call to setState in here for testing environments. + // eslint-disable-next-line react-hooks/exhaustive-deps useLayoutEffect(() => { if (!isUpdatingSize.current && (lastContentSize.current == null || !contentSize.equals(lastContentSize.current))) { // React doesn't allow flushSync inside effects, so queue a microtask. @@ -218,7 +221,7 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject updateSize(flushSync)); + queueMicrotask(() => updateSizeEvent(flushSync)); } } @@ -227,7 +230,7 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject { - updateSize(fn => fn()); + updateSizeEvent(fn => fn()); }, [update]); let onResize = useCallback(() => { diff --git a/packages/@react-spectrum/s2/src/CardView.tsx b/packages/@react-spectrum/s2/src/CardView.tsx index e589a1cbd4a..2023a8c0d95 100644 --- a/packages/@react-spectrum/s2/src/CardView.tsx +++ b/packages/@react-spectrum/s2/src/CardView.tsx @@ -24,7 +24,7 @@ import { WaterfallLayout } from 'react-aria-components'; import {CardContext, InternalCardViewContext} from './Card'; -import {createContext, forwardRef, ReactElement, useMemo, useRef, useState} from 'react'; +import {createContext, forwardRef, ReactElement, useCallback, useMemo, useRef, useState} from 'react'; import {DOMRef, DOMRefValue, forwardRefType, GlobalDOMAttributes, Key, LoadingState} from '@react-types/shared'; import {focusRing, style} from '../style' with {type: 'macro'}; import {getAllowedOverrides, StylesPropWithHeight, UnsafeStyles} from './style-utils' with {type: 'macro'}; @@ -218,7 +218,7 @@ export const CardView = /*#__PURE__*/ (forwardRef as forwardRefType)(function Ca // This calculates the maximum t-shirt size where at least two columns fit in the available width. let [maxSizeIndex, setMaxSizeIndex] = useState(SIZES.length - 1); - let updateSize = useEffectEvent(() => { + let updateSize = useCallback(() => { let w = scrollRef.current?.clientWidth ?? 0; let i = SIZES.length - 1; while (i > 0) { @@ -229,7 +229,8 @@ export const CardView = /*#__PURE__*/ (forwardRef as forwardRefType)(function Ca i--; } setMaxSizeIndex(i); - }); + }, [scrollRef, density]); + let updateSizeEvent = useEffectEvent(updateSize); useResizeObserver({ ref: scrollRef, @@ -238,8 +239,8 @@ export const CardView = /*#__PURE__*/ (forwardRef as forwardRefType)(function Ca }); useLayoutEffect(() => { - updateSize(); - }, [updateSize]); + updateSizeEvent(); + }, []); // The actual rendered t-shirt size is the minimum between the size prop and the maximum possible size. let size = SIZES[Math.min(maxSizeIndex, SIZES.indexOf(sizeProp))]; diff --git a/packages/@react-spectrum/s2/src/CoachMark.tsx b/packages/@react-spectrum/s2/src/CoachMark.tsx index a4d6cc0a70d..41229c198d1 100644 --- a/packages/@react-spectrum/s2/src/CoachMark.tsx +++ b/packages/@react-spectrum/s2/src/CoachMark.tsx @@ -504,6 +504,7 @@ export const CoachMarkIndicator = /*#__PURE__*/ (forwardRef as forwardRefType)(f objRef.current.style.minWidth = childMinWidth; objRef.current.style.minHeight = childMinHeight; } + // eslint-disable-next-line react-hooks/exhaustive-deps }, [children]); return ( diff --git a/packages/@react-spectrum/s2/src/Skeleton.tsx b/packages/@react-spectrum/s2/src/Skeleton.tsx index 1fb2bdb185b..b8ebd13cba4 100644 --- a/packages/@react-spectrum/s2/src/Skeleton.tsx +++ b/packages/@react-spectrum/s2/src/Skeleton.tsx @@ -41,7 +41,7 @@ export function useLoadingAnimation(isAnimating: boolean): (element: HTMLElement animationRef.current.cancel(); animationRef.current = null; } - }, [isAnimating]); + }, [isAnimating, reduceMotion]); } export type SkeletonElement = ReactElement<{ diff --git a/packages/@react-spectrum/s2/src/Tabs.tsx b/packages/@react-spectrum/s2/src/Tabs.tsx index 329a6284daa..e3af2c8fb9b 100644 --- a/packages/@react-spectrum/s2/src/Tabs.tsx +++ b/packages/@react-spectrum/s2/src/Tabs.tsx @@ -216,7 +216,7 @@ export function TabList(props: TabListProps): ReactNode | n if (showTabs) { return ; } - + return (
{listRef &&
@@ -628,7 +628,8 @@ let CollapsingTabs = ({collection, containerRef, ...props}: {collection: Collect let children = useMemo(() => [...collection], [collection]); let listRef = useRef(null); - let updateOverflow = useEffectEvent(() => { + + let updateOverflow = () => { if (orientation === 'vertical' || !listRef.current || !containerRef?.current) { return; } @@ -642,29 +643,30 @@ let CollapsingTabs = ({collection, containerRef, ...props}: {collection: Collect } else { setShowItems?.(lastTabRect.left >= containerRect.left); } - }); + }; + + let updateOverflowEffect = useEffectEvent(updateOverflow); useResizeObserver({ref: containerRef, onResize: updateOverflow}); useLayoutEffect(() => { if (collection.size > 0) { - queueMicrotask(updateOverflow); + queueMicrotask(updateOverflowEffect); } - }, [collection.size, updateOverflow]); + }, [collection.size]); // start with null so that the first render won't have a flicker let prevOrientation = useRef(null); useLayoutEffect(() => { if (collection.size > 0 && prevOrientation.current !== orientation) { - updateOverflow(); + updateOverflowEffect(); } prevOrientation.current = orientation; - }, [collection.size, updateOverflow, orientation]); + }, [collection.size, orientation]); useEffect(() => { // Recalculate visible tags when fonts are loaded. - document.fonts?.ready.then(() => updateOverflow()); - // eslint-disable-next-line react-hooks/exhaustive-deps + document.fonts?.ready.then(() => updateOverflowEffect()); }, []); let menuId = useId(); diff --git a/packages/@react-spectrum/s2/src/TagGroup.tsx b/packages/@react-spectrum/s2/src/TagGroup.tsx index 2beaaa2bcf0..c7eb82ba4b2 100644 --- a/packages/@react-spectrum/s2/src/TagGroup.tsx +++ b/packages/@react-spectrum/s2/src/TagGroup.tsx @@ -146,7 +146,7 @@ function TagGroupInner({ [collection, tagState.visibleTagCount, isCollapsed] ); - let updateVisibleTagCount = useEffectEvent(() => { + let updateVisibleTagCount = () => { if (maxRows == null) { setTagState({visibleTagCount: collection.size, showCollapseButton: false}); } @@ -216,20 +216,21 @@ function TagGroupInner({ setTagState(result); }); } - }); + }; + + let updateVisibleTagCountEffect = useEffectEvent(updateVisibleTagCount); useResizeObserver({ref: maxRows != null ? containerRef : undefined, onResize: updateVisibleTagCount}); useLayoutEffect(() => { if (collection.size > 0 && (maxRows != null && maxRows > 0)) { - queueMicrotask(updateVisibleTagCount); + queueMicrotask(updateVisibleTagCountEffect); } - }, [collection.size, updateVisibleTagCount, maxRows]); + }, [collection.size, maxRows]); useEffect(() => { // Recalculate visible tags when fonts are loaded. - document.fonts?.ready.then(() => updateVisibleTagCount()); - // eslint-disable-next-line react-hooks/exhaustive-deps + document.fonts?.ready.then(() => updateVisibleTagCountEffect()); }, []); let handlePressCollapse = () => { diff --git a/packages/@react-spectrum/table/stories/Table.stories.tsx b/packages/@react-spectrum/table/stories/Table.stories.tsx index 27558e39fb2..c01caba0f87 100644 --- a/packages/@react-spectrum/table/stories/Table.stories.tsx +++ b/packages/@react-spectrum/table/stories/Table.stories.tsx @@ -1253,6 +1253,7 @@ function useMountEffect(fn: () => void, deps: Array): void { } else { mounted.current = true; } + // eslint-disable-next-line react-hooks/exhaustive-deps }, deps); } @@ -1631,12 +1632,12 @@ function TableWithBreadcrumbs(props) { {key: 'd', name: 'File D', value: '10 MB', parent: 'a'} ]; - const [loadingState, setLoadingState] = useState('idle' as 'idle'); + const [loadingState, setLoadingState] = useState('idle' as const); const [selection, setSelection] = useState<'all' | Iterable>(new Set([])); const [items, setItems] = useState(() => fs.filter(item => !item.parent)); const changeFolder = (folder) => { setItems([]); - setLoadingState('loading' as 'loading'); + setLoadingState('loading' as const); // mimic loading behavior setTimeout(() => { @@ -2175,7 +2176,7 @@ function LoadingTable(): JSX.Element { setItems([]); setLoadingState('loading'); setTimeout(() => { - setItems(items.length > 1 ? [...items.slice(0, 1)] : []); + setItems(items.length > 1 ? items.slice(0, 1) : []); setLoadingState('idle'); }, 1000); }; diff --git a/packages/dev/s2-docs/src/VisualExampleClient.tsx b/packages/dev/s2-docs/src/VisualExampleClient.tsx index 30c718569a7..58eee84bb9c 100644 --- a/packages/dev/s2-docs/src/VisualExampleClient.tsx +++ b/packages/dev/s2-docs/src/VisualExampleClient.tsx @@ -93,6 +93,7 @@ export function VisualExampleClient({component, name, importSource, controls, ch } setProps(newProps); } + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); return ( diff --git a/packages/react-aria-components/src/HiddenDateInput.tsx b/packages/react-aria-components/src/HiddenDateInput.tsx index f0ce4b971b7..60fc3af948e 100644 --- a/packages/react-aria-components/src/HiddenDateInput.tsx +++ b/packages/react-aria-components/src/HiddenDateInput.tsx @@ -63,7 +63,7 @@ export function useHiddenDateInput(props: HiddenDateInputProps, state: DateField if (state.granularity === 'second') { inputStep = 1; } else if (state.granularity === 'hour') { - inputStep = 3600; + inputStep = 3600; } let dateValue = state.value == null ? '' : state.value.toString(); @@ -107,15 +107,17 @@ export function useHiddenDateInput(props: HiddenDateInputProps, state: DateField } // We check to to see if setSegment exists in the state since it only exists in DateFieldState and not DatePickerState. // The setValue method has different behavior depending on if it's coming from DateFieldState or DatePickerState. - // In DateFieldState, setValue firsts checks to make sure that each segment is filled before committing the newValue - // which is why in the code below we first set each segment to validate it before committing the new value. - // However, in DatePickerState, since we have to be able to commit values from the Calendar popover, we are also able to + // In DateFieldState, setValue firsts checks to make sure that each segment is filled before committing the newValue + // which is why in the code below we first set each segment to validate it before committing the new value. + // However, in DatePickerState, since we have to be able to commit values from the Calendar popover, we are also able to // set a new value when the field itself is empty. if ('setSegment' in state) { for (let type in targetValue) { + // eslint-disable-next-line max-depth if (dateSegments.includes(type)) { state.setSegment(type as DateSegmentType, targetValue[type]); } + // eslint-disable-next-line max-depth if (timeSegments.includes(type)) { state.setSegment(type as DateSegmentType, targetValue[type]); } diff --git a/packages/react-aria-components/test/Button.test.js b/packages/react-aria-components/test/Button.test.js index 08dda75820f..ef9a3dc34c1 100644 --- a/packages/react-aria-components/test/Button.test.js +++ b/packages/react-aria-components/test/Button.test.js @@ -321,6 +321,7 @@ describe('Button', () => { function TestComponent(props) { let [pending, setPending] = useState(false); return ( + // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions
{ // forms are submitted implicitly on keydown, so we need to wait to set pending until after to set pending From 116fe2776dcf37cfd1f99751addc106890c6304e Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Tue, 14 Oct 2025 16:49:37 +1100 Subject: [PATCH 06/26] enable all rules we are already passing --- eslint.config.mjs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index 417342a1567..c1ac2344dd9 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -230,21 +230,21 @@ export default [{ "react-hooks/exhaustive-deps": WARN, // React Compiler rules - // 'react-hooks/config': ERROR, - // 'react-hooks/error-boundaries': ERROR, - // 'react-hooks/component-hook-factories': ERROR, - // 'react-hooks/gating': ERROR, + 'react-hooks/config': ERROR, + 'react-hooks/error-boundaries': ERROR, + 'react-hooks/component-hook-factories': ERROR, + 'react-hooks/gating': 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/set-state-in-render': ERROR, + 'react-hooks/set-state-in-render': ERROR, // 'react-hooks/static-components': ERROR, - // 'react-hooks/unsupported-syntax': WARN, - // 'react-hooks/use-memo': ERROR, - // 'react-hooks/incompatible-library': WARN, + 'react-hooks/unsupported-syntax': WARN, + 'react-hooks/use-memo': ERROR, + 'react-hooks/incompatible-library': WARN, "rsp-rules/no-react-key": [ERROR], "rsp-rules/sort-imports": [ERROR], From a7d4e7e1fa243a1a536e105d27513edcbd1308d6 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Wed, 15 Oct 2025 11:01:30 +1100 Subject: [PATCH 07/26] fix event order and cleanup --- .../@react-aria/interactions/src/useMove.ts | 18 +++++++++++-- .../@react-aria/interactions/src/usePress.ts | 27 ++++++++++++++----- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/packages/@react-aria/interactions/src/useMove.ts b/packages/@react-aria/interactions/src/useMove.ts index 24584260cfb..387fe474c9a 100644 --- a/packages/@react-aria/interactions/src/useMove.ts +++ b/packages/@react-aria/interactions/src/useMove.ts @@ -13,7 +13,7 @@ import {disableTextSelection, restoreTextSelection} from './textSelection'; import {DOMAttributes, MoveEvents, PointerType} from '@react-types/shared'; import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; -import {useEffectEvent, useGlobalListeners} from '@react-aria/utils'; +import {useEffectEvent, useGlobalListeners, useLayoutEffect} from '@react-aria/utils'; export interface MoveResult { /** Props to spread on the target element. */ @@ -88,7 +88,7 @@ export function useMove(props: MoveEvents): MoveResult { let endEvent = useEffectEvent(end); let [pointerDown, setPointerDown] = useState<'pointer' | 'mouse' | 'touch' | null>(null); - useEffect(() => { + useLayoutEffect(() => { if (pointerDown === 'pointer') { let onPointerMove = (e: PointerEvent) => { if (e.pointerId === state.current.id) { @@ -116,6 +116,11 @@ export function useMove(props: MoveEvents): MoveResult { addGlobalListener(window, 'pointermove', onPointerMove, false); addGlobalListener(window, 'pointerup', onPointerUp, false); addGlobalListener(window, 'pointercancel', onPointerUp, false); + return () => { + removeGlobalListener(window, 'pointermove', onPointerMove, false); + removeGlobalListener(window, 'pointerup', onPointerUp, false); + removeGlobalListener(window, 'pointercancel', onPointerUp, false); + }; } else if (pointerDown === 'mouse' && process.env.NODE_ENV === 'test') { let onMouseMove = (e: MouseEvent) => { if (e.button === 0) { @@ -133,6 +138,10 @@ export function useMove(props: MoveEvents): MoveResult { }; addGlobalListener(window, 'mousemove', onMouseMove, false); addGlobalListener(window, 'mouseup', onMouseUp, false); + return () => { + removeGlobalListener(window, 'mousemove', onMouseMove, false); + removeGlobalListener(window, 'mouseup', onMouseUp, false); + }; } else if (pointerDown === 'touch' && process.env.NODE_ENV === 'test') { let onTouchMove = (e: TouchEvent) => { let touch = [...e.changedTouches].findIndex(({identifier}) => identifier === state.current.id); @@ -156,6 +165,11 @@ export function useMove(props: MoveEvents): MoveResult { addGlobalListener(window, 'touchmove', onTouchMove, false); addGlobalListener(window, 'touchend', onTouchEnd, false); addGlobalListener(window, 'touchcancel', onTouchEnd, false); + return () => { + removeGlobalListener(window, 'touchmove', onTouchMove, false); + removeGlobalListener(window, 'touchend', onTouchEnd, false); + removeGlobalListener(window, 'touchcancel', onTouchEnd, false); + }; } }, [pointerDown, addGlobalListener, removeGlobalListener]); diff --git a/packages/@react-aria/interactions/src/usePress.ts b/packages/@react-aria/interactions/src/usePress.ts index fce29504349..6dc4fd7f757 100644 --- a/packages/@react-aria/interactions/src/usePress.ts +++ b/packages/@react-aria/interactions/src/usePress.ts @@ -29,6 +29,7 @@ import { openLink, useEffectEvent, useGlobalListeners, + useLayoutEffect, useSyncRef } from '@react-aria/utils'; import {createSyntheticEvent, preventFocus, setEventTarget} from './utils'; @@ -195,7 +196,7 @@ export function usePress(props: PressHookProps): PressResult { disposables: [] }); - let {addGlobalListener, removeAllGlobalListeners} = useGlobalListeners(); + let {addGlobalListener, removeAllGlobalListeners, removeGlobalListener} = useGlobalListeners(); let triggerPressStart = useCallback((originalEvent: EventBase, pointerType: PointerType) => { let state = ref.current; @@ -328,7 +329,7 @@ export function usePress(props: PressHookProps): PressResult { let triggerSyntheticClickEvent = useEffectEvent(triggerSyntheticClick); let [isElemKeyPressed, setIsElemKeyPressed] = useState(false); - useEffect(() => { + useLayoutEffect(() => { let state = ref.current; if (isElemKeyPressed) { let onKeyUp = (e: KeyboardEvent) => { @@ -378,12 +379,16 @@ export function usePress(props: PressHookProps): PressResult { triggerPressUpEvent(createEvent(state.target, e), 'keyboard'); } }; - addGlobalListener(getOwnerDocument(state.target), 'keyup', chain(pressUp, onKeyUp), true); + let listener = chain(pressUp, onKeyUp); + addGlobalListener(getOwnerDocument(state.target), 'keyup', listener, true); + return () => { + removeGlobalListener(getOwnerDocument(state.target), 'keyup', listener, true); + }; } - }, [isElemKeyPressed, addGlobalListener, removeAllGlobalListeners]); + }, [isElemKeyPressed, addGlobalListener, removeAllGlobalListeners, removeGlobalListener]); let [isPointerPressed, setIsPointerPressed] = useState<'pointer' | 'mouse' | 'touch' | null>(null); - useEffect(() => { + useLayoutEffect(() => { let state = ref.current; if (isPointerPressed === 'pointer') { let onPointerUp = (e: PointerEvent) => { @@ -429,6 +434,10 @@ export function usePress(props: PressHookProps): PressResult { addGlobalListener(getOwnerDocument(state.target), 'pointerup', onPointerUp, false); addGlobalListener(getOwnerDocument(state.target), 'pointercancel', onPointerCancel, false); + return () => { + removeGlobalListener(getOwnerDocument(state.target), 'pointerup', onPointerUp, false); + removeGlobalListener(getOwnerDocument(state.target), 'pointercancel', onPointerCancel, false); + }; } else if (isPointerPressed === 'mouse' && process.env.NODE_ENV === 'test') { let onMouseUp = (e: MouseEvent) => { // Only handle left clicks @@ -452,6 +461,9 @@ export function usePress(props: PressHookProps): PressResult { }; addGlobalListener(getOwnerDocument(state.target), 'mouseup', onMouseUp, false); + return () => { + removeGlobalListener(getOwnerDocument(state.target), 'mouseup', onMouseUp, false); + }; } else if (isPointerPressed === 'touch' && process.env.NODE_ENV === 'test') { let onScroll = (e: Event) => { if (state.isPressed && nodeContains(getEventTarget(e), state.target)) { @@ -466,8 +478,11 @@ export function usePress(props: PressHookProps): PressResult { }; addGlobalListener(getOwnerWindow(state.target), 'scroll', onScroll, true); + return () => { + removeGlobalListener(getOwnerWindow(state.target), 'scroll', onScroll, true); + }; } - }, [isPointerPressed, addGlobalListener]); + }, [isPointerPressed, addGlobalListener, removeGlobalListener]); let pressProps = useMemo(() => { let state = ref.current; From 30bc1fa2c81afbd849a15c8a8c80e66dad9bb984 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Wed, 15 Oct 2025 11:19:13 +1100 Subject: [PATCH 08/26] fix lint --- packages/@react-aria/interactions/src/useMove.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-aria/interactions/src/useMove.ts b/packages/@react-aria/interactions/src/useMove.ts index 387fe474c9a..c8158b4f9c3 100644 --- a/packages/@react-aria/interactions/src/useMove.ts +++ b/packages/@react-aria/interactions/src/useMove.ts @@ -12,7 +12,7 @@ import {disableTextSelection, restoreTextSelection} from './textSelection'; import {DOMAttributes, MoveEvents, PointerType} from '@react-types/shared'; -import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; +import React, {useCallback, useMemo, useRef, useState} from 'react'; import {useEffectEvent, useGlobalListeners, useLayoutEffect} from '@react-aria/utils'; export interface MoveResult { From ba77f40298c31fb052c20b7a584123aca62c7541 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Wed, 15 Oct 2025 13:39:43 +1100 Subject: [PATCH 09/26] turn on and fix static-components --- eslint.config.mjs | 2 +- packages/react-aria-components/src/Table.tsx | 52 +++++++++++++++----- 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index c1ac2344dd9..c55353b652c 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -241,7 +241,7 @@ export default [{ // 'react-hooks/refs': ERROR, // '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, diff --git a/packages/react-aria-components/src/Table.tsx b/packages/react-aria-components/src/Table.tsx index 03bbb3395a0..6796ba57e26 100644 --- a/packages/react-aria-components/src/Table.tsx +++ b/packages/react-aria-components/src/Table.tsx @@ -584,6 +584,14 @@ class TableHeaderNode extends CollectionNode { static readonly type = 'tableheader'; } +function THeadElementType(props) { + let {isVirtualized} = useContext(CollectionRendererContext); + if (isVirtualized) { + return
; + } + return ; +} + /** * A header within a ``, containing the table columns. */ @@ -603,7 +611,6 @@ export const TableHeader = /*#__PURE__*/ createBranchComponent( }, []) }); - let THead = useElementType('thead'); let {rowGroupProps} = useTableRowGroup(); let {hoverProps, isHovered} = useHover({ onHoverStart: props.onHoverStart, @@ -621,13 +628,13 @@ export const TableHeader = /*#__PURE__*/ createBranchComponent( }); return ( - {headerRows} - + ); }, props => ( @@ -637,16 +644,23 @@ export const TableHeader = /*#__PURE__*/ createBranchComponent( ) ); +let TableHeaderRowElementType = forwardRef(function TableHeaderRowElementType(props: any, ref: ForwardedRef) { + let {isVirtualized} = useContext(CollectionRendererContext); + if (isVirtualized) { + return
; + } + return
; +}); + function TableHeaderRow({item}: {item: GridNode}) { let ref = useRef(null); let state = useContext(TableStateContext)!; let {isVirtualized, CollectionBranch} = useContext(CollectionRendererContext); let {rowProps} = useTableHeaderRow({node: item, isVirtualized}, state, ref); let {checkboxProps} = useTableSelectAllCheckbox(state); - let TR = useElementType('tr'); return ( - + }) { ]}> - + ); } @@ -1372,6 +1386,21 @@ interface TableDropIndicatorProps extends DropIndicatorProps, GlobalDOMAttribute buttonRef: RefObject } +let TableDropIndicatorRowElementType = forwardRef(function TableDropIndicatorRowElementType(props: any, ref: ForwardedRef) { + let {isVirtualized} = useContext(CollectionRendererContext); + if (isVirtualized) { + return
; + } + return
; +}); +let TableDropIndicatorTDElementType = forwardRef(function TableDropIndicatorTDElementType(props: any, ref: ForwardedRef) { + let {isVirtualized} = useContext(CollectionRendererContext); + if (isVirtualized) { + return
; + } + return
- + + ); } From 8f1cd8ef031d2699211a3092651f77f319b23cc2 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Wed, 15 Oct 2025 15:16:59 +1100 Subject: [PATCH 10/26] turn on and fix set-state-in-effect --- eslint.config.mjs | 2 +- .../menu/src/useSafelyMouseToSubmenu.ts | 4 ++-- packages/@react-aria/toast/src/useToast.ts | 4 ++-- .../autocomplete/src/SearchAutocomplete.tsx | 6 +++++- packages/@react-spectrum/color/src/ColorWheel.tsx | 6 +++--- packages/@react-spectrum/combobox/src/ComboBox.tsx | 8 ++++++-- packages/@react-spectrum/menu/src/Menu.tsx | 14 ++++++-------- packages/@react-spectrum/tabs/src/Tabs.tsx | 2 +- .../test/Autocomplete.test.tsx | 13 ++++++------- 9 files changed, 32 insertions(+), 27 deletions(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index c55353b652c..c9f5d827cd6 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -239,7 +239,7 @@ export default [{ // 'react-hooks/preserve-manual-memoization': ERROR, // 'react-hooks/purity': ERROR, // 'react-hooks/refs': ERROR, - // 'react-hooks/set-state-in-effect': ERROR, + 'react-hooks/set-state-in-effect': ERROR, 'react-hooks/set-state-in-render': ERROR, 'react-hooks/static-components': ERROR, 'react-hooks/unsupported-syntax': WARN, diff --git a/packages/@react-aria/menu/src/useSafelyMouseToSubmenu.ts b/packages/@react-aria/menu/src/useSafelyMouseToSubmenu.ts index f36eb66261e..f4fe57fb2bd 100644 --- a/packages/@react-aria/menu/src/useSafelyMouseToSubmenu.ts +++ b/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; diff --git a/packages/@react-aria/toast/src/useToast.ts b/packages/@react-aria/toast/src/useToast.ts index 710ca3e2297..33c408606db 100644 --- a/packages/@react-aria/toast/src/useToast.ts +++ b/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(props: AriaToastProps, state: ToastState, 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); }, []); diff --git a/packages/@react-spectrum/autocomplete/src/SearchAutocomplete.tsx b/packages/@react-spectrum/autocomplete/src/SearchAutocomplete.tsx index 19b4dad828c..4994ed9bbd4 100644 --- a/packages/@react-spectrum/autocomplete/src/SearchAutocomplete.tsx +++ b/packages/@react-spectrum/autocomplete/src/SearchAutocomplete.tsx @@ -288,7 +288,6 @@ function ForwardSearchAutocompleteInput(props: SearchAutocompleteInputProps(props: SearchAutocompleteInputProps { + useLayoutEffect(() => { // the size observer's fallback to the window resize event doesn't fire on mount if (wheelRadius === 0) { resizeHandler(); diff --git a/packages/@react-spectrum/combobox/src/ComboBox.tsx b/packages/@react-spectrum/combobox/src/ComboBox.tsx index b39df0ec9b5..70e78af304a 100644 --- a/packages/@react-spectrum/combobox/src/ComboBox.tsx +++ b/packages/@react-spectrum/combobox/src/ComboBox.tsx @@ -278,8 +278,6 @@ const ComboBoxInput = React.forwardRef(function ComboBoxInput(props: ComboBoxInp }, 500); } } else if (!isLoading) { - // If loading is no longer happening, clear any timers and hide the loading circle - setShowLoading(false); if (timeout.current) { clearTimeout(timeout.current); } @@ -289,6 +287,12 @@ const ComboBoxInput = React.forwardRef(function ComboBoxInput(props: ComboBoxInp lastInputValue.current = inputValue; }, [isLoading, showLoading, inputValue]); + let [prevIsLoading, setPrevIsLoading] = useState(isLoading); + if (prevIsLoading !== isLoading && !isLoading) { + setShowLoading(false); + setPrevIsLoading(isLoading); + } + useEffect(() => { return () => { if (timeout.current) { diff --git a/packages/@react-spectrum/menu/src/Menu.tsx b/packages/@react-spectrum/menu/src/Menu.tsx index b14b256a51d..97e5de589e1 100644 --- a/packages/@react-spectrum/menu/src/Menu.tsx +++ b/packages/@react-spectrum/menu/src/Menu.tsx @@ -54,14 +54,12 @@ export const Menu = React.forwardRef(function Menu(props: Spec let {styleProps} = useStyleProps(completeProps); useSyncRef(contextProps, domRef); let [leftOffset, setLeftOffset] = useState({left: 0}); - let prevPopoverContainer = useRef(null); - useEffect(() => { - if (popoverContainer && prevPopoverContainer.current !== popoverContainer && leftOffset.left === 0) { - prevPopoverContainer.current = popoverContainer; - let {left} = popoverContainer.getBoundingClientRect(); - setLeftOffset({left: -1 * left}); - } - }, [leftOffset, popoverContainer]); + let [prevPopoverContainer, setPrevPopoverContainer] = useState(null); + if (popoverContainer && prevPopoverContainer !== popoverContainer && leftOffset.left === 0) { + let {left} = popoverContainer.getBoundingClientRect(); + setLeftOffset({left: -1 * left}); + setPrevPopoverContainer(popoverContainer); + } let menuLevel = contextProps.submenuLevel ?? -1; let nextMenuLevelKey = rootMenuTriggerState?.expandedKeysStack[menuLevel + 1]; diff --git a/packages/@react-spectrum/tabs/src/Tabs.tsx b/packages/@react-spectrum/tabs/src/Tabs.tsx index b7731a8b22d..4090507353d 100644 --- a/packages/@react-spectrum/tabs/src/Tabs.tsx +++ b/packages/@react-spectrum/tabs/src/Tabs.tsx @@ -112,7 +112,7 @@ export const Tabs = React.forwardRef(function Tabs(props: Spec } }, [tablistRef, wrapperRef, direction, orientation, setCollapsed, prevTabPositions, setTabPositions]); - useEffect(() => { + useLayoutEffect(() => { checkShouldCollapse(); }, [children, checkShouldCollapse]); diff --git a/packages/react-aria-components/test/Autocomplete.test.tsx b/packages/react-aria-components/test/Autocomplete.test.tsx index 20eaeaaf0a3..8bcd266cb6c 100644 --- a/packages/react-aria-components/test/Autocomplete.test.tsx +++ b/packages/react-aria-components/test/Autocomplete.test.tsx @@ -13,7 +13,7 @@ import {act, pointerMap, render, within} from '@react-spectrum/test-utils-internal'; import {AriaAutocompleteTests} from './AriaAutocomplete.test-util'; import {Autocomplete, Breadcrumb, Breadcrumbs, Button, Cell, Collection, Column, Dialog, DialogTrigger, GridList, GridListItem, Header, Input, Label, ListBox, ListBoxItem, ListBoxLoadMoreItem, ListBoxSection, Menu, MenuItem, MenuSection, Popover, Row, SearchField, Select, SelectValue, Separator, SubmenuTrigger, Tab, Table, TableBody, TableHeader, TabList, TabPanel, Tabs, Tag, TagGroup, TagList, Text, TextField, Tree, TreeItem, TreeItemContent} from '..'; -import React, {ReactNode, useEffect, useState} from 'react'; +import React, {ReactNode, useState} from 'react'; import {useAsyncList} from 'react-stately'; import {useFilter} from '@react-aria/i18n'; import userEvent from '@testing-library/user-event'; @@ -971,11 +971,13 @@ describe('Autocomplete', () => { const [options, setOptions] = useState(defaultOptions); const [inputValue, onInputChange] = useState(''); - useEffect(() => { + let [prevInputValue, setPrevInputValue] = useState(inputValue); + if (prevInputValue !== inputValue) { setOptions( defaultOptions.filter(({value}) => value.includes(inputValue)) ); - }, [inputValue]); + setPrevInputValue(inputValue); + } return ( @@ -1014,10 +1016,7 @@ describe('Autocomplete', () => { act(() => jest.runAllTimers()); options = within(listbox).queryAllByRole('option'); expect(options).toHaveLength(0); - // TODO: this is strange, still need to investigate. Ideally this would be removed - // but the collection in this configuration doesn't seem to update in time, so - // useSelectableCollection doesn't properly resend virtual focus to the input - expect(input).toHaveAttribute('aria-activedescendant'); + expect(input).not.toHaveAttribute('aria-activedescendant'); await user.keyboard('{Backspace}'); act(() => jest.runAllTimers()); From 95687e983afc373e0b8fd925edcd9c8f52d8664d Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Wed, 15 Oct 2025 15:41:53 +1100 Subject: [PATCH 11/26] turn on and fix purity --- eslint.config.mjs | 4 +-- .../@react-aria/dnd/stories/Reorderable.tsx | 3 +- .../stories/ColorSwatchPicker.stories.tsx | 6 ++-- .../list/stories/ListViewDnDExamples.tsx | 10 ++++-- .../table/stories/TableDnDExamples.tsx | 10 ++++-- .../stories/ListBox.stories.tsx | 36 +++++++++---------- 6 files changed, 40 insertions(+), 29 deletions(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index c9f5d827cd6..fa42a517f2f 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -237,8 +237,8 @@ export default [{ // 'react-hooks/globals': ERROR, // 'react-hooks/immutability': ERROR, // 'react-hooks/preserve-manual-memoization': ERROR, - // 'react-hooks/purity': ERROR, - // 'react-hooks/refs': ERROR, + '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, diff --git a/packages/@react-aria/dnd/stories/Reorderable.tsx b/packages/@react-aria/dnd/stories/Reorderable.tsx index f6749e0a377..04c37fb1c7d 100644 --- a/packages/@react-aria/dnd/stories/Reorderable.tsx +++ b/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(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, diff --git a/packages/@react-spectrum/color/stories/ColorSwatchPicker.stories.tsx b/packages/@react-spectrum/color/stories/ColorSwatchPicker.stories.tsx index d1d40c30719..d2c4d07b140 100644 --- a/packages/@react-spectrum/color/stories/ColorSwatchPicker.stories.tsx +++ b/packages/@react-spectrum/color/stories/ColorSwatchPicker.stories.tsx @@ -48,10 +48,12 @@ export const Default: ColorSwatchPickerStory = (args) => ( ); +let randomColors = Array.from(Array(24)).map(() => { + return `#${Math.floor(Math.random() * 0xffffff).toString(16).padStart(6, '0')}`; +}); export const ManySwatches: ColorSwatchPickerStory = (args) => ( - {Array.from(Array(24)).map(() => { - let color = `#${Math.floor(Math.random() * 0xffffff).toString(16).padStart(6, '0')}`; + {randomColors.map((color) => { return ; })} diff --git a/packages/@react-spectrum/list/stories/ListViewDnDExamples.tsx b/packages/@react-spectrum/list/stories/ListViewDnDExamples.tsx index 3df3fe23339..18211199321 100644 --- a/packages/@react-spectrum/list/stories/ListViewDnDExamples.tsx +++ b/packages/@react-spectrum/list/stories/ListViewDnDExamples.tsx @@ -83,6 +83,7 @@ let itemList2 = [ {id: '12', type: 'item', textValue: 'Item Twelve'} ]; +let randomDragTypeReorderExample = `keys-${Math.random().toString(36).slice(2)}`; export function ReorderExample(props: SpectrumListViewProps & DragAndDropOptions & {getAllowedDropOperationsAction?: () => void}): JSX.Element { let {items, onDrop, onDragStart, onDragEnd, disabledKeys = ['2'], ...otherprops} = props; let list = useListData({ @@ -90,7 +91,7 @@ export function ReorderExample(props: SpectrumListViewProps & DragAndDropOp }); // 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 onMove = (keys: Key[], target: ItemDropTarget) => { if (target.dropPosition === 'before') { @@ -166,6 +167,7 @@ export function ReorderExample(props: SpectrumListViewProps & DragAndDropOp ); } +let randomDragTypeDragIntoItemExample = `keys-${Math.random().toString(36).slice(2)}`; export function DragIntoItemExample(props: {listViewProps: SpectrumListViewProps, dragHookOptions?: DragAndDropOptions, dropHookOptions?: DragAndDropOptions, getAllowedDropOperationsAction?: () => void}): JSX.Element { let { listViewProps = {}, @@ -193,7 +195,7 @@ export function DragIntoItemExample(props: {listViewProps: SpectrumListViewProps let disabledKeys: Key[] = ['2', '7']; // 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(() => randomDragTypeDragIntoItemExample, []); let onMove = (keys: Key[], target: ItemDropTarget) => { let folderItem = list.getItem(target.key)!; @@ -274,6 +276,8 @@ export function DragIntoItemExample(props: {listViewProps: SpectrumListViewProps ); } +let randomDragTypeDragBetweenListsExample = `keys-${Math.random().toString(36).slice(2)}`; + export function DragBetweenListsExample(props: SpectrumListViewProps & DragAndDropOptions & {getAllowedDropOperationsAction?: () => void, items1?: any[], items2?: any[]}): JSX.Element { let {onDragStart, onDragEnd, onDrop} = props; let onDropAction = chain(action('onDrop'), onDrop); @@ -311,7 +315,7 @@ export function DragBetweenListsExample(props: SpectrumListViewProps & Drag }; // Use a random drag type so the items can only be reordered within the two lists and not dragged elsewhere. - let dragType = React.useMemo(() => `keys-${Math.random().toString(36).slice(2)}`, []); + let dragType = React.useMemo(() => randomDragTypeDragBetweenListsExample, []); let {dragAndDropHooks} = useDragAndDrop({ getItems(keys) { diff --git a/packages/@react-spectrum/table/stories/TableDnDExamples.tsx b/packages/@react-spectrum/table/stories/TableDnDExamples.tsx index cde0101e701..b68daff48d9 100644 --- a/packages/@react-spectrum/table/stories/TableDnDExamples.tsx +++ b/packages/@react-spectrum/table/stories/TableDnDExamples.tsx @@ -112,6 +112,7 @@ export function DragWithoutRowHeaderExample(props: {tableViewProps?: Omit void, onDragStart?: (e: DragStartEvent) => void, onDragEnd?: (e: DragEndEvent) => void, tableViewProps?: Omit, 'children'>, items?: any[]}): JSX.Element { let {onDrop, onDragStart, onDragEnd, tableViewProps} = props; let list = useListData({ @@ -120,7 +121,7 @@ export function ReorderExample(props: {onDrop?: (e: DropEvent) => void, onDragSt }); // Use a random drag type so the items can only be reordered within this table and not dragged elsewhere. - let dragType = React.useMemo(() => `keys-${Math.random().toString(36).slice(2)}`, []); + let dragType = React.useMemo(() => randomDragTypeReorderExample, []); let onMove = (keys: Key[], target: ItemDropTarget) => { if (target.dropPosition === 'before') { @@ -192,6 +193,7 @@ export function ReorderExample(props: {onDrop?: (e: DropEvent) => void, onDragSt ); } +let randomDragTypeDragOntoRowExample = `keys-${Math.random().toString(36).slice(2)}`; export function DragOntoRowExample(props: {tableViewProps?: Omit, 'children'>, dragHookOptions?: any, dropHookOptions?: {onDrop?: (e: DropEvent) => void}}): JSX.Element { let { tableViewProps = {}, @@ -226,7 +228,7 @@ export function DragOntoRowExample(props: {tableViewProps?: Omit `keys-${Math.random().toString(36).slice(2)}`, []); + let dragType = React.useMemo(() => randomDragTypeDragOntoRowExample, []); let onMove = (keys: Key[], target: ItemDropTarget) => { let folderItem = list.getItem(target.key)!; @@ -328,6 +330,8 @@ let itemColumns = [ {name: 'Name', key: 'name'} ]; +let randomDragTypeDragBetweenTablesExample = `keys-${Math.random().toString(36).slice(2)}`; + export function DragBetweenTablesExample(props: {onDragStart?: (e: DragStartEvent) => void, onDragEnd?: (e: DragEndEvent) => void, onDrop?: (e: DropEvent) => void, tableViewProps?: Omit, 'children'>, items1?: any[], items2?: any[]}): JSX.Element { let {onDragStart, onDragEnd, onDrop} = props; let onDropAction = chain(action('onDrop'), onDrop); @@ -365,7 +369,7 @@ export function DragBetweenTablesExample(props: {onDragStart?: (e: DragStartEven }; // Use a random drag type so the items can only be reordered within the two lists and not dragged elsewhere. - let dragType = React.useMemo(() => `keys-${Math.random().toString(36).slice(2)}`, []); + let dragType = React.useMemo(() => randomDragTypeDragBetweenTablesExample, []); let {dragAndDropHooks} = useDragAndDrop({ getItems(keys) { diff --git a/packages/react-aria-components/stories/ListBox.stories.tsx b/packages/react-aria-components/stories/ListBox.stories.tsx index 0dec20894b4..e107e58d810 100644 --- a/packages/react-aria-components/stories/ListBox.stories.tsx +++ b/packages/react-aria-components/stories/ListBox.stories.tsx @@ -15,7 +15,7 @@ import {Collection, DropIndicator, GridLayout, Header, ListBox, ListBoxItem, Lis import {ListBoxLoadMoreItem} from '../'; import {LoadingSpinner, MyListBoxItem} from './utils'; import {Meta, StoryFn, StoryObj} from '@storybook/react'; -import React, {JSX} from 'react'; +import React, {JSX, useState} from 'react'; import {Size} from '@react-stately/virtualizer'; import styles from '../example/index.css'; import './styles.css'; @@ -514,14 +514,14 @@ export const VirtualizedListBoxGrid: StoryObj @@ -749,11 +749,11 @@ export const ListBoxScrollMargin: ListBoxStory = (args) => { items.push({id: i, name: `Item ${i}`, description: `Description ${i}`}); } return ( - {item => ( @@ -771,12 +771,12 @@ export const ListBoxSmoothScroll: ListBoxStory = (args) => { items.push({id: i, name: `Item ${i}`}); } return ( - {item => {item.name}} From a5803c0f66f126b0b0952d11a757628cd743616b Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Wed, 15 Oct 2025 15:46:17 +1100 Subject: [PATCH 12/26] fix lower react version tests --- packages/react-aria-components/src/Table.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/react-aria-components/src/Table.tsx b/packages/react-aria-components/src/Table.tsx index 6796ba57e26..74d8421f207 100644 --- a/packages/react-aria-components/src/Table.tsx +++ b/packages/react-aria-components/src/Table.tsx @@ -584,13 +584,13 @@ class TableHeaderNode extends CollectionNode { static readonly type = 'tableheader'; } -function THeadElementType(props) { +let THeadElementType = forwardRef(function THeadElementType(props: any, ref: ForwardedRef) { let {isVirtualized} = useContext(CollectionRendererContext); if (isVirtualized) { - return
; + return
; } - return
; -} + return ; +}); /** * A header within a `
; +}); + function TableDropIndicator(props: TableDropIndicatorProps, ref: ForwardedRef) { let { dropIndicatorProps, @@ -1390,24 +1419,21 @@ function TableDropIndicator(props: TableDropIndicatorProps, ref: ForwardedRef} data-drop-target={isDropTarget || undefined}> -
{renderProps.children} -
`, containing the table columns. @@ -1343,6 +1343,7 @@ export const Cell = /*#__PURE__*/ createLeafComponent(TableCellNode, (props: Cel } }); + // TODO: Lint doesn't catch these, it thinks we're not in a component render cycle here? let TD = useElementType('td'); let DOMProps = filterDOMProps(props as any, {global: true}); delete DOMProps.id; From be907420ca717a71e7a0ba1c9fe86f72c3c0d468 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Wed, 15 Oct 2025 16:16:07 +1100 Subject: [PATCH 13/26] turn on globals and fix errors --- eslint.config.mjs | 4 +- packages/@react-aria/dnd/test/dnd.test.js | 14 +++-- .../@react-aria/utils/test/mergeRefs.test.tsx | 23 ++++++--- .../breadcrumbs/test/Breadcrumbs.test.js | 13 +++-- .../@react-spectrum/utils/test/Slots.test.js | 25 +++++---- .../@react-spectrum/well/test/Well.test.js | 9 ++-- .../test/useCheckboxGroupState.test.tsx | 51 +++++++++++++------ packages/react-aria-components/src/utils.tsx | 1 - .../test/ListBox.test.js | 10 ++-- 9 files changed, 95 insertions(+), 55 deletions(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index fa42a517f2f..ce69f768024 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -234,9 +234,9 @@ 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/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, diff --git a/packages/@react-aria/dnd/test/dnd.test.js b/packages/@react-aria/dnd/test/dnd.test.js index 0c38d0b45d1..e470f91ae33 100644 --- a/packages/@react-aria/dnd/test/dnd.test.js +++ b/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 (<> @@ -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 (<> diff --git a/packages/@react-aria/utils/test/mergeRefs.test.tsx b/packages/@react-aria/utils/test/mergeRefs.test.tsx index e71a61d904a..a73b3479b90 100644 --- a/packages/@react-aria/utils/test/mergeRefs.test.tsx +++ b/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 ; }; render(); 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 ; }; diff --git a/packages/@react-spectrum/breadcrumbs/test/Breadcrumbs.test.js b/packages/@react-spectrum/breadcrumbs/test/Breadcrumbs.test.js index 43c50730cae..9a1c2d2b845 100644 --- a/packages/@react-spectrum/breadcrumbs/test/Breadcrumbs.test.js +++ b/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 ( - + Folder 1 ); - }; - let {getByLabelText} = render(); + }); + let {getByLabelText} = render(); let breadcrumb = getByLabelText('breadcrumbs-test'); expect(breadcrumb).toBe(ref.current.UNSAFE_getDOMNode()); }); diff --git a/packages/@react-spectrum/utils/test/Slots.test.js b/packages/@react-spectrum/utils/test/Slots.test.js index 8059f8edde9..8e178918bfd 100644 --- a/packages/@react-spectrum/utils/test/Slots.test.js +++ b/packages/@react-spectrum/utils/test/Slots.test.js @@ -12,7 +12,7 @@ import {ClearSlots, SlotProvider, useSlotProps} from '../'; import {pointerMap, render} from '@react-spectrum/test-utils-internal'; -import React, {StrictMode, useRef} from 'react'; +import React, {StrictMode, useEffect, useRef} from 'react'; import {useId, useSlotId} from '@react-aria/utils'; import {usePress} from '@react-aria/interactions'; import userEvent from '@testing-library/user-event'; @@ -25,11 +25,13 @@ describe('Slots', function () { }); function Component(props) { - results = useSlotProps(props, 'slotname'); - props = results; + let internalResults = useSlotProps(props, 'slotname'); + useEffect(() => { + results = internalResults; + }, [internalResults]); let ref = useRef(); - let {pressProps} = usePress({onPress: props.onPress, ref}); - let id = useId(props.id); + let {pressProps} = usePress({onPress: internalResults.onPress, ref}); + let id = useId(internalResults.id); return ; } @@ -174,7 +176,10 @@ describe('Slots', function () { function SlotsUseSlotId() { let id1 = useId(); let id2 = useId(); - id = useRef(id2).current; + let id3 = useRef(id2); + useEffect(() => { + id = id3.current; + }, [id3]); return ( <> @@ -228,12 +233,12 @@ describe('Slots', function () { ); let renderCountBeforeRerender = renderCount; - + // Trigger a rerender with the same stable props rerender( ); - + expect(renderCount).toEqual(renderCountBeforeRerender); }); @@ -277,12 +282,12 @@ describe('Slots', function () { ); let renderCountBeforeRerender = renderCount; - + // Trigger a rerender with the same stable props rerender( ); - + expect(renderCount).toEqual(renderCountBeforeRerender); }); }); diff --git a/packages/@react-spectrum/well/test/Well.test.js b/packages/@react-spectrum/well/test/Well.test.js index c60b45a598d..9e6b11a3301 100644 --- a/packages/@react-spectrum/well/test/Well.test.js +++ b/packages/@react-spectrum/well/test/Well.test.js @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import React, {useRef} from 'react'; +import React, {useEffect, useRef} from 'react'; import {render} from '@react-spectrum/test-utils-internal'; import {Well} from '../'; @@ -18,8 +18,11 @@ let refExists = (ComponentToCheck, children, props) => { let ref; let dataTestId = props['data-testid'] || 'refTestId'; let Component = () => { - ref = useRef(); - return ({children}); + let internalRef = useRef(); + useEffect(() => { + ref = internalRef; + }, [internalRef]); + return ({children}); }; let {getByText, getByTestId} = render(); diff --git a/packages/@react-stately/checkbox/test/useCheckboxGroupState.test.tsx b/packages/@react-stately/checkbox/test/useCheckboxGroupState.test.tsx index e0fe0a14152..8cadf75f91b 100644 --- a/packages/@react-stately/checkbox/test/useCheckboxGroupState.test.tsx +++ b/packages/@react-stately/checkbox/test/useCheckboxGroupState.test.tsx @@ -11,7 +11,7 @@ */ import {act, render} from '@react-spectrum/test-utils-internal'; -import React from 'react'; +import React, {useEffect} from 'react'; import {useCheckboxGroupState} from '../'; describe('useCheckboxGroupState', () => { @@ -107,7 +107,9 @@ describe('useCheckboxGroupState', () => { let setValue: (value: string[]) => void; function Test() { const state = useCheckboxGroupState({defaultValue: ['foo']}); - setValue = state.setValue; + useEffect(() => { + setValue = state.setValue; + }, [state]); return <>{state.value.join(', ')}; } const {container} = render(); @@ -128,7 +130,9 @@ describe('useCheckboxGroupState', () => { function Test() { const state = useCheckboxGroupState({defaultValue: ['foo'], onChange: onChangeSpy}); - setValue = state.setValue; + useEffect(() => { + setValue = state.setValue; + }, [state]); return <>{state.value.join(', ')}; } @@ -145,7 +149,9 @@ describe('useCheckboxGroupState', () => { let addValue: (value: string) => void; function Test() { const state = useCheckboxGroupState({defaultValue: ['foo']}); - addValue = state.addValue; + useEffect(() => { + addValue = state.addValue; + }, [state]); return <>{state.value.join(', ')}; } const {container} = render(); @@ -161,7 +167,9 @@ describe('useCheckboxGroupState', () => { let addValue: (value: string) => void; function Test() { const state = useCheckboxGroupState({defaultValue: ['foo']}); - addValue = state.addValue; + useEffect(() => { + addValue = state.addValue; + }, [state]); return <>{state.value.join(', ')}; } const {container} = render(); @@ -180,7 +188,9 @@ describe('useCheckboxGroupState', () => { let removeValue: (value: string) => void; function Test() { const state = useCheckboxGroupState({defaultValue: ['foo', 'qwe']}); - removeValue = state.removeValue; + useEffect(() => { + removeValue = state.removeValue; + }, [state]); return <>{state.value.join(', ')}; } const {container} = render(); @@ -196,7 +206,9 @@ describe('useCheckboxGroupState', () => { let toggleValue: (value: string) => void; function Test() { const state = useCheckboxGroupState({defaultValue: ['foo', 'qwe']}); - toggleValue = state.toggleValue; + useEffect(() => { + toggleValue = state.toggleValue; + }, [state]); return <>{state.value.join(', ')}; } const {container} = render(); @@ -218,7 +230,10 @@ describe('useCheckboxGroupState', () => { let toggleValue: (value: string) => void; function Test() { const state = useCheckboxGroupState({defaultValue: ['foo', 'qwe']}); - toggleValue = state.toggleValue; + useEffect(() => { + toggleValue = state.toggleValue; + }, [state]); + return <>{state.value.join(', ')}; } const {container} = render(); @@ -239,10 +254,12 @@ describe('useCheckboxGroupState', () => { function Test() { const state = useCheckboxGroupState({isReadOnly: true, defaultValue: ['test']}); - addValue = state.addValue; - removeValue = state.removeValue; - toggleValue = state.toggleValue; - setValue = state.setValue; + useEffect(() => { + addValue = state.addValue; + removeValue = state.removeValue; + toggleValue = state.toggleValue; + setValue = state.setValue; + }, [state]); return <>{state.value.join(', ')}; } @@ -284,10 +301,12 @@ describe('useCheckboxGroupState', () => { function Test() { const state = useCheckboxGroupState({isDisabled: true, defaultValue: ['test']}); - addValue = state.addValue; - removeValue = state.removeValue; - toggleValue = state.toggleValue; - setValue = state.setValue; + useEffect(() => { + addValue = state.addValue; + removeValue = state.removeValue; + toggleValue = state.toggleValue; + setValue = state.setValue; + }, [state]); return <>{state.value.join(', ')}; } diff --git a/packages/react-aria-components/src/utils.tsx b/packages/react-aria-components/src/utils.tsx index f75ec2dde13..30c686ec3ec 100644 --- a/packages/react-aria-components/src/utils.tsx +++ b/packages/react-aria-components/src/utils.tsx @@ -182,7 +182,6 @@ export function useSlottedContext(context: Context>, s export function useContextProps(props: T & SlotProps, ref: ForwardedRef | undefined, context: Context>): [T, RefObject] { let ctx = useSlottedContext(context, props.slot) || {}; - // @ts-ignore - TS says "Type 'unique symbol' cannot be used as an index type." but not sure why. let {ref: contextRef, ...contextProps} = ctx as any; let mergedRef = useObjectRef(useMemo(() => mergeRefs(ref, contextRef), [ref, contextRef])); let mergedProps = mergeProps(contextProps, props) as unknown as T; diff --git a/packages/react-aria-components/test/ListBox.test.js b/packages/react-aria-components/test/ListBox.test.js index b0173fc5a03..d5c23744631 100644 --- a/packages/react-aria-components/test/ListBox.test.js +++ b/packages/react-aria-components/test/ListBox.test.js @@ -29,7 +29,7 @@ import { Virtualizer } from '../'; import {ListBoxLoadMoreItem} from '../src/ListBox'; -import React, {useState} from 'react'; +import React, {useEffect, useState} from 'react'; import {User} from '@react-aria/test-utils'; import userEvent from '@testing-library/user-event'; @@ -312,9 +312,11 @@ describe('ListBox', () => { let setItemText; function Child() { let [showTwo, _setShowTwo] = useState(false); - setShowTwo = _setShowTwo; let [itemText, _setItemText] = useState('One'); - setItemText = _setItemText; + useEffect(() => { + setItemText = _setItemText; + setShowTwo = _setShowTwo; + }, [_setItemText, _setShowTwo]); return ( <> {itemText} @@ -1771,7 +1773,7 @@ describe('ListBox', () => { let {getByRole} = renderListbox({}, {onAction, onPressStart, onPressEnd, onPress, onClick}); let listBoxTester = testUtilUser.createTester('ListBox', {root: getByRole('listbox')}); await listBoxTester.triggerOptionAction({option: 1, interactionType}); - + expect(onAction).toHaveBeenCalledTimes(1); expect(onPressStart).toHaveBeenCalledTimes(1); expect(onPressEnd).toHaveBeenCalledTimes(1); From 90d350c085a9baff18d147acd8b5e61ef7cc7130 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Wed, 15 Oct 2025 17:29:52 +1100 Subject: [PATCH 14/26] Turn on immutability and start trying to fix it --- eslint.config.mjs | 2 +- .../checkbox/test/CheckboxGroup.test.js | 8 +++++--- .../combobox/src/MobileComboBox.tsx | 15 ++++++++------- .../@react-spectrum/datepicker/src/DateField.tsx | 5 +++-- .../@react-spectrum/datepicker/src/DatePicker.tsx | 5 +++-- .../datepicker/src/DateRangePicker.tsx | 11 ++++++----- packages/@react-spectrum/layout/src/Grid.tsx | 7 ++++--- .../menu/src/ContextualHelpTrigger.tsx | 7 ++++--- packages/@react-spectrum/menu/src/MenuItem.tsx | 5 +++-- packages/@react-spectrum/s2/src/RangeSlider.tsx | 4 ++-- .../s2/test/EditableTableView.test.tsx | 2 +- packages/@react-spectrum/slider/src/Slider.tsx | 8 ++++---- packages/@react-spectrum/tabs/src/Tabs.tsx | 5 +++-- .../@react-spectrum/textfield/src/TextArea.tsx | 6 +++--- .../@react-spectrum/textfield/src/TextField.tsx | 6 +++--- .../checkbox/src/useCheckboxGroupState.ts | 2 ++ .../datepicker/src/useDateFieldState.ts | 1 + .../datepicker/src/useDateRangePickerState.ts | 2 ++ .../dev/docs/pages/react-aria/home/Styles.tsx | 13 +++++++------ packages/react-aria-components/src/Dialog.tsx | 11 ++++++----- packages/react-aria-components/src/DropZone.tsx | 6 +++--- packages/react-aria-components/src/Table.tsx | 4 ++-- 22 files changed, 76 insertions(+), 59 deletions(-) diff --git a/eslint.config.mjs b/eslint.config.mjs index ce69f768024..9f010596984 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -235,7 +235,7 @@ export default [{ 'react-hooks/component-hook-factories': ERROR, 'react-hooks/gating': ERROR, 'react-hooks/globals': ERROR, - // 'react-hooks/immutability': ERROR, + 'react-hooks/immutability': 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 diff --git a/packages/@react-spectrum/checkbox/test/CheckboxGroup.test.js b/packages/@react-spectrum/checkbox/test/CheckboxGroup.test.js index 3f644c4af48..b6729d3d0e7 100644 --- a/packages/@react-spectrum/checkbox/test/CheckboxGroup.test.js +++ b/packages/@react-spectrum/checkbox/test/CheckboxGroup.test.js @@ -416,10 +416,11 @@ describe('CheckboxGroup', () => { }); if (parseInt(React.version, 10) >= 19) { - it('resets to defaultValue when submitting form action', async () => { - function Test() { + it.only('resets to defaultValue when submitting form action', async () => { + function Test() { const [value, formAction] = React.useActionState(() => ['dogs', 'cats'], []); - + console.log('value', value); + return ( @@ -442,6 +443,7 @@ describe('CheckboxGroup', () => { let button = getByTestId('submit'); await user.click(button); + console.log('after click'); expect(checkboxes[0]).toBeChecked(); expect(checkboxes[1]).toBeChecked(); expect(checkboxes[2]).not.toBeChecked(); diff --git a/packages/@react-spectrum/combobox/src/MobileComboBox.tsx b/packages/@react-spectrum/combobox/src/MobileComboBox.tsx index 5954ab301a3..0f36c80ff06 100644 --- a/packages/@react-spectrum/combobox/src/MobileComboBox.tsx +++ b/packages/@react-spectrum/combobox/src/MobileComboBox.tsx @@ -385,10 +385,11 @@ function ComboBoxTray(props: ComboBoxTrayProps) { // "double tap to edit text", as with a textbox or searchbox. We'd like double tapping to // open the virtual keyboard rather than closing the tray. // Unlike "combobox", "aria-expanded" is not a valid attribute on "searchbox". - inputProps.role = 'searchbox'; - inputProps['aria-haspopup'] = 'listbox'; - delete inputProps['aria-expanded']; - delete inputProps.onTouchEnd; + let newInputProps = {...inputProps}; + newInputProps.role = 'searchbox'; + newInputProps['aria-haspopup'] = 'listbox'; + delete newInputProps['aria-expanded']; + delete newInputProps.onTouchEnd; let clearButton = ( { if (loadingState === 'filtering' && !showLoading) { @@ -477,7 +478,7 @@ function ComboBoxTray(props: ComboBoxTrayProps) { if (e.key === 'Enter' && state.selectionManager.focusedKey == null) { popoverRef.current?.focus(); } else { - inputProps.onKeyDown?.(e); + newInputProps.onKeyDown?.(e); } }; @@ -496,7 +497,7 @@ function ComboBoxTray(props: ComboBoxTrayProps) { (props: SpectrumDateRangePickerProps, ref: FocusableRef) { - props = useProviderProps(props); - props = useFormProps(props); +export const DateRangePicker = React.forwardRef(function DateRangePicker(outerProps: SpectrumDateRangePickerProps, ref: FocusableRef) { + let withProviderProps = useProviderProps(outerProps); + let props = useFormProps(withProviderProps); let { isQuiet, isDisabled, @@ -101,8 +101,9 @@ export const DateRangePicker = React.forwardRef(function DateRangePicker +
{children}
); diff --git a/packages/@react-spectrum/menu/src/ContextualHelpTrigger.tsx b/packages/@react-spectrum/menu/src/ContextualHelpTrigger.tsx index b51d3580b4d..6bfece2a931 100644 --- a/packages/@react-spectrum/menu/src/ContextualHelpTrigger.tsx +++ b/packages/@react-spectrum/menu/src/ContextualHelpTrigger.tsx @@ -104,9 +104,10 @@ function ContextualHelpTrigger(props: InternalMenuDialogTriggerProps): ReactElem }, 220); // Matches transition duration }; + let newSubmenuTriggerProps = {...submenuTriggerProps}; if (isMobile) { - delete submenuTriggerProps.onBlur; - delete submenuTriggerProps.onHoverChange; + delete newSubmenuTriggerProps.onBlur; + delete newSubmenuTriggerProps.onHoverChange; if (trayContainerRef.current && submenuTriggerState.isOpen) { let subDialogKeyDown: KeyboardEventHandler = (e) => { switch (e.key) { @@ -160,7 +161,7 @@ function ContextualHelpTrigger(props: InternalMenuDialogTriggerProps): ReactElem return ( <> - {trigger} + {trigger} {submenuTriggerState.isOpen && overlay} diff --git a/packages/@react-spectrum/menu/src/MenuItem.tsx b/packages/@react-spectrum/menu/src/MenuItem.tsx index 519e514787d..118a969548c 100644 --- a/packages/@react-spectrum/menu/src/MenuItem.tsx +++ b/packages/@react-spectrum/menu/src/MenuItem.tsx @@ -92,9 +92,10 @@ export function MenuItem(props: MenuItemProps): JSX.Element { ); let endId = useSlotId(); let endProps: DOMAttributes = {}; + let newMenuItemProps = {...menuItemProps}; if (endId) { endProps.id = endId; - menuItemProps['aria-describedby'] = [menuItemProps['aria-describedby'], endId].filter(Boolean).join(' '); + newMenuItemProps['aria-describedby'] = [menuItemProps['aria-describedby'], endId].filter(Boolean).join(' '); } let contents = typeof rendered === 'string' @@ -104,7 +105,7 @@ export function MenuItem(props: MenuItemProps): JSX.Element { return ( ) { +export const RangeSlider = /*#__PURE__*/ forwardRef(function RangeSlider(outerProps: RangeSliderProps, outerRef: FocusableRef) { let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-spectrum/s2'); - [props, ref] = useSpectrumContextProps(props, ref, RangeSliderContext); + let [props, ref] = useSpectrumContextProps(outerProps, outerRef, RangeSliderContext); let formContext = useContext(FormContext); props = useFormProps(props); let { diff --git a/packages/@react-spectrum/s2/test/EditableTableView.test.tsx b/packages/@react-spectrum/s2/test/EditableTableView.test.tsx index 745e36b9959..aff46967e29 100644 --- a/packages/@react-spectrum/s2/test/EditableTableView.test.tsx +++ b/packages/@react-spectrum/s2/test/EditableTableView.test.tsx @@ -130,11 +130,11 @@ describe('TableView', () => { let [editableItems, setEditableItems] = useState(defaultItems); let intermediateValue = useRef(null); + let currentRequests = useRef}>>(new Map()); let saveItem = useCallback((id: Key, columnId: Key) => { setEditableItems(prev => prev.map(i => i.id === id ? {...i, isSaving: {...i.isSaving, [columnId]: false}} : i)); currentRequests.current.delete(id); }, []); - let currentRequests = useRef}>>(new Map()); let onChange = useCallback((id: Key, columnId: Key) => { let value = intermediateValue.current; if (value === null) { diff --git a/packages/@react-spectrum/slider/src/Slider.tsx b/packages/@react-spectrum/slider/src/Slider.tsx index fa531df2a6b..f8086797121 100644 --- a/packages/@react-spectrum/slider/src/Slider.tsx +++ b/packages/@react-spectrum/slider/src/Slider.tsx @@ -54,7 +54,7 @@ export const Slider = React.forwardRef(function Slider(props: SpectrumSliderProp {'--spectrum-slider-track-gradient': trackGradient && `linear-gradient(to ${direction === 'ltr' ? 'right' : 'left'}, ${trackGradient.join(', ')})`} }> {({trackRef, inputRef, state}: SliderBaseChildArguments) => { - fillOffset = fillOffset != null ? clamp(fillOffset, state.getThumbMinValue(0), state.getThumbMaxValue(0)) : fillOffset; + let newFillOffset = fillOffset != null ? clamp(fillOffset, state.getThumbMinValue(0), state.getThumbMaxValue(0)) : fillOffset; let cssDirection = direction === 'rtl' ? 'right' : 'left'; let lowerTrack = ( @@ -84,10 +84,10 @@ export const Slider = React.forwardRef(function Slider(props: SpectrumSliderProp ); let filledTrack: ReactNode = null; - if (isFilled && fillOffset != null) { - let width = state.getThumbPercent(0) - state.getValuePercent(fillOffset); + if (isFilled && newFillOffset != null) { + let width = state.getThumbPercent(0) - state.getValuePercent(newFillOffset); let isRightOfOffset = width > 0; - let offset = isRightOfOffset ? state.getValuePercent(fillOffset) : state.getThumbPercent(0); + let offset = isRightOfOffset ? state.getValuePercent(newFillOffset) : state.getThumbPercent(0); filledTrack = (
-
+
{props.children}
diff --git a/packages/@react-spectrum/textfield/src/TextArea.tsx b/packages/@react-spectrum/textfield/src/TextArea.tsx index 2424823560b..03a31278eab 100644 --- a/packages/@react-spectrum/textfield/src/TextArea.tsx +++ b/packages/@react-spectrum/textfield/src/TextArea.tsx @@ -24,9 +24,9 @@ import {useTextField} from '@react-aria/textfield'; * a sizable amount of text to enter. They allow for all customizations that * are available to text fields. */ -export const TextArea = React.forwardRef(function TextArea(props: SpectrumTextAreaProps, ref: Ref>) { - props = useProviderProps(props); - props = useFormProps(props); +export const TextArea = React.forwardRef(function TextArea(outerProps: SpectrumTextAreaProps, ref: Ref>) { + let withProviderProps = useProviderProps(outerProps); + let props = useFormProps(withProviderProps); let { isDisabled = false, isQuiet = false, diff --git a/packages/@react-spectrum/textfield/src/TextField.tsx b/packages/@react-spectrum/textfield/src/TextField.tsx index 4381c066d60..650e5724ce4 100644 --- a/packages/@react-spectrum/textfield/src/TextField.tsx +++ b/packages/@react-spectrum/textfield/src/TextField.tsx @@ -22,9 +22,9 @@ import {useTextField} from '@react-aria/textfield'; * with a keyboard. Various decorations can be displayed around the field to * communicate the entry requirements. */ -export const TextField = forwardRef(function TextField(props: SpectrumTextFieldProps, ref: Ref) { - props = useProviderProps(props); - props = useFormProps(props); +export const TextField = forwardRef(function TextField(outerProps: SpectrumTextFieldProps, ref: Ref) { + let withProviderProps = useProviderProps(outerProps); + let props = useFormProps(withProviderProps); let inputRef = useRef(null); let result = useTextField(props, inputRef); diff --git a/packages/@react-stately/checkbox/src/useCheckboxGroupState.ts b/packages/@react-stately/checkbox/src/useCheckboxGroupState.ts index b1f1d2db9fa..7e44cfa3988 100644 --- a/packages/@react-stately/checkbox/src/useCheckboxGroupState.ts +++ b/packages/@react-stately/checkbox/src/useCheckboxGroupState.ts @@ -101,6 +101,8 @@ export function useCheckboxGroupState(props: CheckboxGroupProps = {}): CheckboxG if (!selectedValues.includes(value)) { selectedValues = selectedValues.concat(value); setValue(selectedValues); + // TODO: I don't know how to fix this one, we shared the variable so that when each individual checkbox updates during the reset (all back to back) we'd use the correct value + // Otherwise we update the base state each time and never accumulate. It's all because we don't support setState callbacks. } }, removeValue(value) { diff --git a/packages/@react-stately/datepicker/src/useDateFieldState.ts b/packages/@react-stately/datepicker/src/useDateFieldState.ts index 2bf28053575..ccf7782e021 100644 --- a/packages/@react-stately/datepicker/src/useDateFieldState.ts +++ b/packages/@react-stately/datepicker/src/useDateFieldState.ts @@ -238,6 +238,7 @@ export function useDateFieldState(props: DateFi // If there is a value prop, and some segments were previously placeholders, mark them all as valid. if (value && Object.keys(validSegments).length < Object.keys(allSegments).length) { + // TODO: I don't know how to fix this one, the file is a bit crazy in terms of scoped variable sharing, I'm not sure if it's related to the useDateRangePickerState and useCheckboxGroupState reason validSegments = {...allSegments}; setValidSegments(validSegments); } diff --git a/packages/@react-stately/datepicker/src/useDateRangePickerState.ts b/packages/@react-stately/datepicker/src/useDateRangePickerState.ts index 18b1231491d..1f2f23cab2a 100644 --- a/packages/@react-stately/datepicker/src/useDateRangePickerState.ts +++ b/packages/@react-stately/datepicker/src/useDateRangePickerState.ts @@ -97,6 +97,7 @@ export function useDateRangePickerState(props: let value = controlledValue || placeholderValue; let setValue = (newValue: RangeValue | null) => { + // TODO: I don't know how to fix this one value = newValue || {start: null, end: null}; setPlaceholderValue(value); if (isCompleteRange(value)) { @@ -203,6 +204,7 @@ export function useDateRangePickerState(props: granularity, hasTime, setDate(part, date) { + // Both the start and end datefields update on form reset back to back, so this is a problem due to not supporting setState callbacks if (part === 'start') { setDateRange({start: date, end: dateRange?.end ?? null}); } else { diff --git a/packages/dev/docs/pages/react-aria/home/Styles.tsx b/packages/dev/docs/pages/react-aria/home/Styles.tsx index 9ca4af33d0f..6a6ffc76f6d 100644 --- a/packages/dev/docs/pages/react-aria/home/Styles.tsx +++ b/packages/dev/docs/pages/react-aria/home/Styles.tsx @@ -185,12 +185,6 @@ function AnimatedTabs({tabs}: {tabs: TabOptions[]}) { let x = useTransform(scrollXProgress, (x) => transform(x, 'offsetLeft')); let width = useTransform(scrollXProgress, (x) => transform(x, 'offsetWidth')); - // When the user scrolls, update the selected key - // so that the correct tab panel becomes interactive. - useMotionValueEvent(scrollXProgress, 'change', (x) => { - if (animationRef.current || !tabElements.length) {return;} - setSelectedKey(tabs[getIndex(x)].id); - }); // When the user clicks on a tab perform an animation of // the scroll position to the newly selected tab panel. @@ -236,6 +230,13 @@ function AnimatedTabs({tabs}: {tabs: TabOptions[]}) { ); }; + // When the user scrolls, update the selected key + // so that the correct tab panel becomes interactive. + useMotionValueEvent(scrollXProgress, 'change', (x) => { + if (animationRef.current || !tabElements.length) {return;} + setSelectedKey(tabs[getIndex(x)].id); + }); + // Scroll selected tab into view. let tabListScrollRef = useRef(null); useEffect(() => { diff --git a/packages/react-aria-components/src/Dialog.tsx b/packages/react-aria-components/src/Dialog.tsx index 5fc9a33cbe6..75f8fed2e01 100644 --- a/packages/react-aria-components/src/Dialog.tsx +++ b/packages/react-aria-components/src/Dialog.tsx @@ -70,23 +70,24 @@ export function DialogTrigger(props: DialogTriggerProps): JSX.Element { // This is done in RAC instead of hooks because otherwise we cannot distinguish // between context and props. Normally aria-labelledby overrides the title // but when sent by context we want the title to win. - triggerProps.id = useId(); - overlayProps['aria-labelledby'] = triggerProps.id; + let id = useId(); + let newTriggerProps = {...triggerProps, id}; + let newOverlayProps = {...overlayProps, 'aria-labelledby': id}; return ( - + {props.children} diff --git a/packages/react-aria-components/src/DropZone.tsx b/packages/react-aria-components/src/DropZone.tsx index 1138dd248c0..f23e9a4c5a1 100644 --- a/packages/react-aria-components/src/DropZone.tsx +++ b/packages/react-aria-components/src/DropZone.tsx @@ -68,9 +68,9 @@ export const DropZoneContext = createContext) { - let {isDisabled = false} = props; - [props, ref] = useContextProps(props, ref, DropZoneContext); +export const DropZone = forwardRef(function DropZone(outerProps: DropZoneProps, outerRef: ForwardedRef) { + let {isDisabled = false} = outerProps; + let [props, ref] = useContextProps(outerProps, outerRef, DropZoneContext); let dropzoneRef = useObjectRef(ref); let buttonRef = useRef(null); let {dropProps, dropButtonProps, isDropTarget} = useDrop({...props, ref: buttonRef, hasDropButton: true}); diff --git a/packages/react-aria-components/src/Table.tsx b/packages/react-aria-components/src/Table.tsx index 74d8421f207..716dc20172a 100644 --- a/packages/react-aria-components/src/Table.tsx +++ b/packages/react-aria-components/src/Table.tsx @@ -350,8 +350,8 @@ export interface TableProps extends Omit, 'children'>, Sty * A table displays data in rows and columns and enables a user to navigate its contents via directional navigation keys, * and optionally supports row selection and sorting. */ -export const Table = forwardRef(function Table(props: TableProps, ref: ForwardedRef) { - [props, ref] = useContextProps(props, ref, TableContext); +export const Table = forwardRef(function Table(outerProps: TableProps, outerRef: ForwardedRef) { + let [props, ref] = useContextProps(outerProps, outerRef, TableContext); // Separate selection state so we have access to it from collection components via useTableOptions. let selectionState = useMultipleSelectionState(props); From 8c6d21f4ff83ea6c5d2438541cafe404a571d679 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Thu, 16 Oct 2025 07:17:45 +1100 Subject: [PATCH 15/26] fix all the "easy" cases --- .../breadcrumbs/src/useBreadcrumbItem.ts | 3 +- packages/@react-aria/button/src/useButton.ts | 3 +- .../button/src/useToggleButtonGroup.ts | 3 +- .../calendar/src/useRangeCalendar.ts | 7 ++-- .../@react-aria/color/src/useColorArea.ts | 13 ++++--- .../@react-aria/color/src/useColorSlider.ts | 3 +- .../datepicker/src/useDateField.ts | 7 ++-- .../@react-aria/dnd/src/useDraggableItem.ts | 3 +- .../@react-aria/i18n/src/useDateFormatter.ts | 4 +-- .../@react-aria/menu/src/useMenuTrigger.ts | 3 +- packages/@react-aria/select/src/useSelect.ts | 5 +-- .../@react-aria/slider/test/useSlider.test.js | 8 +++-- .../slider/test/useSliderThumb.test.js | 8 +++-- .../spinbutton/src/useSpinButton.ts | 1 + packages/@react-aria/ssr/src/SSRProvider.tsx | 1 + .../@react-aria/table/src/useTableCell.ts | 3 +- packages/@react-aria/table/src/useTableRow.ts | 3 +- .../@react-aria/toast/src/useToastRegion.ts | 28 +++++++-------- packages/@react-aria/tree/src/useTree.ts | 3 +- .../actiongroup/src/ActionGroup.tsx | 3 +- .../src/MobileSearchAutocomplete.tsx | 34 +++++++++++-------- .../autocomplete/src/SearchAutocomplete.tsx | 6 ++-- .../@react-spectrum/card/src/CardView.tsx | 5 +-- .../@react-spectrum/color/src/ColorField.tsx | 10 +++--- .../@react-spectrum/combobox/src/ComboBox.tsx | 6 ++-- .../combobox/src/MobileComboBox.tsx | 19 ++++++----- 26 files changed, 114 insertions(+), 78 deletions(-) diff --git a/packages/@react-aria/breadcrumbs/src/useBreadcrumbItem.ts b/packages/@react-aria/breadcrumbs/src/useBreadcrumbItem.ts index fc791dd1dd3..b7c44318eb9 100644 --- a/packages/@react-aria/breadcrumbs/src/useBreadcrumbItem.ts +++ b/packages/@react-aria/breadcrumbs/src/useBreadcrumbItem.ts @@ -32,7 +32,8 @@ export function useBreadcrumbItem(props: AriaBreadcrumbItemProps, ref: RefObject ...otherProps } = props; - let {linkProps} = useLink({isDisabled: isDisabled || isCurrent, elementType, ...otherProps}, ref); + let {linkProps: linkBaseLinkProps} = useLink({isDisabled: isDisabled || isCurrent, elementType, ...otherProps}, ref); + let linkProps = {...linkBaseLinkProps}; let isHeading = /^h[1-6]$/.test(elementType); let itemProps: DOMAttributes = {}; diff --git a/packages/@react-aria/button/src/useButton.ts b/packages/@react-aria/button/src/useButton.ts index 163b00c05e6..d990e9fc3e9 100644 --- a/packages/@react-aria/button/src/useButton.ts +++ b/packages/@react-aria/button/src/useButton.ts @@ -101,7 +101,8 @@ export function useButton(props: AriaButtonOptions, ref: RefObject< ref }); - let {focusableProps} = useFocusable(props, ref); + let {focusableProps: focusableBaseFocusableProps} = useFocusable(props, ref); + let focusableProps = {...focusableBaseFocusableProps}; if (allowFocusWhenDisabled) { focusableProps.tabIndex = isDisabled ? -1 : focusableProps.tabIndex; } diff --git a/packages/@react-aria/button/src/useToggleButtonGroup.ts b/packages/@react-aria/button/src/useToggleButtonGroup.ts index 3115b1ade3f..3158322dc67 100644 --- a/packages/@react-aria/button/src/useToggleButtonGroup.ts +++ b/packages/@react-aria/button/src/useToggleButtonGroup.ts @@ -77,11 +77,12 @@ export function useToggleButtonGroupItem(props: AriaToggleButtonGroupItemOptions } }; - let {isPressed, isSelected, isDisabled, buttonProps} = useToggleButton({ + let {isPressed, isSelected, isDisabled, buttonProps: toggleButtonProps} = useToggleButton({ ...props, id: undefined, isDisabled: props.isDisabled || state.isDisabled }, toggleState, ref); + let buttonProps = {...toggleButtonProps}; if (state.selectionMode === 'single') { buttonProps.role = 'radio'; buttonProps['aria-checked'] = toggleState.isSelected; diff --git a/packages/@react-aria/calendar/src/useRangeCalendar.ts b/packages/@react-aria/calendar/src/useRangeCalendar.ts index 87695d4268d..110c7ee4592 100644 --- a/packages/@react-aria/calendar/src/useRangeCalendar.ts +++ b/packages/@react-aria/calendar/src/useRangeCalendar.ts @@ -22,7 +22,8 @@ import {useRef} from 'react'; * A range calendar displays one or more date grids and allows users to select a contiguous range of dates. */ export function useRangeCalendar(props: AriaRangeCalendarProps, state: RangeCalendarState, ref: RefObject): CalendarAria { - let res = useCalendarBase(props, state); + let {calendarProps: calendarBaseCalendarProps, ...res} = useCalendarBase(props, state); + let calendarProps = {...calendarBaseCalendarProps}; // We need to ignore virtual pointer events from VoiceOver due to these bugs. // https://bugs.webkit.org/show_bug.cgi?id=222627 @@ -62,7 +63,7 @@ export function useRangeCalendar(props: AriaRangeCalendarPr useEvent(windowRef, 'pointerup', endDragging); // Also stop range selection on blur, e.g. tabbing away from the calendar. - res.calendarProps.onBlur = e => { + calendarProps.onBlur = e => { if (!ref.current) { return; } @@ -78,5 +79,5 @@ export function useRangeCalendar(props: AriaRangeCalendarPr } }, {passive: false, capture: true}); - return res; + return {...res, calendarProps}; } diff --git a/packages/@react-aria/color/src/useColorArea.ts b/packages/@react-aria/color/src/useColorArea.ts index 13d56119eef..66fca3cb2bd 100644 --- a/packages/@react-aria/color/src/useColorArea.ts +++ b/packages/@react-aria/color/src/useColorArea.ts @@ -63,7 +63,12 @@ export function useColorArea(props: AriaColorAreaOptions, state: ColorAreaState) let {direction, locale} = useLocale(); - let [focusedInput, setFocusedInput] = useState<'x' | 'y' | null>(null); + let [focusedInput, _setFocusedInput] = useState<'x' | 'y' | null>(null); + let focusedInputRef = useRef<'x' | 'y' | null>(focusedInput); + let setFocusedInput = useCallback((newFocusedInput: 'x' | 'y' | null) => { + focusedInputRef.current = newFocusedInput; + _setFocusedInput(newFocusedInput); + }, [_setFocusedInput]); let focusInput = useCallback((inputRef:RefObject = inputXRef) => { if (inputRef.current) { focusWithoutScrolling(inputRef.current); @@ -157,8 +162,8 @@ export function useColorArea(props: AriaColorAreaOptions, state: ColorAreaState) } setValueChangedViaKeyboard(valueChanged); // set the focused input based on which axis has the greater delta - focusedInput = valueChanged && Math.abs(deltaY) > Math.abs(deltaX) ? 'y' : 'x'; - setFocusedInput(focusedInput); + let newFocusedInput = valueChanged && Math.abs(deltaY) > Math.abs(deltaX) ? 'y' as const : 'x' as const; + setFocusedInput(newFocusedInput); } else { currentPosition.current.x += (direction === 'rtl' ? -1 : 1) * deltaX / width ; currentPosition.current.y += deltaY / height; @@ -168,7 +173,7 @@ export function useColorArea(props: AriaColorAreaOptions, state: ColorAreaState) onMoveEnd() { isOnColorArea.current = false; state.setDragging(false); - let input = focusedInput === 'x' ? inputXRef : inputYRef; + let input = focusedInputRef.current === 'x' ? inputXRef : inputYRef; focusInput(input); } }; diff --git a/packages/@react-aria/color/src/useColorSlider.ts b/packages/@react-aria/color/src/useColorSlider.ts index 804914a983f..c5a19ee10db 100644 --- a/packages/@react-aria/color/src/useColorSlider.ts +++ b/packages/@react-aria/color/src/useColorSlider.ts @@ -55,7 +55,7 @@ export function useColorSlider(props: AriaColorSliderOptions, state: ColorSlider // @ts-ignore - ignore unused incompatible props let {groupProps, trackProps, labelProps, outputProps} = useSlider({...props, 'aria-label': ariaLabel}, state, trackRef); - let {inputProps, thumbProps} = useSliderThumb({ + let {inputProps: sliderInputProps, thumbProps} = useSliderThumb({ index: 0, orientation, isDisabled: props.isDisabled, @@ -64,6 +64,7 @@ export function useColorSlider(props: AriaColorSliderOptions, state: ColorSlider trackRef, inputRef }, state); + let inputProps = {...sliderInputProps}; let value = state.getDisplayColor(); let generateBackground = () => { diff --git a/packages/@react-aria/datepicker/src/useDateField.ts b/packages/@react-aria/datepicker/src/useDateField.ts index 5a04a465421..a448d7b338e 100644 --- a/packages/@react-aria/datepicker/src/useDateField.ts +++ b/packages/@react-aria/datepicker/src/useDateField.ts @@ -207,7 +207,8 @@ export interface AriaTimeFieldOptions extends AriaTimeField * Each part of a time value is displayed in an individually editable segment. */ export function useTimeField(props: AriaTimeFieldOptions, state: TimeFieldState, ref: RefObject): DateFieldAria { - let res = useDateField(props, state, ref); - res.inputProps.value = state.timeValue?.toString() || ''; - return res; + let {inputProps: dateFieldInputProps, ...res} = useDateField(props, state, ref); + let inputProps = {...dateFieldInputProps}; + inputProps.value = state.timeValue?.toString() || ''; + return {...res, inputProps}; } diff --git a/packages/@react-aria/dnd/src/useDraggableItem.ts b/packages/@react-aria/dnd/src/useDraggableItem.ts index 6d5781ddeaa..f48af63def9 100644 --- a/packages/@react-aria/dnd/src/useDraggableItem.ts +++ b/packages/@react-aria/dnd/src/useDraggableItem.ts @@ -66,7 +66,7 @@ const MESSAGES = { export function useDraggableItem(props: DraggableItemProps, state: DraggableCollectionState): DraggableItemResult { let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-aria/dnd'); let isDisabled = state.isDisabled || state.selectionManager.isDisabled(props.key); - let {dragProps, dragButtonProps} = useDrag({ + let {dragProps: draggableDragProps, dragButtonProps} = useDrag({ getItems() { return state.getItems(props.key); }, @@ -88,6 +88,7 @@ export function useDraggableItem(props: DraggableItemProps, state: DraggableColl clearGlobalDnDState(); } }); + let dragProps = {...draggableDragProps}; let item = state.collection.getItem(props.key); let numKeysForDrag = state.getKeysForDrag(props.key).size; diff --git a/packages/@react-aria/i18n/src/useDateFormatter.ts b/packages/@react-aria/i18n/src/useDateFormatter.ts index 482ea626dbc..a54b58bc20b 100644 --- a/packages/@react-aria/i18n/src/useDateFormatter.ts +++ b/packages/@react-aria/i18n/src/useDateFormatter.ts @@ -26,9 +26,9 @@ export interface DateFormatterOptions extends Intl.DateTimeFormatOptions { */ export function useDateFormatter(options?: DateFormatterOptions): DateFormatter { // Reuse last options object if it is shallowly equal, which allows the useMemo result to also be reused. - options = useDeepMemo(options ?? {}, isEqual); + let memoizedOptions = useDeepMemo(options ?? {}, isEqual); let {locale} = useLocale(); - return useMemo(() => new DateFormatter(locale, options), [locale, options]); + return useMemo(() => new DateFormatter(locale, memoizedOptions), [locale, memoizedOptions]); } function isEqual(a: DateFormatterOptions, b: DateFormatterOptions) { diff --git a/packages/@react-aria/menu/src/useMenuTrigger.ts b/packages/@react-aria/menu/src/useMenuTrigger.ts index 2d2227276a3..b4684b88deb 100644 --- a/packages/@react-aria/menu/src/useMenuTrigger.ts +++ b/packages/@react-aria/menu/src/useMenuTrigger.ts @@ -53,7 +53,8 @@ export function useMenuTrigger(props: AriaMenuTriggerProps, state: MenuTrigge } = props; let menuTriggerId = useId(); - let {triggerProps, overlayProps} = useOverlayTrigger({type}, state, ref); + let {triggerProps: overlayTriggerProps, overlayProps} = useOverlayTrigger({type}, state, ref); + let triggerProps = {...overlayTriggerProps}; let onKeyDown = (e) => { if (isDisabled) { diff --git a/packages/@react-aria/select/src/useSelect.ts b/packages/@react-aria/select/src/useSelect.ts index daebc1d3910..bac3640ae0d 100644 --- a/packages/@react-aria/select/src/useSelect.ts +++ b/packages/@react-aria/select/src/useSelect.ts @@ -99,7 +99,7 @@ export function useSelect(props: AriaSele if (state.selectionManager.selectionMode === 'multiple') { return; } - + switch (e.key) { case 'ArrowLeft': { // prevent scrolling containers @@ -124,13 +124,14 @@ export function useSelect(props: AriaSele } }; - let {typeSelectProps} = useTypeSelect({ + let {typeSelectProps: selectTypeSelectProps} = useTypeSelect({ keyboardDelegate: delegate, selectionManager: state.selectionManager, onTypeSelect(key) { state.setSelectedKey(key); } }); + let typeSelectProps = {...selectTypeSelectProps}; let {isInvalid, validationErrors, validationDetails} = state.displayValidation; let {labelProps, fieldProps, descriptionProps, errorMessageProps} = useField({ diff --git a/packages/@react-aria/slider/test/useSlider.test.js b/packages/@react-aria/slider/test/useSlider.test.js index 7bcaed97e0d..850e97c897e 100644 --- a/packages/@react-aria/slider/test/useSlider.test.js +++ b/packages/@react-aria/slider/test/useSlider.test.js @@ -64,7 +64,9 @@ describe('useSlider', () => { function Example(props) { let trackRef = useRef(null); let state = useSliderState({...props, numberFormatter}); - stateRef.current = state; + React.useEffect(() => { + stateRef.current = state; + }, [state]); let {trackProps} = useSlider(props, state, trackRef); return
; } @@ -182,7 +184,9 @@ describe('useSlider', () => { function Example(props) { let trackRef = useRef(null); let state = useSliderState({...props, numberFormatter}); - stateRef.current = state; + React.useEffect(() => { + stateRef.current = state; + }, [state]); let {trackProps} = useSlider(props, state, trackRef); return
; } diff --git a/packages/@react-aria/slider/test/useSliderThumb.test.js b/packages/@react-aria/slider/test/useSliderThumb.test.js index eefe094d5b6..cfb7fed181a 100644 --- a/packages/@react-aria/slider/test/useSliderThumb.test.js +++ b/packages/@react-aria/slider/test/useSliderThumb.test.js @@ -113,7 +113,9 @@ describe('useSliderThumb', () => { let input0Ref = useRef(null); let input1Ref = useRef(null); let state = useSliderState({...props, numberFormatter}); - stateRef.current = state; + React.useEffect(() => { + stateRef.current = state; + }, [state]); let {trackProps, thumbProps: commonThumbProps} = useSlider(props, state, trackRef); let {inputProps: input0Props, thumbProps: thumb0Props} = useSliderThumb({ ...commonThumbProps, @@ -273,7 +275,9 @@ describe('useSliderThumb', () => { let trackRef = useRef(null); let inputRef = useRef(null); let state = useSliderState({...props, numberFormatter}); - stateRef.current = state; + React.useEffect(() => { + stateRef.current = state; + }, [state]); let {trackProps} = useSlider(props, state, trackRef); let {inputProps, thumbProps} = useSliderThumb({ ...props, diff --git a/packages/@react-aria/spinbutton/src/useSpinButton.ts b/packages/@react-aria/spinbutton/src/useSpinButton.ts index de772fa5106..60bf944d33c 100644 --- a/packages/@react-aria/spinbutton/src/useSpinButton.ts +++ b/packages/@react-aria/spinbutton/src/useSpinButton.ts @@ -161,6 +161,7 @@ export function useSpinButton( _async.current = window.setTimeout( () => { if ((maxValue === undefined || isNaN(maxValue)) || (value === undefined || isNaN(value)) || value < maxValue) { + // TODO: false positive?? onIncrementPressStart(60); } }, diff --git a/packages/@react-aria/ssr/src/SSRProvider.tsx b/packages/@react-aria/ssr/src/SSRProvider.tsx index 83e07aa46e6..49bf1b2f98f 100644 --- a/packages/@react-aria/ssr/src/SSRProvider.tsx +++ b/packages/@react-aria/ssr/src/SSRProvider.tsx @@ -101,6 +101,7 @@ let canUseDOM = Boolean( let componentIds = new WeakMap(); +// TODO: this function needs serious help, haha function useCounter(isDisabled = false) { let ctx = useContext(SSRContext); let ref = useRef(null); diff --git a/packages/@react-aria/table/src/useTableCell.ts b/packages/@react-aria/table/src/useTableCell.ts index 1016cc13297..1159206867b 100644 --- a/packages/@react-aria/table/src/useTableCell.ts +++ b/packages/@react-aria/table/src/useTableCell.ts @@ -45,7 +45,8 @@ export interface TableCellAria { * @param ref - The ref attached to the cell element. */ export function useTableCell(props: AriaTableCellProps, state: TableState, ref: RefObject): TableCellAria { - let {gridCellProps, isPressed} = useGridCell(props, state, ref); + let {gridCellProps: tableGridCellProps, isPressed} = useGridCell(props, state, ref); + let gridCellProps = {...tableGridCellProps}; let columnKey = props.node.column?.key; if (columnKey != null && state.collection.rowHeaderColumnKeys.has(columnKey)) { gridCellProps.role = 'rowheader'; diff --git a/packages/@react-aria/table/src/useTableRow.ts b/packages/@react-aria/table/src/useTableRow.ts index 41556dd6e10..cd72e01c015 100644 --- a/packages/@react-aria/table/src/useTableRow.ts +++ b/packages/@react-aria/table/src/useTableRow.ts @@ -40,7 +40,8 @@ const EXPANSION_KEYS = { */ export function useTableRow(props: GridRowProps, state: TableState | TreeGridState, ref: RefObject): GridRowAria { let {node, isVirtualized} = props; - let {rowProps, ...states} = useGridRow, TableState>(props, state, ref); + let {rowProps: gridRowProps, ...states} = useGridRow, TableState>(props, state, ref); + let rowProps = {...gridRowProps}; let {direction} = useLocale(); if (isVirtualized && !(tableNestedRows() && 'expandedKeys' in state)) { diff --git a/packages/@react-aria/toast/src/useToastRegion.ts b/packages/@react-aria/toast/src/useToastRegion.ts index 1c6c3dbaf84..b2457321e4f 100644 --- a/packages/@react-aria/toast/src/useToastRegion.ts +++ b/packages/@react-aria/toast/src/useToastRegion.ts @@ -65,6 +65,20 @@ export function useToastRegion(props: AriaToastRegionProps, state: ToastState } }); + let lastFocused = useRef(null); + let {focusWithinProps} = useFocusWithin({ + onFocusWithin: (e) => { + isFocused.current = true; + lastFocused.current = e.relatedTarget as FocusableElement; + updateTimers(); + }, + onBlurWithin: () => { + isFocused.current = false; + lastFocused.current = null; + updateTimers(); + } + }); + // Manage focus within the toast region. // If a focused containing toast is removed, move focus to the next toast, or the previous toast if there is no next toast. let toasts = useRef([]); @@ -135,20 +149,6 @@ export function useToastRegion(props: AriaToastRegionProps, state: ToastState prevVisibleToasts.current = state.visibleToasts; }, [state.visibleToasts, ref]); - let lastFocused = useRef(null); - let {focusWithinProps} = useFocusWithin({ - onFocusWithin: (e) => { - isFocused.current = true; - lastFocused.current = e.relatedTarget as FocusableElement; - updateTimers(); - }, - onBlurWithin: () => { - isFocused.current = false; - lastFocused.current = null; - updateTimers(); - } - }); - // When the number of visible toasts becomes 0 or the region unmounts, // restore focus to the last element that had focus before the user moved focus // into the region. FocusScope restore focus doesn't update whenever the focus diff --git a/packages/@react-aria/tree/src/useTree.ts b/packages/@react-aria/tree/src/useTree.ts index 8936e026757..c015fbcff67 100644 --- a/packages/@react-aria/tree/src/useTree.ts +++ b/packages/@react-aria/tree/src/useTree.ts @@ -42,7 +42,8 @@ export interface TreeAria { * @param ref - The ref attached to the treegrid element. */ export function useTree(props: AriaTreeOptions, state: TreeState, ref: RefObject): TreeAria { - let {gridProps} = useGridList(props, state, ref); + let {gridProps: gridListGridProps} = useGridList(props, state, ref); + let gridProps = {...gridListGridProps}; gridProps.role = 'treegrid'; return { diff --git a/packages/@react-spectrum/actiongroup/src/ActionGroup.tsx b/packages/@react-spectrum/actiongroup/src/ActionGroup.tsx index 865a651eaba..9bf9d5ba3d7 100644 --- a/packages/@react-spectrum/actiongroup/src/ActionGroup.tsx +++ b/packages/@react-spectrum/actiongroup/src/ActionGroup.tsx @@ -377,9 +377,10 @@ function ActionGroupMenu({state, isDisabled, isEmphasized, staticColor, items // Use the key of the first item within the menu as the key of the button. // The key must actually exist in the collection for focus to work correctly. let key = items[0].key; - let {buttonProps} = useActionGroupItem({key}, state); + let {buttonProps: actionGroupItemButtonProps} = useActionGroupItem({key}, state); let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-spectrum/actiongroup'); + let buttonProps = {...actionGroupItemButtonProps}; // The menu button shouldn't act like an actual action group item. delete buttonProps.onPress; delete buttonProps.role; diff --git a/packages/@react-spectrum/autocomplete/src/MobileSearchAutocomplete.tsx b/packages/@react-spectrum/autocomplete/src/MobileSearchAutocomplete.tsx index 242a1e0c91b..e4b11bf54b6 100644 --- a/packages/@react-spectrum/autocomplete/src/MobileSearchAutocomplete.tsx +++ b/packages/@react-spectrum/autocomplete/src/MobileSearchAutocomplete.tsx @@ -52,8 +52,8 @@ import {useFormValidation} from '@react-aria/form'; import {useProviderProps} from '@react-spectrum/provider'; import {useSearchAutocomplete} from '@react-aria/autocomplete'; -function ForwardMobileSearchAutocomplete(props: SpectrumSearchAutocompleteProps, ref: FocusableRef) { - props = useProviderProps(props); +function ForwardMobileSearchAutocomplete(outerProps: SpectrumSearchAutocompleteProps, ref: FocusableRef) { + let props = useProviderProps(outerProps); let { isQuiet, @@ -95,18 +95,21 @@ function ForwardMobileSearchAutocomplete(props: SpectrumSearch let validationState = props.validationState || (isInvalid ? 'invalid' : undefined); let errorMessage = props.errorMessage ?? validationErrors.join(' '); - let {labelProps, fieldProps, descriptionProps, errorMessageProps} = useField({ + let {labelProps: fieldLabelProps, fieldProps, descriptionProps, errorMessageProps} = useField({ ...props, labelElementType: 'span', isInvalid, errorMessage }); - // Focus the button and show focus ring when clicking on the label - labelProps.onClick = () => { - if (!props.isDisabled && buttonRef.current) { - buttonRef.current.focus(); - setInteractionModality('keyboard'); + let labelProps = { + ...fieldLabelProps, + // Focus the button and show focus ring when clicking on the label + onClick: () => { + if (!props.isDisabled && buttonRef.current) { + buttonRef.current.focus(); + setInteractionModality('keyboard'); + } } }; @@ -435,9 +438,10 @@ function SearchAutocompleteTray(props: SearchAutocompleteTrayProps) { // VoiceOver on iOS reads "double tap to collapse" when focused on the input rather than // "double tap to edit text", as with a textbox or searchbox. We'd like double tapping to // open the virtual keyboard rather than closing the tray. - inputProps.role = 'searchbox'; - inputProps['aria-haspopup'] = 'listbox'; - delete inputProps.onTouchEnd; + let newInputProps = {...inputProps}; + newInputProps.role = 'searchbox'; + newInputProps['aria-haspopup'] = 'listbox'; + delete newInputProps.onTouchEnd; let clearButton = ( (props: SearchAutocompleteTrayProps) { } }, [inputRef, popoverRef, isTouchDown]); - let inputValue = inputProps.value; + let inputValue = newInputProps.value; let lastInputValue = useRef(inputValue); useEffect(() => { if (loadingState === 'filtering' && !showLoading) { @@ -531,8 +535,8 @@ function SearchAutocompleteTray(props: SearchAutocompleteTrayProps) { onSubmit(inputValue == null ? null : inputValue.toString(), null); } } else { - if (inputProps.onKeyDown) { - inputProps.onKeyDown(e); + if (newInputProps.onKeyDown) { + newInputProps.onKeyDown(e); } } }; @@ -562,7 +566,7 @@ function SearchAutocompleteTray(props: SearchAutocompleteTrayProps) { (props: SpectrumSearchAutocompleteProps, ref: FocusableRef) { - props = useProviderProps(props); - props = useFormProps(props); +function SearchAutocomplete(outerProps: SpectrumSearchAutocompleteProps, ref: FocusableRef) { + let withProviderProps = useProviderProps(outerProps); + let props = useFormProps(withProviderProps); let hasWarned = useRef(false); useEffect(() => { diff --git a/packages/@react-spectrum/card/src/CardView.tsx b/packages/@react-spectrum/card/src/CardView.tsx index e060a2e84a4..405b77ecc0e 100644 --- a/packages/@react-spectrum/card/src/CardView.tsx +++ b/packages/@react-spectrum/card/src/CardView.tsx @@ -230,15 +230,16 @@ function InternalCard(props) { cardOrientation = 'vertical'; } + let newGridCellProps = {...gridCellProps}; // We don't want to focus the checkbox (or any other focusable elements) within the Card // when pressing the arrow keys so we delete the key down handler here. Arrow key navigation between // the cards in the CardView is handled by useGrid => useSelectableCollection instead. - delete gridCellProps.onKeyDownCapture; + delete newGridCellProps.onKeyDownCapture; return (
) { - props = useProviderProps(props); - props = useFormProps(props); - [props] = useContextProps(props, null, ColorFieldContext); - +export const ColorField = React.forwardRef(function ColorField(outerProps: SpectrumColorFieldProps, ref: Ref) { + let withProviderProps = useProviderProps(outerProps); + let withFormProps = useFormProps(withProviderProps); + let [props] = useContextProps(withFormProps, null, ColorFieldContext); + let hasWarned = useRef(false); useEffect(() => { if (props.placeholder && !hasWarned.current && process.env.NODE_ENV !== 'production') { diff --git a/packages/@react-spectrum/combobox/src/ComboBox.tsx b/packages/@react-spectrum/combobox/src/ComboBox.tsx index 70e78af304a..2b30e99fd5d 100644 --- a/packages/@react-spectrum/combobox/src/ComboBox.tsx +++ b/packages/@react-spectrum/combobox/src/ComboBox.tsx @@ -56,9 +56,9 @@ import {useProvider, useProviderProps} from '@react-spectrum/provider'; /** * ComboBoxes combine a text entry with a picker menu, allowing users to filter longer lists to only the selections matching a query. */ -export const ComboBox = React.forwardRef(function ComboBox(props: SpectrumComboBoxProps, ref: FocusableRef) { - props = useProviderProps(props); - props = useFormProps(props); +export const ComboBox = React.forwardRef(function ComboBox(outerProps: SpectrumComboBoxProps, ref: FocusableRef) { + let withProviderProps = useProviderProps(outerProps); + let props = useFormProps(withProviderProps); let hasWarned = useRef(false); useEffect(() => { diff --git a/packages/@react-spectrum/combobox/src/MobileComboBox.tsx b/packages/@react-spectrum/combobox/src/MobileComboBox.tsx index 0f36c80ff06..7416b57fd6e 100644 --- a/packages/@react-spectrum/combobox/src/MobileComboBox.tsx +++ b/packages/@react-spectrum/combobox/src/MobileComboBox.tsx @@ -45,8 +45,8 @@ import {useFilter, useLocalizedStringFormatter} from '@react-aria/i18n'; import {useFormValidation} from '@react-aria/form'; import {useProviderProps} from '@react-spectrum/provider'; -export const MobileComboBox = React.forwardRef(function MobileComboBox(props: SpectrumComboBoxProps, ref: FocusableRef) { - props = useProviderProps(props); +export const MobileComboBox = React.forwardRef(function MobileComboBox(outerProps: SpectrumComboBoxProps, ref: FocusableRef) { + let props = useProviderProps(outerProps); let { isQuiet, @@ -86,18 +86,21 @@ export const MobileComboBox = React.forwardRef(function MobileComboBox(props: Sp let validationState = props.validationState || (isInvalid ? 'invalid' : undefined); let errorMessage = props.errorMessage ?? validationErrors.join(' '); - let {labelProps, fieldProps, descriptionProps, errorMessageProps} = useField({ + let {labelProps: fieldLabelProps, fieldProps, descriptionProps, errorMessageProps} = useField({ ...props, labelElementType: 'span', isInvalid, errorMessage }); - // Focus the button and show focus ring when clicking on the label - labelProps.onClick = () => { - if (!props.isDisabled) { - buttonRef.current?.focus(); - setInteractionModality('keyboard'); + let labelProps = { + ...fieldLabelProps, + onClick: () => { + // Focus the button and show focus ring when clicking on the label + if (!props.isDisabled) { + buttonRef.current?.focus(); + setInteractionModality('keyboard'); + } } }; From e7b7fea587ea24ab5fb7c1dba56daff770fb1ac0 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Wed, 15 Oct 2025 19:26:11 -0700 Subject: [PATCH 16/26] feat: Support function setState callback in useControlledState # Conflicts: # packages/@react-stately/checkbox/src/useCheckboxGroupState.ts --- .../checkbox/src/useCheckboxGroupState.ts | 12 +- .../utils/src/useControlledState.ts | 76 ++++---- .../utils/test/useControlledState.test.tsx | 174 ++++++++++++++++-- 3 files changed, 195 insertions(+), 67 deletions(-) diff --git a/packages/@react-stately/checkbox/src/useCheckboxGroupState.ts b/packages/@react-stately/checkbox/src/useCheckboxGroupState.ts index 7e44cfa3988..3a06fdd83b4 100644 --- a/packages/@react-stately/checkbox/src/useCheckboxGroupState.ts +++ b/packages/@react-stately/checkbox/src/useCheckboxGroupState.ts @@ -98,12 +98,12 @@ export function useCheckboxGroupState(props: CheckboxGroupProps = {}): CheckboxG if (props.isReadOnly || props.isDisabled) { return; } - if (!selectedValues.includes(value)) { - selectedValues = selectedValues.concat(value); - setValue(selectedValues); - // TODO: I don't know how to fix this one, we shared the variable so that when each individual checkbox updates during the reset (all back to back) we'd use the correct value - // Otherwise we update the base state each time and never accumulate. It's all because we don't support setState callbacks. - } + setValue(selectedValues => { + if (!selectedValues.includes(value)) { + return selectedValues.concat(value); + } + return selectedValues; + }); }, removeValue(value) { if (props.isReadOnly || props.isDisabled) { diff --git a/packages/@react-stately/utils/src/useControlledState.ts b/packages/@react-stately/utils/src/useControlledState.ts index 0bebf58b0f6..f40a59f06bc 100644 --- a/packages/@react-stately/utils/src/useControlledState.ts +++ b/packages/@react-stately/utils/src/useControlledState.ts @@ -10,12 +10,20 @@ * governing permissions and limitations under the License. */ -import {useCallback, useEffect, useRef, useState} from 'react'; +import React, {SetStateAction, useCallback, useEffect, useRef, useState} from 'react'; -export function useControlledState(value: Exclude, defaultValue: Exclude | undefined, onChange?: (v: C, ...args: any[]) => void): [T, (value: T, ...args: any[]) => void]; -export function useControlledState(value: Exclude | undefined, defaultValue: Exclude, onChange?: (v: C, ...args: any[]) => void): [T, (value: T, ...args: any[]) => void]; -export function useControlledState(value: T, defaultValue: T, onChange?: (v: C, ...args: any[]) => void): [T, (value: T, ...args: any[]) => void] { +// Copied from @react-aria/utils +const useLayoutEffect: typeof React.useLayoutEffect = typeof document !== 'undefined' + ? React.useLayoutEffect + : () => {}; + +export function useControlledState(value: Exclude, defaultValue: Exclude | undefined, onChange?: (v: C, ...args: any[]) => void): [T, (value: SetStateAction, ...args: any[]) => void]; +export function useControlledState(value: Exclude | undefined, defaultValue: Exclude, onChange?: (v: C, ...args: any[]) => void): [T, (value: SetStateAction, ...args: any[]) => void]; +export function useControlledState(value: T, defaultValue: T, onChange?: (v: C, ...args: any[]) => void): [T, (value: SetStateAction, ...args: any[]) => void] { + // Store the value in both state and a ref. The state value will only be used when uncontrolled. + // The ref is used to track the most current value, which is passed to the function setState callback. let [stateValue, setStateValue] = useState(value || defaultValue); + let valueRef = useRef(stateValue); let isControlledRef = useRef(value !== undefined); let isControlled = value !== undefined; @@ -27,49 +35,29 @@ export function useControlledState(value: T, defaultValue: T, onChange isControlledRef.current = isControlled; }, [isControlled]); + // After each render, update the ref to the current value. + // This ensures that the setState callback argument is reset. + // Note: the effect should not have any dependencies so that controlled values always reset. let currentValue = isControlled ? value : stateValue; - let setValue = useCallback((value, ...args) => { - let onChangeCaller = (value, ...onChangeArgs) => { - if (onChange) { - if (!Object.is(currentValue, value)) { - onChange(value, ...onChangeArgs); - } - } - if (!isControlled) { - // If uncontrolled, mutate the currentValue local variable so that - // calling setState multiple times with the same value only emits onChange once. - // We do not use a ref for this because we specifically _do_ want the value to - // reset every render, and assigning to a ref in render breaks aborted suspended renders. - // eslint-disable-next-line react-hooks/exhaustive-deps - currentValue = value; - } - }; + useLayoutEffect(() => { + valueRef.current = currentValue; + }); + + let setValue = useCallback((value: SetStateAction, ...args: any[]) => { + // @ts-ignore - TS doesn't know that T cannot be a function. + let newValue = typeof value === 'function' ? value(valueRef.current) : value; + if (!Object.is(valueRef.current, newValue)) { + // Update the ref so that the next setState callback has the most recent value. + valueRef.current = newValue; + + // Always trigger a setState, even when controlled, so that the layout effect above runs to reset the value. + setStateValue(newValue); - if (typeof value === 'function') { - if (process.env.NODE_ENV !== 'production') { - console.warn('We can not support a function callback. See Github Issues for details https://github.com/adobe/react-spectrum/issues/2320'); - } - // this supports functional updates https://reactjs.org/docs/hooks-reference.html#functional-updates - // when someone using useControlledState calls setControlledState(myFunc) - // this will call our useState setState with a function as well which invokes myFunc and calls onChange with the value from myFunc - // if we're in an uncontrolled state, then we also return the value of myFunc which to setState looks as though it was just called with myFunc from the beginning - // otherwise we just return the controlled value, which won't cause a rerender because React knows to bail out when the value is the same - let updateFunction = (oldValue, ...functionArgs) => { - let interceptedValue = value(isControlled ? currentValue : oldValue, ...functionArgs); - onChangeCaller(interceptedValue, ...args); - if (!isControlled) { - return interceptedValue; - } - return oldValue; - }; - setStateValue(updateFunction); - } else { - if (!isControlled) { - setStateValue(value); - } - onChangeCaller(value, ...args); + // Trigger onChange. Note that if setState is called multiple times in a single event, + // onChange will be called for each one instead of only once. + onChange?.(newValue, ...args); } - }, [isControlled, currentValue, onChange]); + }, [onChange]); return [currentValue, setValue]; } diff --git a/packages/@react-stately/utils/test/useControlledState.test.tsx b/packages/@react-stately/utils/test/useControlledState.test.tsx index ea578a8ba37..18c9307afe5 100644 --- a/packages/@react-stately/utils/test/useControlledState.test.tsx +++ b/packages/@react-stately/utils/test/useControlledState.test.tsx @@ -70,15 +70,12 @@ describe('useControlledState tests', function () { expect(onChangeSpy).toHaveBeenCalledTimes(1); }); - // @deprecated - ignore TS it('can handle callback setValue behavior', () => { let onChangeSpy = jest.fn(); - let consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); let {result} = renderHook(() => useControlledState(undefined, 'defaultValue', onChangeSpy)); let [value, setValue] = result.current; expect(value).toBe('defaultValue'); expect(onChangeSpy).not.toHaveBeenCalled(); - // @ts-ignore act(() => setValue((prevValue) => { expect(prevValue).toBe('defaultValue'); return 'newValue'; @@ -86,7 +83,6 @@ describe('useControlledState tests', function () { [value, setValue] = result.current; expect(value).toBe('newValue'); expect(onChangeSpy).toHaveBeenLastCalledWith('newValue'); - expect(consoleWarnSpy).toHaveBeenLastCalledWith('We can not support a function callback. See Github Issues for details https://github.com/adobe/react-spectrum/issues/2320'); }); it('does not trigger too many renders', async () => { @@ -136,16 +132,13 @@ describe('useControlledState tests', function () { expect(onChangeSpy).not.toHaveBeenCalled(); }); - // @deprecated - ignore TS it('can handle controlled callback setValue behavior', () => { let onChangeSpy = jest.fn(); - let consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); let {result} = renderHook(() => useControlledState('controlledValue', 'defaultValue', onChangeSpy)); let [value, setValue] = result.current; expect(value).toBe('controlledValue'); expect(onChangeSpy).not.toHaveBeenCalled(); - // @ts-ignore act(() => setValue((prevValue) => { expect(prevValue).toBe('controlledValue'); return 'newValue'; @@ -156,7 +149,6 @@ describe('useControlledState tests', function () { onChangeSpy.mockClear(); - // @ts-ignore act(() => setValue((prevValue) => { expect(prevValue).toBe('controlledValue'); return 'controlledValue'; @@ -164,14 +156,43 @@ describe('useControlledState tests', function () { [value, setValue] = result.current; expect(value).toBe('controlledValue'); expect(onChangeSpy).not.toHaveBeenCalled(); - expect(consoleWarnSpy).toHaveBeenLastCalledWith('We can not support a function callback. See Github Issues for details https://github.com/adobe/react-spectrum/issues/2320'); }); - // @deprecated - ignore TS + it('can handle controlled callback setValue behavior called multiple times within a single act', async () => { + let onChangeSpy = jest.fn(); + let TestComponent = () => { + let [val, setVal] = useState('controlledValue'); + let [value, setValue] = useControlledState(val, 'defaultValue', (newval) => { + setVal(newval); + onChangeSpy(newval); + }); + return ( + + ); + }; + let {getByRole} = render(); + let button = getByRole('button'); + expect(button).toHaveTextContent('controlledValue'); + expect(onChangeSpy).not.toHaveBeenCalled(); + await user.click(button); + expect(button).toHaveTextContent('controlledValue-newValue-wombat'); + expect(onChangeSpy).toHaveBeenCalledTimes(2); + expect(onChangeSpy).toHaveBeenNthCalledWith(1, 'controlledValue-newValue'); + expect(onChangeSpy).toHaveBeenNthCalledWith(2, 'controlledValue-newValue-wombat'); + }); it('can handle controlled callback setValue behavior after prop change', () => { let onChangeSpy = jest.fn(); let propValue = 'controlledValue'; - let consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); let {result, rerender} = renderHook(() => useControlledState(propValue, 'defaultValue', onChangeSpy)); let [value, setValue] = result.current; expect(value).toBe('controlledValue'); @@ -181,7 +202,6 @@ describe('useControlledState tests', function () { rerender(); [value, setValue] = result.current; - // @ts-ignore act(() => setValue((prevValue) => { expect(prevValue).toBe('updated'); return 'newValue'; @@ -192,7 +212,6 @@ describe('useControlledState tests', function () { onChangeSpy.mockClear(); - // @ts-ignore act(() => setValue((prevValue) => { expect(prevValue).toBe('updated'); return 'updated'; @@ -200,9 +219,6 @@ describe('useControlledState tests', function () { [value, setValue] = result.current; expect(value).toBe('updated'); expect(onChangeSpy).not.toHaveBeenCalled(); - expect(consoleWarnSpy).toHaveBeenCalledTimes(2); - expect(consoleWarnSpy).toHaveBeenLastCalledWith('We can not support a function callback. See Github Issues for details https://github.com/adobe/react-spectrum/issues/2320'); - }); it('will console warn if the programmer tries to switch from controlled to uncontrolled', () => { @@ -316,7 +332,75 @@ describe('useControlledState tests', function () { await user.click(show); expect(show).toHaveTextContent('Loading'); expect(value).toHaveTextContent('2'); + // Since the previous render was thrown away, the current value shown + // to the user is still 2. Clicking the button should bump it to 3 again. + await user.click(value); + expect(value).toHaveTextContent('3'); + expect(onChange).toHaveBeenCalledTimes(2); + expect(onChange).toHaveBeenLastCalledWith(3); + }); + it('should work with suspense when controlled with function set state', async () => { + if (parseInt(React.version, 10) < 18) { + return; + } + const AsyncChild = React.lazy(() => new Promise(() => {})); + function Test(props) { + let [value, setValue] = useState(1); + let [showChild, setShowChild] = useState(false); + return ( + <> + { + setValue(3); + setShowChild(true); + }} /> + { + setValue(v); + props.onChange(v); + }} /> + {showChild && } + + ); + } + function Child(props) { + let [value, setValue] = useControlledState(props.value, props.defaultValue, props.onChange); + return ( + + ); + } + function TransitionButton({onClick}) { + let [isPending, startTransition] = React.useTransition(); + return ( + + ); + } + let onChange = jest.fn(); + let tree = render(); + let value = tree.getByTestId('value'); + let show = tree.getByTestId('show'); + expect(value).toHaveTextContent('1'); + await user.click(value); + // Clicking the button should update the value as normal. + expect(value).toHaveTextContent('2'); + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenLastCalledWith(2); + // Clicking the show button starts a transition. The new value of 3 + // will be thrown away by React since the component suspended. + expect(show).toHaveTextContent('Show'); + await user.click(show); + expect(show).toHaveTextContent('Loading'); + expect(value).toHaveTextContent('2'); // Since the previous render was thrown away, the current value shown // to the user is still 2. Clicking the button should bump it to 3 again. await user.click(value); @@ -374,7 +458,7 @@ describe('useControlledState tests', function () { // Attempting to change the value will be aborted again. await user.click(value); expect(value).toHaveTextContent('1 (Loading)'); - expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenCalledTimes(2); expect(onChange).toHaveBeenLastCalledWith(2); // Now resolve the suspended component. @@ -386,7 +470,63 @@ describe('useControlledState tests', function () { // Now incrementing works again. await user.click(value); expect(value).toHaveTextContent('3'); + expect(onChange).toHaveBeenCalledTimes(3); + expect(onChange).toHaveBeenLastCalledWith(3); + }); + + it('should work with suspense when uncontrolled with function set state', async () => { + if (parseInt(React.version, 10) < 18) { + return; + } + let resolve; + const AsyncChild = React.lazy(() => new Promise((r) => {resolve = r;})); + function Test(props) { + let [value, setValue] = useControlledState(undefined, 1, props.onChange); + let [showChild, setShowChild] = useState(false); + let [isPending, startTransition] = React.useTransition(); + return ( + <> + + {showChild && } + + ); + } + function LoadedComponent() { + return
Hello
; + } + let onChange = jest.fn(); + let tree = render(); + let value = tree.getByTestId('value'); + expect(value).toHaveTextContent('1'); + await user.click(value); + // React aborts the render, so the value stays at 1. + expect(value).toHaveTextContent('1 (Loading)'); + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenLastCalledWith(2); + // Attempting to change the value will be aborted again. + await user.click(value); + expect(value).toHaveTextContent('1 (Loading)'); expect(onChange).toHaveBeenCalledTimes(2); + expect(onChange).toHaveBeenLastCalledWith(2); + // Now resolve the suspended component. + // Value should now update to the latest one. + resolve({default: LoadedComponent}); + await act(() => Promise.resolve()); + expect(value).toHaveTextContent('2'); + // Now incrementing works again. + await user.click(value); + expect(value).toHaveTextContent('3'); + expect(onChange).toHaveBeenCalledTimes(3); expect(onChange).toHaveBeenLastCalledWith(3); }); }); From 7057ed4119b21f39a8ea739e334ac8a44baab063 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Wed, 15 Oct 2025 20:00:22 -0700 Subject: [PATCH 17/26] fixes --- packages/@react-stately/calendar/src/types.ts | 6 +++--- .../@react-stately/calendar/src/useRangeCalendarState.ts | 2 +- packages/@react-stately/utils/src/useControlledState.ts | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/@react-stately/calendar/src/types.ts b/packages/@react-stately/calendar/src/types.ts index 4fbf3c3fe60..ec7b26470c6 100644 --- a/packages/@react-stately/calendar/src/types.ts +++ b/packages/@react-stately/calendar/src/types.ts @@ -106,11 +106,11 @@ export interface CalendarState extends CalendarStateBase { setValue(value: CalendarDate | null): void } -export interface RangeCalendarState extends CalendarStateBase { +export interface RangeCalendarState extends CalendarStateBase { /** The currently selected date range. */ - readonly value: RangeValue | null, + readonly value: RangeValue | null, /** Sets the currently selected date range. */ - setValue(value: RangeValue | null): void, + setValue(value: RangeValue | null): void, /** Highlights the given date during selection, e.g. by hovering or dragging. */ highlightDate(date: CalendarDate): void, /** The current anchor date that the user clicked on to begin range selection. */ diff --git a/packages/@react-stately/calendar/src/useRangeCalendarState.ts b/packages/@react-stately/calendar/src/useRangeCalendarState.ts index e57623fe57c..284500aa184 100644 --- a/packages/@react-stately/calendar/src/useRangeCalendarState.ts +++ b/packages/@react-stately/calendar/src/useRangeCalendarState.ts @@ -45,7 +45,7 @@ export interface RangeCalendarStateOptions exte * Provides state management for a range calendar component. * A range calendar displays one or more date grids and allows users to select a contiguous range of dates. */ -export function useRangeCalendarState(props: RangeCalendarStateOptions): RangeCalendarState { +export function useRangeCalendarState(props: RangeCalendarStateOptions): RangeCalendarState { let { value: valueProp, defaultValue, diff --git a/packages/@react-stately/utils/src/useControlledState.ts b/packages/@react-stately/utils/src/useControlledState.ts index f40a59f06bc..0e52b499b6b 100644 --- a/packages/@react-stately/utils/src/useControlledState.ts +++ b/packages/@react-stately/utils/src/useControlledState.ts @@ -12,9 +12,9 @@ import React, {SetStateAction, useCallback, useEffect, useRef, useState} from 'react'; -// Copied from @react-aria/utils -const useLayoutEffect: typeof React.useLayoutEffect = typeof document !== 'undefined' - ? React.useLayoutEffect +// Use the earliest effect possible to reset the ref below. +const useEarlyEffect: typeof React.useLayoutEffect = typeof document !== 'undefined' + ? React['useInsertionEffect'] ?? React.useLayoutEffect : () => {}; export function useControlledState(value: Exclude, defaultValue: Exclude | undefined, onChange?: (v: C, ...args: any[]) => void): [T, (value: SetStateAction, ...args: any[]) => void]; @@ -39,7 +39,7 @@ export function useControlledState(value: T, defaultValue: T, onChange // This ensures that the setState callback argument is reset. // Note: the effect should not have any dependencies so that controlled values always reset. let currentValue = isControlled ? value : stateValue; - useLayoutEffect(() => { + useEarlyEffect(() => { valueRef.current = currentValue; }); From 3a5e3b2743e8ed72a0f89df844d3a56d3723dd37 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Thu, 16 Oct 2025 15:37:31 +1100 Subject: [PATCH 18/26] fix docs build --- packages/@react-stately/calendar/docs/useRangeCalendarState.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-stately/calendar/docs/useRangeCalendarState.mdx b/packages/@react-stately/calendar/docs/useRangeCalendarState.mdx index d29afae707f..590eae2c19a 100644 --- a/packages/@react-stately/calendar/docs/useRangeCalendarState.mdx +++ b/packages/@react-stately/calendar/docs/useRangeCalendarState.mdx @@ -33,7 +33,7 @@ keywords: [date, calendar, state] ## Interface - + ## Example From c0dcf3fe1d7410bc641a06ae6ac8638787d157ec Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Thu, 16 Oct 2025 17:55:51 +1100 Subject: [PATCH 19/26] fix useDateFieldState immutable lint errors --- .../checkbox/test/CheckboxGroup.test.js | 4 +- .../datepicker/src/useDateFieldState.ts | 76 +++++++++---------- 2 files changed, 35 insertions(+), 45 deletions(-) diff --git a/packages/@react-spectrum/checkbox/test/CheckboxGroup.test.js b/packages/@react-spectrum/checkbox/test/CheckboxGroup.test.js index b6729d3d0e7..02557da8356 100644 --- a/packages/@react-spectrum/checkbox/test/CheckboxGroup.test.js +++ b/packages/@react-spectrum/checkbox/test/CheckboxGroup.test.js @@ -416,10 +416,9 @@ describe('CheckboxGroup', () => { }); if (parseInt(React.version, 10) >= 19) { - it.only('resets to defaultValue when submitting form action', async () => { + it('resets to defaultValue when submitting form action', async () => { function Test() { const [value, formAction] = React.useActionState(() => ['dogs', 'cats'], []); - console.log('value', value); return ( @@ -443,7 +442,6 @@ describe('CheckboxGroup', () => { let button = getByTestId('submit'); await user.click(button); - console.log('after click'); expect(checkboxes[0]).toBeChecked(); expect(checkboxes[1]).toBeChecked(); expect(checkboxes[2]).not.toBeChecked(); diff --git a/packages/@react-stately/datepicker/src/useDateFieldState.ts b/packages/@react-stately/datepicker/src/useDateFieldState.ts index ccf7782e021..b505080cbbd 100644 --- a/packages/@react-stately/datepicker/src/useDateFieldState.ts +++ b/packages/@react-stately/datepicker/src/useDateFieldState.ts @@ -16,7 +16,7 @@ import {DatePickerProps, DateValue, Granularity, MappedDateValue} from '@react-t import {FormValidationState, useFormValidationState} from '@react-stately/form'; import {getPlaceholder} from './placeholders'; import {useControlledState} from '@react-stately/utils'; -import {useEffect, useMemo, useRef, useState} from 'react'; +import {useMemo, useRef, useState} from 'react'; import {ValidationState} from '@react-types/shared'; export type SegmentType = 'era' | 'year' | 'month' | 'day' | 'hour' | 'minute' | 'second' | 'dayPeriod' | 'literal' | 'timeZoneName'; @@ -224,39 +224,34 @@ export function useDateFieldState(props: DateFi let clearedSegment = useRef(null); // Reset placeholder when calendar changes - let lastCalendar = useRef(calendar); - useEffect(() => { - if (!isEqualCalendar(calendar, lastCalendar.current)) { - lastCalendar.current = calendar; - setPlaceholderDate(placeholder => - Object.keys(validSegments).length > 0 - ? toCalendar(placeholder, calendar) - : createPlaceholderDate(props.placeholderValue, granularity, calendar, defaultTimeZone) - ); - } - }, [calendar, granularity, validSegments, defaultTimeZone, props.placeholderValue]); + let [lastCalendar, setLastCalendar] = useState(calendar); + if (!isEqualCalendar(calendar, lastCalendar)) { + setLastCalendar(calendar); + setPlaceholderDate(placeholder => + Object.keys(validSegments).length > 0 + ? toCalendar(placeholder, calendar) + : createPlaceholderDate(props.placeholderValue, granularity, calendar, defaultTimeZone) + ); + } // If there is a value prop, and some segments were previously placeholders, mark them all as valid. if (value && Object.keys(validSegments).length < Object.keys(allSegments).length) { - // TODO: I don't know how to fix this one, the file is a bit crazy in terms of scoped variable sharing, I'm not sure if it's related to the useDateRangePickerState and useCheckboxGroupState reason - validSegments = {...allSegments}; - setValidSegments(validSegments); + setValidSegments({...allSegments}); } // If the value is set to null and all segments are valid, reset the placeholder. if (value == null && Object.keys(validSegments).length === Object.keys(allSegments).length) { - validSegments = {}; - setValidSegments(validSegments); + setValidSegments({}); setPlaceholderDate(createPlaceholderDate(props.placeholderValue, granularity, calendar, defaultTimeZone)); } // If all segments are valid, use the date from state, otherwise use the placeholder date. let displayValue = calendarValue && Object.keys(validSegments).length >= Object.keys(allSegments).length ? calendarValue : placeholderDate; - let setValue = (newValue: DateValue) => { + let setValue = (newValue: DateValue, newValidSegments: Partial = validSegments) => { if (props.isDisabled || props.isReadOnly) { return; } - let validKeys = Object.keys(validSegments); + let validKeys = Object.keys(newValidSegments); let allKeys = Object.keys(allSegments); // if all the segments are completed or a timefield with everything but am/pm set the time, also ignore when am/pm cleared @@ -267,13 +262,12 @@ export function useDateFieldState(props: DateFi } else if ( (validKeys.length === 0 && clearedSegment.current == null) || validKeys.length >= allKeys.length || - (validKeys.length === allKeys.length - 1 && allSegments.dayPeriod && !validSegments.dayPeriod && clearedSegment.current !== 'dayPeriod') + (validKeys.length === allKeys.length - 1 && allSegments.dayPeriod && !newValidSegments.dayPeriod && clearedSegment.current !== 'dayPeriod') ) { // If the field was empty (no valid segments) or all segments are completed, commit the new value. // When committing from an empty state, mark every segment as valid so value is committed. if (validKeys.length === 0) { - validSegments = {...allSegments}; - setValidSegments(validSegments); + setValidSegments({...allSegments}); } // The display calendar should not have any effect on the emitted value. @@ -294,28 +288,27 @@ export function useDateFieldState(props: DateFi // When the era field appears, mark it valid if the year field is already valid. // If the era field disappears, remove it from the valid segments. if (allSegments.era && validSegments.year && !validSegments.era) { - validSegments.era = true; - setValidSegments({...validSegments}); + setValidSegments({...validSegments, era: true}); } else if (!allSegments.era && validSegments.era) { - delete validSegments.era; - setValidSegments({...validSegments}); + setValidSegments({...validSegments, era: undefined}); } let markValid = (part: Intl.DateTimeFormatPartTypes) => { - validSegments[part] = true; + let newValidSegments = {...validSegments, [part]: true}; if (part === 'year' && allSegments.era) { - validSegments.era = true; + newValidSegments.era = true; } - setValidSegments({...validSegments}); + setValidSegments(newValidSegments); + return newValidSegments; }; let adjustSegment = (type: Intl.DateTimeFormatPartTypes, amount: number) => { if (!validSegments[type]) { - markValid(type); - let validKeys = Object.keys(validSegments); + let newValidSegments = markValid(type); + let validKeys = Object.keys(newValidSegments); let allKeys = Object.keys(allSegments); - if (validKeys.length >= allKeys.length || (validKeys.length === allKeys.length - 1 && allSegments.dayPeriod && !validSegments.dayPeriod)) { - setValue(displayValue); + if (validKeys.length >= allKeys.length || (validKeys.length === allKeys.length - 1 && allSegments.dayPeriod && !newValidSegments.dayPeriod)) { + setValue(displayValue, newValidSegments); } } else { setValue(addSegment(displayValue, type, amount, resolvedOptions)); @@ -368,27 +361,26 @@ export function useDateFieldState(props: DateFi adjustSegment(part, -(PAGE_STEP[part] || 1)); }, setSegment(part, v: string | number) { - markValid(part); - setValue(setSegment(displayValue, part, v, resolvedOptions)); + let newValidSegments = markValid(part); + setValue(setSegment(displayValue, part, v, resolvedOptions), newValidSegments); }, confirmPlaceholder() { if (props.isDisabled || props.isReadOnly) { return; } - // Confirm the placeholder if only the day period is not filled in. let validKeys = Object.keys(validSegments); let allKeys = Object.keys(allSegments); if (validKeys.length === allKeys.length - 1 && allSegments.dayPeriod && !validSegments.dayPeriod) { - validSegments = {...allSegments}; - setValidSegments(validSegments); + setValidSegments({...allSegments}); setValue(displayValue.copy()); } }, clearSegment(part) { - delete validSegments[part]; + let newValidSegments = {...validSegments}; + delete newValidSegments[part]; clearedSegment.current = part; - setValidSegments({...validSegments}); + setValidSegments(newValidSegments); let placeholder = createPlaceholderDate(props.placeholderValue, granularity, calendar, defaultTimeZone); let value = displayValue; @@ -402,14 +394,14 @@ export function useDateFieldState(props: DateFi } else if (!isPM && shouldBePM) { value = displayValue.set({hour: displayValue.hour + 12}); } - } else if (part === 'hour' && 'hour' in displayValue && displayValue.hour >= 12 && validSegments.dayPeriod) { + } else if (part === 'hour' && 'hour' in displayValue && displayValue.hour >= 12 && newValidSegments.dayPeriod) { value = displayValue.set({hour: placeholder['hour'] + 12}); } else if (part in displayValue) { value = displayValue.set({[part]: placeholder[part]}); } setDate(null); - setValue(value); + setValue(value, newValidSegments); }, formatValue(fieldOptions: FieldOptions) { if (!calendarValue) { From a84fd4765e65d7fbff34eb7eb9d3de3a9138036e Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Fri, 17 Oct 2025 16:54:45 +1100 Subject: [PATCH 20/26] fix as much immutability and eslint-disables as possible --- eslint.config.mjs | 1 - .../collections/src/CollectionBuilder.tsx | 29 +++-- .../@react-aria/collections/src/Document.ts | 4 + .../@react-aria/form/src/useFormValidation.ts | 12 ++- .../interactions/src/PressResponder.tsx | 2 +- .../interactions/src/useFocusable.tsx | 2 +- .../@react-aria/interactions/src/usePress.ts | 2 +- .../spinbutton/src/useSpinButton.ts | 5 +- packages/@react-aria/ssr/src/SSRContext.tsx | 36 +++++++ packages/@react-aria/ssr/src/SSRProvider.tsx | 73 +------------ packages/@react-aria/ssr/src/useCounter.ts | 57 ++++++++++ packages/@react-aria/table/src/useTable.ts | 7 +- .../@react-aria/toolbar/src/useToolbar.ts | 6 +- packages/@react-aria/utils/src/useDeepMemo.ts | 2 - packages/@react-aria/utils/src/useDrag1D.ts | 2 - packages/@react-aria/utils/src/useSyncRef.ts | 15 +-- .../virtualizer/src/ScrollView.tsx | 75 +++++++------ .../visually-hidden/src/VisuallyHidden.tsx | 3 +- .../src/MobileSearchAutocomplete.tsx | 2 + .../buttongroup/src/ButtonGroup.tsx | 8 +- .../buttongroup/test/ButtonGroup.test.js | 6 +- .../@react-spectrum/card/src/CardView.tsx | 2 + .../@react-spectrum/checkbox/src/Checkbox.tsx | 101 ++++++++++++++---- .../combobox/src/MobileComboBox.tsx | 4 +- .../datepicker/test/DateRangePicker.test.js | 10 +- .../@react-spectrum/dialog/src/Dialog.tsx | 3 +- .../dialog/src/DialogTrigger.tsx | 5 +- packages/@react-spectrum/menu/src/Menu.tsx | 2 +- .../datepicker/src/useDateRangePickerState.ts | 1 + 29 files changed, 292 insertions(+), 185 deletions(-) create mode 100644 packages/@react-aria/ssr/src/SSRContext.tsx create mode 100644 packages/@react-aria/ssr/src/useCounter.ts diff --git a/eslint.config.mjs b/eslint.config.mjs index 9f010596984..243ac9a43fc 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -250,7 +250,6 @@ export default [{ "rsp-rules/sort-imports": [ERROR], "rulesdir/imports": [ERROR], "rulesdir/useLayoutEffectRule": [ERROR], - "rulesdir/pure-render": [ERROR], "jsx-a11y/accessible-emoji": ERROR, "jsx-a11y/alt-text": ERROR, "jsx-a11y/anchor-has-content": ERROR, diff --git a/packages/@react-aria/collections/src/CollectionBuilder.tsx b/packages/@react-aria/collections/src/CollectionBuilder.tsx index 7726c2c6168..7962e6a3672 100644 --- a/packages/@react-aria/collections/src/CollectionBuilder.tsx +++ b/packages/@react-aria/collections/src/CollectionBuilder.tsx @@ -47,9 +47,16 @@ export function CollectionBuilder>(props: Colle // Otherwise, render a hidden copy of the children so that we can build the collection before constructing the state. // This should always come before the real DOM content so we have built the collection by the time it renders during SSR. + return ( + + {props.children} + + ); +} - // This is fine. CollectionDocumentContext never changes after mounting. - // eslint-disable-next-line react-hooks/rules-of-hooks +function CollectionBuilderInner(props) { + // Otherwise, render a hidden copy of the children so that we can build the collection before constructing the state. + // This should always come before the real DOM content so we have built the collection by the time it renders during SSR. let {collection, document} = useCollectionDocument(props.createCollection); return ( <> @@ -80,7 +87,6 @@ function useSyncExternalStoreFallback(subscribe: (onStoreChange: () => void) // This is read immediately inside the wrapper, which also runs during render. // We just need a ref to avoid invalidating the callback itself, which // would cause React to re-run the callback more than necessary. - // eslint-disable-next-line rulesdir/pure-render isSSRRef.current = isSSR; let getSnapshotWrapper = useCallback(() => { @@ -109,7 +115,7 @@ function useCollectionDocument>(cr return collection; }, [document]); let getServerSnapshot = useCallback(() => { - document.isSSR = true; + document.setSSR(true); return document.getCollection(); }, [document]); let collection = useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot); @@ -132,8 +138,9 @@ function createCollectionNodeClass(type: string): CollectionNodeClass { function useSSRCollectionNode(CollectionNodeClass: CollectionNodeClass | string, props: object, ref: ForwardedRef, rendered?: any, children?: ReactNode, render?: (node: Node) => ReactElement) { // To prevent breaking change, if CollectionNodeClass is a string, create a CollectionNodeClass using the string as the type - if (typeof CollectionNodeClass === 'string') { - CollectionNodeClass = createCollectionNodeClass(CollectionNodeClass); + let CollectionNodeClassLocal = CollectionNodeClass; + if (typeof CollectionNodeClassLocal === 'string') { + CollectionNodeClassLocal = createCollectionNodeClass(CollectionNodeClassLocal); } // During SSR, portals are not supported, so the collection children will be wrapped in an SSRContext. @@ -142,15 +149,15 @@ function useSSRCollectionNode(CollectionNodeClass: Collection // collection by the time we need to use the collection to render to the real DOM. // After hydration, we switch to client rendering using the portal. let itemRef = useCallback((element: ElementNode | null) => { - element?.setProps(props, ref, CollectionNodeClass, rendered, render); - }, [props, ref, rendered, render, CollectionNodeClass]); + element?.setProps(props, ref, CollectionNodeClassLocal, rendered, render); + }, [props, ref, rendered, render, CollectionNodeClassLocal]); let parentNode = useContext(SSRContext); if (parentNode) { // Guard against double rendering in strict mode. let element = parentNode.ownerDocument.nodesByProps.get(props); if (!element) { - element = parentNode.ownerDocument.createElement(CollectionNodeClass.type); - element.setProps(props, ref, CollectionNodeClass, rendered, render); + element = parentNode.ownerDocument.createElement(CollectionNodeClassLocal.type); + element.setProps(props, ref, CollectionNodeClassLocal, rendered, render); parentNode.appendChild(element); parentNode.ownerDocument.updateCollection(); parentNode.ownerDocument.nodesByProps.set(props, element); @@ -162,7 +169,7 @@ function useSSRCollectionNode(CollectionNodeClass: Collection } // @ts-ignore - return {children}; + return {children}; } export function createLeafComponent(CollectionNodeClass: CollectionNodeClass | string, render: (props: P, ref: ForwardedRef) => ReactElement | null): (props: P & React.RefAttributes) => ReactElement | null; diff --git a/packages/@react-aria/collections/src/Document.ts b/packages/@react-aria/collections/src/Document.ts index dd1dbeb4f36..5b4925b3acf 100644 --- a/packages/@react-aria/collections/src/Document.ts +++ b/packages/@react-aria/collections/src/Document.ts @@ -431,6 +431,10 @@ export class Document = BaseCollection> extend return true; } + setSSR(value: boolean): void { + this.isSSR = value; + } + createElement(type: string): ElementNode { return new ElementNode(type, this); } diff --git a/packages/@react-aria/form/src/useFormValidation.ts b/packages/@react-aria/form/src/useFormValidation.ts index b4699bf8046..b5663f531e5 100644 --- a/packages/@react-aria/form/src/useFormValidation.ts +++ b/packages/@react-aria/form/src/useFormValidation.ts @@ -11,10 +11,10 @@ */ import {FormValidationState} from '@react-stately/form'; +import {getOwnerDocument, useEffectEvent, useLayoutEffect} from '@react-aria/utils'; import {RefObject, Validation, ValidationResult} from '@react-types/shared'; import {setInteractionModality} from '@react-aria/interactions'; import {useEffect, useRef} from 'react'; -import {useEffectEvent, useLayoutEffect} from '@react-aria/utils'; type ValidatableElement = HTMLInputElement | HTMLTextAreaElement | HTMLSelectElement; @@ -84,7 +84,15 @@ export function useFormValidation(props: FormValidationProps, state: FormV return; } - let form = input.form; + // Uses closest and querySelector instead of just the form property to work around a React compiler bug. + // https://github.com/facebook/react/issues/34891 + let form = input.closest('form'); + if (!form) { + let formId = input.getAttribute('form'); + if (formId) { + form = getOwnerDocument(input).querySelector(`#${formId}`) as HTMLFormElement | null; + } + } let reset = form?.reset; if (form) { diff --git a/packages/@react-aria/interactions/src/PressResponder.tsx b/packages/@react-aria/interactions/src/PressResponder.tsx index de0a7c78e66..6578bbd3d29 100644 --- a/packages/@react-aria/interactions/src/PressResponder.tsx +++ b/packages/@react-aria/interactions/src/PressResponder.tsx @@ -37,7 +37,7 @@ React.forwardRef(({children, ...props}: PressResponderProps, ref: ForwardedRef { if (!isRegistered.current) { diff --git a/packages/@react-aria/interactions/src/useFocusable.tsx b/packages/@react-aria/interactions/src/useFocusable.tsx index ac9f81251c6..74ef7c26e22 100644 --- a/packages/@react-aria/interactions/src/useFocusable.tsx +++ b/packages/@react-aria/interactions/src/useFocusable.tsx @@ -37,7 +37,7 @@ export let FocusableContext: React.Context = React function useFocusableContext(ref: RefObject): FocusableContextValue { let context = useContext(FocusableContext) || {}; - useSyncRef(context, ref); + useSyncRef(context.ref, ref); // eslint-disable-next-line let {ref: _, ...otherProps} = context; diff --git a/packages/@react-aria/interactions/src/usePress.ts b/packages/@react-aria/interactions/src/usePress.ts index 6dc4fd7f757..331bbc434e3 100644 --- a/packages/@react-aria/interactions/src/usePress.ts +++ b/packages/@react-aria/interactions/src/usePress.ts @@ -102,7 +102,7 @@ function usePressResponderContext(props: PressHookProps): PressHookProps { props = mergeProps(contextProps, props) as PressHookProps; register(); } - useSyncRef(context, props.ref); + useSyncRef(context.ref, props.ref); return props; } diff --git a/packages/@react-aria/spinbutton/src/useSpinButton.ts b/packages/@react-aria/spinbutton/src/useSpinButton.ts index 60bf944d33c..c1a4e9f5888 100644 --- a/packages/@react-aria/spinbutton/src/useSpinButton.ts +++ b/packages/@react-aria/spinbutton/src/useSpinButton.ts @@ -161,7 +161,8 @@ export function useSpinButton( _async.current = window.setTimeout( () => { if ((maxValue === undefined || isNaN(maxValue)) || (value === undefined || isNaN(value)) || value < maxValue) { - // TODO: false positive?? + // https://github.com/facebook/react/issues/34888 + // eslint-disable-next-line react-hooks/immutability onIncrementPressStart(60); } }, @@ -179,6 +180,8 @@ export function useSpinButton( _async.current = window.setTimeout( () => { if ((minValue === undefined || isNaN(minValue)) || (value === undefined || isNaN(value)) || value > minValue) { + // https://github.com/facebook/react/issues/34888 + // eslint-disable-next-line react-hooks/immutability onDecrementPressStart(60); } }, diff --git a/packages/@react-aria/ssr/src/SSRContext.tsx b/packages/@react-aria/ssr/src/SSRContext.tsx new file mode 100644 index 00000000000..3f67b680bfa --- /dev/null +++ b/packages/@react-aria/ssr/src/SSRContext.tsx @@ -0,0 +1,36 @@ +/* + * Copyright 2020 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ +import React from 'react'; + + +// To support SSR, the auto incrementing id counter is stored in a context. This allows +// it to be reset on every request to ensure the client and server are consistent. +// There is also a prefix string that is used to support async loading components +// Each async boundary must be wrapped in an SSR provider, which appends to the prefix +// and resets the current id counter. This ensures that async loaded components have +// consistent ids regardless of the loading order. +export interface SSRContextValue { + prefix: string, + current: number +} + +// Default context value to use in case there is no SSRProvider. This is fine for +// client-only apps. In order to support multiple copies of React Aria potentially +// being on the page at once, the prefix is set to a random number. SSRProvider +// will reset this to zero for consistency between server and client, so in the +// SSR case multiple copies of React Aria is not supported. +export const defaultContext: SSRContextValue = { + prefix: String(Math.round(Math.random() * 10000000000)), + current: 0 +}; + +export const SSRContext = React.createContext(defaultContext); diff --git a/packages/@react-aria/ssr/src/SSRProvider.tsx b/packages/@react-aria/ssr/src/SSRProvider.tsx index 49bf1b2f98f..b8f5606397e 100644 --- a/packages/@react-aria/ssr/src/SSRProvider.tsx +++ b/packages/@react-aria/ssr/src/SSRProvider.tsx @@ -10,33 +10,14 @@ * governing permissions and limitations under the License. */ +import {defaultContext, SSRContext, SSRContextValue} from './SSRContext'; // We must avoid a circular dependency with @react-aria/utils, and this useLayoutEffect is // guarded by a check that it only runs on the client side. // eslint-disable-next-line rulesdir/useLayoutEffectRule -import React, {JSX, ReactNode, useContext, useLayoutEffect, useMemo, useRef, useState} from 'react'; - -// To support SSR, the auto incrementing id counter is stored in a context. This allows -// it to be reset on every request to ensure the client and server are consistent. -// There is also a prefix string that is used to support async loading components -// Each async boundary must be wrapped in an SSR provider, which appends to the prefix -// and resets the current id counter. This ensures that async loaded components have -// consistent ids regardless of the loading order. -interface SSRContextValue { - prefix: string, - current: number -} +import React, {JSX, ReactNode, useContext, useLayoutEffect, useMemo, useState} from 'react'; +import {useCounter} from './useCounter'; + -// Default context value to use in case there is no SSRProvider. This is fine for -// client-only apps. In order to support multiple copies of React Aria potentially -// being on the page at once, the prefix is set to a random number. SSRProvider -// will reset this to zero for consistency between server and client, so in the -// SSR case multiple copies of React Aria is not supported. -const defaultContext: SSRContextValue = { - prefix: String(Math.round(Math.random() * 10000000000)), - current: 0 -}; - -const SSRContext = React.createContext(defaultContext); const IsSSRContext = React.createContext(false); export interface SSRProviderProps { @@ -99,50 +80,6 @@ let canUseDOM = Boolean( window.document.createElement ); -let componentIds = new WeakMap(); - -// TODO: this function needs serious help, haha -function useCounter(isDisabled = false) { - let ctx = useContext(SSRContext); - let ref = useRef(null); - // eslint-disable-next-line rulesdir/pure-render - if (ref.current === null && !isDisabled) { - // In strict mode, React renders components twice, and the ref will be reset to null on the second render. - // This means our id counter will be incremented twice instead of once. This is a problem because on the - // server, components are only rendered once and so ids generated on the server won't match the client. - // In React 18, useId was introduced to solve this, but it is not available in older versions. So to solve this - // we need to use some React internals to access the underlying Fiber instance, which is stable between renders. - // This is exposed as ReactCurrentOwner in development, which is all we need since StrictMode only runs in development. - // To ensure that we only increment the global counter once, we store the starting id for this component in - // a weak map associated with the Fiber. On the second render, we reset the global counter to this value. - // Since React runs the second render immediately after the first, this is safe. - // @ts-ignore - let currentOwner = React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED?.ReactCurrentOwner?.current; - if (currentOwner) { - let prevComponentValue = componentIds.get(currentOwner); - if (prevComponentValue == null) { - // On the first render, and first call to useId, store the id and state in our weak map. - componentIds.set(currentOwner, { - id: ctx.current, - state: currentOwner.memoizedState - }); - } else if (currentOwner.memoizedState !== prevComponentValue.state) { - // On the second render, the memoizedState gets reset by React. - // Reset the counter, and remove from the weak map so we don't - // do this for subsequent useId calls. - ctx.current = prevComponentValue.id; - componentIds.delete(currentOwner); - } - } - - // eslint-disable-next-line rulesdir/pure-render - ref.current = ++ctx.current; - } - - // eslint-disable-next-line rulesdir/pure-render - return ref.current; -} - function useLegacySSRSafeId(defaultId?: string): string { let ctx = useContext(SSRContext); @@ -188,7 +125,7 @@ function subscribe(onStoreChange: () => void): () => void { * until after hydration. */ export function useIsSSR(): boolean { - // In React 18, we can use useSyncExternalStore to detect if we're server rendering or hydrating. + // In React 18+, we can use useSyncExternalStore to detect if we're server rendering or hydrating. if (typeof React['useSyncExternalStore'] === 'function') { return React['useSyncExternalStore'](subscribe, getSnapshot, getServerSnapshot); } diff --git a/packages/@react-aria/ssr/src/useCounter.ts b/packages/@react-aria/ssr/src/useCounter.ts new file mode 100644 index 00000000000..39b33c6bcad --- /dev/null +++ b/packages/@react-aria/ssr/src/useCounter.ts @@ -0,0 +1,57 @@ +/* + * Copyright 2020 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ +import React, {useContext, useRef} from 'react'; +import {SSRContext} from './SSRContext'; + +// Moved this to a separate file to avoid the compiler bailing on the entire file, this is only used in React 16 and 17 + +let componentIds = new WeakMap(); + +export function useCounter(isDisabled = false) { + let ctx = useContext(SSRContext); + let ref = useRef(null); + if (ref.current === null && !isDisabled) { + // In strict mode, React renders components twice, and the ref will be reset to null on the second render. + // This means our id counter will be incremented twice instead of once. This is a problem because on the + // server, components are only rendered once and so ids generated on the server won't match the client. + // In React 18, useId was introduced to solve this, but it is not available in older versions. So to solve this + // we need to use some React internals to access the underlying Fiber instance, which is stable between renders. + // This is exposed as ReactCurrentOwner in development, which is all we need since StrictMode only runs in development. + // To ensure that we only increment the global counter once, we store the starting id for this component in + // a weak map associated with the Fiber. On the second render, we reset the global counter to this value. + // Since React runs the second render immediately after the first, this is safe. + // @ts-ignore + let currentOwner = React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED?.ReactCurrentOwner?.current; + if (currentOwner) { + let prevComponentValue = componentIds.get(currentOwner); + if (prevComponentValue == null) { + // On the first render, and first call to useId, store the id and state in our weak map. + componentIds.set(currentOwner, { + id: ctx.current, + state: currentOwner.memoizedState + }); + } else if (currentOwner.memoizedState !== prevComponentValue.state) { + // On the second render, the memoizedState gets reset by React. + // Reset the counter, and remove from the weak map so we don't + // do this for subsequent useId calls. + // eslint-disable-next-line react-hooks/immutability + ctx.current = prevComponentValue.id; + componentIds.delete(currentOwner); + } + } + + // eslint-disable-next-line react-hooks/immutability + ref.current = ++ctx.current; + } + + return ref.current; +} diff --git a/packages/@react-aria/table/src/useTable.ts b/packages/@react-aria/table/src/useTable.ts index 3ed31be82a7..af48c112432 100644 --- a/packages/@react-aria/table/src/useTable.ts +++ b/packages/@react-aria/table/src/useTable.ts @@ -78,12 +78,14 @@ export function useTable(props: AriaTableProps, state: TableState | TreeGr let id = useId(props.id); gridIds.set(state, id); - let {gridProps} = useGrid({ + let {gridProps: gridPropsBase} = useGrid({ ...props, id, keyboardDelegate: delegate }, state, ref); + let gridProps = {...gridPropsBase}; + // Override to include header rows if (isVirtualized) { gridProps['aria-rowcount'] = state.collection.size + state.collection.headerRows.length; @@ -98,8 +100,7 @@ export function useTable(props: AriaTableProps, state: TableState | TreeGr let sortDescription = useMemo(() => { let columnName = state.collection.columns.find(c => c.key === column)?.textValue ?? ''; return sortDirection && column ? stringFormatter.format(`${sortDirection}Sort`, {columnName}) : undefined; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [sortDirection, column, state.collection.columns]); + }, [sortDirection, column, state.collection.columns, stringFormatter]); let descriptionProps = useDescription(sortDescription); diff --git a/packages/@react-aria/toolbar/src/useToolbar.ts b/packages/@react-aria/toolbar/src/useToolbar.ts index b94bb988c57..a5f0af6a77d 100644 --- a/packages/@react-aria/toolbar/src/useToolbar.ts +++ b/packages/@react-aria/toolbar/src/useToolbar.ts @@ -44,12 +44,10 @@ export function useToolbar(props: AriaToolbarProps, ref: RefObject { setInToolbar(!!(ref.current && ref.current.parentElement?.closest('[role="toolbar"]'))); - }); + }, [ref]); const {direction} = useLocale(); const shouldReverse = direction === 'rtl' && orientation === 'horizontal'; let focusManager = createFocusManager(ref); diff --git a/packages/@react-aria/utils/src/useDeepMemo.ts b/packages/@react-aria/utils/src/useDeepMemo.ts index d0f7e144579..2542f2e7b76 100644 --- a/packages/@react-aria/utils/src/useDeepMemo.ts +++ b/packages/@react-aria/utils/src/useDeepMemo.ts @@ -10,8 +10,6 @@ * governing permissions and limitations under the License. */ -/* eslint-disable rulesdir/pure-render */ - import {useRef} from 'react'; export function useDeepMemo(value: T, isEqual: (a: T, b: T) => boolean): T { diff --git a/packages/@react-aria/utils/src/useDrag1D.ts b/packages/@react-aria/utils/src/useDrag1D.ts index e907128c9b3..618cdf0c61e 100644 --- a/packages/@react-aria/utils/src/useDrag1D.ts +++ b/packages/@react-aria/utils/src/useDrag1D.ts @@ -10,8 +10,6 @@ * governing permissions and limitations under the License. */ - /* eslint-disable rulesdir/pure-render */ - import {getOffset} from './getOffset'; import {Orientation} from '@react-types/shared'; import React, {HTMLAttributes, MutableRefObject, useRef} from 'react'; diff --git a/packages/@react-aria/utils/src/useSyncRef.ts b/packages/@react-aria/utils/src/useSyncRef.ts index 5bbc2389325..38866573e0f 100644 --- a/packages/@react-aria/utils/src/useSyncRef.ts +++ b/packages/@react-aria/utils/src/useSyncRef.ts @@ -10,22 +10,17 @@ * governing permissions and limitations under the License. */ -import {MutableRefObject} from 'react'; import {RefObject} from '@react-types/shared'; import {useLayoutEffect} from './'; -interface ContextValue { - ref?: MutableRefObject -} - // Syncs ref from context with ref passed to hook -export function useSyncRef(context?: ContextValue | null, ref?: RefObject): void { +export function useSyncRef(contextRef?: RefObject | null, ref?: RefObject): void { useLayoutEffect(() => { - if (context && context.ref && ref) { - context.ref.current = ref.current; + if (contextRef && ref) { + contextRef.current = ref.current; return () => { - if (context.ref) { - context.ref.current = null; + if (contextRef) { + contextRef.current = null; } }; } diff --git a/packages/@react-aria/virtualizer/src/ScrollView.tsx b/packages/@react-aria/virtualizer/src/ScrollView.tsx index b57efc4f19e..75066785c32 100644 --- a/packages/@react-aria/virtualizer/src/ScrollView.tsx +++ b/packages/@react-aria/virtualizer/src/ScrollView.tsx @@ -73,7 +73,7 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject { let scrollTop = e.currentTarget.scrollTop; let scrollLeft = getScrollLeft(e.currentTarget, direction); + let state = stateRef.current; // Prevent rubber band scrolling from shaking when scrolling out of bounds state.scrollTop = Math.max(0, Math.min(scrollTop, contentSize.height - state.height)); @@ -138,12 +139,13 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject { + let state = stateRef.current; return () => { if (state.scrollTimeout != null) { clearTimeout(state.scrollTimeout); @@ -153,8 +155,7 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject { @@ -175,6 +176,7 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject(null); + }, [ref, stateRef, onVisibleRectChange]); let [update, setUpdate] = useState({}); - // We only contain a call to setState in here for testing environments. - // eslint-disable-next-line react-hooks/exhaustive-deps - useLayoutEffect(() => { - if (!isUpdatingSize.current && (lastContentSize.current == null || !contentSize.equals(lastContentSize.current))) { - // React doesn't allow flushSync inside effects, so queue a microtask. - // We also need to wait until all refs are set (e.g. when passing a ref down from a parent). - // If we are in an `act` environment, update immediately without a microtask so you don't need - // to mock timers in tests. In this case, the update is synchronous already. - // IS_REACT_ACT_ENVIRONMENT is used by React 18. Previous versions checked for the `jest` global. - // https://github.com/reactwg/react-18/discussions/102 - // @ts-ignore - if (typeof IS_REACT_ACT_ENVIRONMENT === 'boolean' ? IS_REACT_ACT_ENVIRONMENT : typeof jest !== 'undefined') { - // This is so we update size in a separate render but within the same act. Needs to be setState instead of refs - // due to strict mode. - setUpdate({}); - lastContentSize.current = contentSize; - return; - } else { - queueMicrotask(() => updateSizeEvent(flushSync)); - } - } + let updateSizeEvent = useEffectEvent(updateSize); - lastContentSize.current = contentSize; - }); + useScrollViewContentSizeChange({updateSize, contentSize, isUpdatingSize, setUpdate}); // Will only run in tests, needs to be in separate effect so it is properly run in the next render in strict mode. useLayoutEffect(() => { @@ -250,7 +227,7 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject
; +}); function TableInner({props, forwardedRef: ref, selectionState, collection}: TableInnerProps) { [props, ref] = useContextProps(props, ref, SelectableCollectionContext); @@ -496,7 +503,6 @@ function TableInner({props, forwardedRef: ref, selectionState, collection}: Tabl } } - let ElementType = useElementType('table'); let DOMProps = filterDOMProps(props, {global: true}); return ( @@ -510,7 +516,7 @@ function TableInner({props, forwardedRef: ref, selectionState, collection}: Tabl [FieldInputContext, null] ]}> - } @@ -526,18 +532,13 @@ function TableInner({props, forwardedRef: ref, selectionState, collection}: Tabl scrollRef={tableContainerContext?.scrollRef ?? ref} persistedKeys={useDndPersistedKeys(selectionManager, dragAndDropHooks, dropState)} /> - + {dragPreview} ); } -function useElementType(element: E): E | 'div' { - let {isVirtualized} = useContext(CollectionRendererContext); - return isVirtualized ? 'div' : element; -} - export interface TableOptionsContextValue { /** The type of selection that is allowed in the table. */ selectionMode: SelectionMode, @@ -744,6 +745,14 @@ class TableColumnNode extends CollectionNode { static readonly type = 'column'; } +let ColumnElementType = forwardRef(function ColumnElementType(props: any, ref: ForwardedRef) { + let {isVirtualized} = useContext(CollectionRendererContext); + if (isVirtualized) { + return
; + } + return
; +}); + /** * A column within a ``. */ @@ -804,12 +813,11 @@ export const Column = /*#__PURE__*/ createLeafComponent(TableColumnNode, (props: style = {...style, width: layoutState.getColumnWidth(column.key)}; } - let TH = useElementType('th'); let DOMProps = filterDOMProps(props as any, {global: true}); delete DOMProps.id; return ( - + ); }); @@ -994,6 +1002,14 @@ class TableBodyNode extends FilterableNode { static readonly type = 'tablebody'; } +let TableBodyElementType = forwardRef(function TableBodyElementType(props: any, ref: ForwardedRef) { + let {isVirtualized} = useContext(CollectionRendererContext); + if (isVirtualized) { + return
; + } + return
; +}); + /** * The body of a `
{renderProps.children} -
`, containing the table rows. */ @@ -1020,8 +1036,6 @@ export const TableBody = /*#__PURE__*/ createBranchComponent(TableBodyNode, - - + + ); } let {rowGroupProps} = useTableRowGroup(); - let TBody = useElementType('tbody'); let DOMProps = filterDOMProps(props, {global: true}); // TODO: TableBody doesn't support being the scrollable body of the table yet, to revisit if needed. Would need to // call useLoadMore here and walk up the DOM to the nearest scrollable element to set scrollRef return ( - @@ -1062,7 +1075,7 @@ export const TableBody = /*#__PURE__*/ createBranchComponent(TableBodyNode, {emptyState} - + ); }); @@ -1117,6 +1130,14 @@ class TableRowNode extends CollectionNode { } } +let TableRowElementType = forwardRef(function TableRowElementType(props: any, ref: ForwardedRef) { + let {isVirtualized} = useContext(CollectionRendererContext); + if (isVirtualized) { + return
; + } + return
; +}); + /** * A row within a `
+ + {props.renderEmptyState(renderValues)} -
`. */ @@ -1196,8 +1217,6 @@ export const Row = /*#__PURE__*/ createBranchComponent( } }); - let TR = useElementType('tr'); - let TD = useElementType('td'); let DOMProps = filterDOMProps(props as any, {global: true}); delete DOMProps.id; delete DOMProps.onClick; @@ -1205,13 +1224,13 @@ export const Row = /*#__PURE__*/ createBranchComponent( return ( <> {dropIndicator && !dropIndicator.isHidden && ( - - - + + )} - - + ); }, @@ -1311,6 +1330,14 @@ class TableCellNode extends CollectionNode { static readonly type = 'cell'; } +let TableCellElementType = forwardRef(function TableCellElementType(props: any, ref: ForwardedRef) { + let {isVirtualized} = useContext(CollectionRendererContext); + if (isVirtualized) { + return
; + } + return
+ ); }); @@ -1449,25 +1474,23 @@ function RootDropIndicator() { }, dropState!, ref); let isDropTarget = dropState!.isDropTarget({type: 'root'}); let {visuallyHiddenProps} = useVisuallyHidden(); - let TR = useElementType('tr'); - let TD = useElementType('td'); if (!isDropTarget && dropIndicatorProps['aria-hidden']) { return null; } return ( - - - + + ); } @@ -1509,8 +1532,6 @@ export const TableLoadMoreItem = createLeafComponent(LoaderNode, function TableL defaultClassName: 'react-aria-TableLoadingIndicator', values: null }); - let TR = useElementType('tr'); - let TD = useElementType('td'); let rowProps = {}; let rowHeaderProps = {}; let style = {}; @@ -1529,21 +1550,21 @@ export const TableLoadMoreItem = createLeafComponent(LoaderNode, function TableL <> {/* Alway render the sentinel. For now onus is on the user for styling when using flex + gap (this would introduce a gap even though it doesn't take room) */} {/* @ts-ignore - compatibility with React < 19 */} - - - + + {isLoading && renderProps.children && ( - }> - - + + )} ); From fe3d7b55c4f9ea22b2922e5a5210eef2fa909bb3 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Thu, 23 Oct 2025 15:29:13 +1100 Subject: [PATCH 24/26] fix read from ref in render --- packages/@react-spectrum/menu/src/Menu.tsx | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/@react-spectrum/menu/src/Menu.tsx b/packages/@react-spectrum/menu/src/Menu.tsx index 97e5de589e1..700f45ff317 100644 --- a/packages/@react-spectrum/menu/src/Menu.tsx +++ b/packages/@react-spectrum/menu/src/Menu.tsx @@ -54,12 +54,15 @@ export const Menu = React.forwardRef(function Menu(props: Spec let {styleProps} = useStyleProps(completeProps); useSyncRef(contextProps, domRef); let [leftOffset, setLeftOffset] = useState({left: 0}); - let [prevPopoverContainer, setPrevPopoverContainer] = useState(null); - if (popoverContainer && prevPopoverContainer !== popoverContainer && leftOffset.left === 0) { - let {left} = popoverContainer.getBoundingClientRect(); - setLeftOffset({left: -1 * left}); - setPrevPopoverContainer(popoverContainer); - } + let prevPopoverContainer = useRef(null); + + useLayoutEffect(() => { + if (popoverContainer && prevPopoverContainer.current !== popoverContainer && leftOffset.left === 0) { + prevPopoverContainer.current = popoverContainer; + let {left} = popoverContainer.getBoundingClientRect(); + setLeftOffset({left: -1 * left}); + } + }, [leftOffset]); let menuLevel = contextProps.submenuLevel ?? -1; let nextMenuLevelKey = rootMenuTriggerState?.expandedKeysStack[menuLevel + 1]; From 094f8ea778215328125ef969d88988c5b777da52 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Mon, 27 Oct 2025 12:59:35 +1100 Subject: [PATCH 25/26] fix docs --- packages/@react-spectrum/checkbox/src/Checkbox.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/@react-spectrum/checkbox/src/Checkbox.tsx b/packages/@react-spectrum/checkbox/src/Checkbox.tsx index 3aabbf18554..0a673d32fbb 100644 --- a/packages/@react-spectrum/checkbox/src/Checkbox.tsx +++ b/packages/@react-spectrum/checkbox/src/Checkbox.tsx @@ -32,7 +32,10 @@ import {useToggleState} from '@react-stately/toggle'; */ export const Checkbox = forwardRef(function Checkbox(props: SpectrumCheckboxProps, ref: FocusableRef) { let groupState = useContext(CheckboxGroupContext); - return groupState ? : ; + if (groupState) { + return ; + } + return ; }); let CheckboxInGroup = forwardRef(function CheckboxInGroup(props: SpectrumCheckboxProps, ref: FocusableRef) { From c42485098cbf2fac8a1a20940a8f3ae6022f4c25 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Fri, 7 Nov 2025 09:13:08 +1100 Subject: [PATCH 26/26] revert scrollview --- .../virtualizer/src/ScrollView.tsx | 75 +++++++++---------- 1 file changed, 34 insertions(+), 41 deletions(-) diff --git a/packages/@react-aria/virtualizer/src/ScrollView.tsx b/packages/@react-aria/virtualizer/src/ScrollView.tsx index 75066785c32..b57efc4f19e 100644 --- a/packages/@react-aria/virtualizer/src/ScrollView.tsx +++ b/packages/@react-aria/virtualizer/src/ScrollView.tsx @@ -73,7 +73,7 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject { let scrollTop = e.currentTarget.scrollTop; let scrollLeft = getScrollLeft(e.currentTarget, direction); - let state = stateRef.current; // Prevent rubber band scrolling from shaking when scrolling out of bounds state.scrollTop = Math.max(0, Math.min(scrollTop, contentSize.height - state.height)); @@ -139,13 +138,12 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject { - let state = stateRef.current; return () => { if (state.scrollTimeout != null) { clearTimeout(state.scrollTimeout); @@ -155,7 +153,8 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject { @@ -176,7 +175,6 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject(null); + let [update, setUpdate] = useState({}); + // We only contain a call to setState in here for testing environments. + // eslint-disable-next-line react-hooks/exhaustive-deps + useLayoutEffect(() => { + if (!isUpdatingSize.current && (lastContentSize.current == null || !contentSize.equals(lastContentSize.current))) { + // React doesn't allow flushSync inside effects, so queue a microtask. + // We also need to wait until all refs are set (e.g. when passing a ref down from a parent). + // If we are in an `act` environment, update immediately without a microtask so you don't need + // to mock timers in tests. In this case, the update is synchronous already. + // IS_REACT_ACT_ENVIRONMENT is used by React 18. Previous versions checked for the `jest` global. + // https://github.com/reactwg/react-18/discussions/102 + // @ts-ignore + if (typeof IS_REACT_ACT_ENVIRONMENT === 'boolean' ? IS_REACT_ACT_ENVIRONMENT : typeof jest !== 'undefined') { + // This is so we update size in a separate render but within the same act. Needs to be setState instead of refs + // due to strict mode. + setUpdate({}); + lastContentSize.current = contentSize; + return; + } else { + queueMicrotask(() => updateSizeEvent(flushSync)); + } + } + + lastContentSize.current = contentSize; + }); // Will only run in tests, needs to be in separate effect so it is properly run in the next render in strict mode. useLayoutEffect(() => { @@ -227,7 +250,7 @@ export function useScrollView(props: ScrollViewProps, ref: RefObject
+ +
-
; +}); + /** * A cell within a table row. */ @@ -1343,13 +1370,11 @@ export const Cell = /*#__PURE__*/ createLeafComponent(TableCellNode, (props: Cel } }); - // TODO: Lint doesn't catch these, it thinks we're not in a component render cycle here? - let TD = useElementType('td'); let DOMProps = filterDOMProps(props as any, {global: true}); delete DOMProps.id; return ( - {renderProps.children} -
-
+ +
-
+ {renderProps.children} -