diff --git a/eslint.config.mjs b/eslint.config.mjs index c1ac2344dd9..ce69f768024 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -234,14 +234,14 @@ export default [{ 'react-hooks/error-boundaries': ERROR, 'react-hooks/component-hook-factories': ERROR, 'react-hooks/gating': ERROR, - // 'react-hooks/globals': ERROR, + 'react-hooks/globals': ERROR, // 'react-hooks/immutability': ERROR, - // 'react-hooks/preserve-manual-memoization': ERROR, - // 'react-hooks/purity': ERROR, - // 'react-hooks/refs': ERROR, - // 'react-hooks/set-state-in-effect': ERROR, + // 'react-hooks/preserve-manual-memoization': ERROR, // No idea how to turn this one on yet + 'react-hooks/purity': ERROR, + // 'react-hooks/refs': ERROR, // can't turn on until https://github.com/facebook/react/issues/34775 is fixed + 'react-hooks/set-state-in-effect': ERROR, 'react-hooks/set-state-in-render': ERROR, - // 'react-hooks/static-components': ERROR, + 'react-hooks/static-components': ERROR, 'react-hooks/unsupported-syntax': WARN, 'react-hooks/use-memo': ERROR, 'react-hooks/incompatible-library': WARN, 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-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/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-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/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 { - 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/color/src/ColorWheel.tsx b/packages/@react-spectrum/color/src/ColorWheel.tsx index b187a4a1f6b..9c92627e5c7 100644 --- a/packages/@react-spectrum/color/src/ColorWheel.tsx +++ b/packages/@react-spectrum/color/src/ColorWheel.tsx @@ -14,14 +14,14 @@ import {classNames, dimensionValue, useFocusableRef, useStyleProps} from '@react import {ColorThumb} from './ColorThumb'; import {ColorWheelContext, useContextProps} from 'react-aria-components'; import {FocusableRef} from '@react-types/shared'; -import React, {useCallback, useEffect, useRef, useState} from 'react'; +import React, {useCallback, useRef, useState} from 'react'; import {SpectrumColorWheelProps} from '@react-types/color'; import styles from '@adobe/spectrum-css-temp/components/colorwheel/vars.css'; import {useColorWheel} from '@react-aria/color'; import {useColorWheelState} from '@react-stately/color'; import {useFocusRing} from '@react-aria/focus'; +import {useLayoutEffect, useResizeObserver} from '@react-aria/utils'; import {useProviderProps} from '@react-spectrum/provider'; -import {useResizeObserver} from '@react-aria/utils'; const WHEEL_THICKNESS = 24; @@ -53,7 +53,7 @@ export const ColorWheel = React.forwardRef(function ColorWheel(props: SpectrumCo } }, [containerRef, setWheelRadius, setWheelThickness]); - useEffect(() => { + useLayoutEffect(() => { // the size observer's fallback to the window resize event doesn't fire on mount if (wheelRadius === 0) { resizeHandler(); 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/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/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/menu/src/Menu.tsx b/packages/@react-spectrum/menu/src/Menu.tsx index b14b256a51d..9fbb712d51b 100644 --- a/packages/@react-spectrum/menu/src/Menu.tsx +++ b/packages/@react-spectrum/menu/src/Menu.tsx @@ -55,7 +55,8 @@ export const Menu = React.forwardRef(function Menu(props: Spec useSyncRef(contextProps, domRef); let [leftOffset, setLeftOffset] = useState({left: 0}); let prevPopoverContainer = useRef(null); - useEffect(() => { + + useLayoutEffect(() => { if (popoverContainer && prevPopoverContainer.current !== popoverContainer && leftOffset.left === 0) { prevPopoverContainer.current = popoverContainer; let {left} = popoverContainer.getBoundingClientRect(); 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-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-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/Table.tsx b/packages/react-aria-components/src/Table.tsx index 32a9ad4b845..090d188c849 100644 --- a/packages/react-aria-components/src/Table.tsx +++ b/packages/react-aria-components/src/Table.tsx @@ -384,6 +384,13 @@ interface TableInnerProps { collection: ITableCollection> } +let TableElementType = forwardRef(function TableElementType(props: any, ref: ForwardedRef) { + let {isVirtualized} = useContext(CollectionRendererContext); + if (isVirtualized) { + return
; + } + return ; +}); 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, @@ -584,6 +585,14 @@ class TableHeaderNode extends CollectionNode { static readonly type = 'tableheader'; } +let THeadElementType = forwardRef(function THeadElementType(props: any, ref: ForwardedRef) { + let {isVirtualized} = useContext(CollectionRendererContext); + if (isVirtualized) { + return
; + } + return
; +}); + /** * A header within a `
`, containing the table columns. */ @@ -603,7 +612,6 @@ export const TableHeader = /*#__PURE__*/ createBranchComponent( }, []) }); - let THead = useElementType('thead'); let {rowGroupProps} = useTableRowGroup(); let {hoverProps, isHovered} = useHover({ onHoverStart: props.onHoverStart, @@ -621,13 +629,13 @@ export const TableHeader = /*#__PURE__*/ createBranchComponent( }); return ( - {headerRows} - + ); }, props => ( @@ -637,16 +645,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 ( - + }) { ]}> - + ); } @@ -735,6 +750,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 ``. */ @@ -796,12 +819,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 ( - + ); }); @@ -987,6 +1009,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. */ @@ -1013,8 +1043,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 ( - @@ -1055,7 +1082,7 @@ export const TableBody = /*#__PURE__*/ createBranchComponent(TableBodyNode, {emptyState} - + ); }); @@ -1110,6 +1137,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)} -
`. */ @@ -1189,8 +1224,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; @@ -1198,13 +1231,13 @@ export const Row = /*#__PURE__*/ createBranchComponent( return ( <> {dropIndicator && !dropIndicator.isHidden && ( - - - + + )} - - + ); }, @@ -1304,6 +1337,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
+ ); }); @@ -1379,6 +1419,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
- + + ); } @@ -1429,25 +1481,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 ( - - - + + ); } @@ -1489,8 +1539,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 = {}; @@ -1509,21 +1557,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 && ( - }> - - + + )} ); 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/stories/ListBox.stories.tsx b/packages/react-aria-components/stories/ListBox.stories.tsx index 74a049009ac..583ef71656b 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 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()); 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);
+ +
-
; +}); + /** * A cell within a table row. */ @@ -1336,12 +1377,11 @@ export const Cell = /*#__PURE__*/ createLeafComponent(TableCellNode, (props: Cel } }); - let TD = useElementType('td'); let DOMProps = filterDOMProps(props as any, {global: true}); delete DOMProps.id; return ( - {renderProps.children} -
; +}); + function TableDropIndicator(props: TableDropIndicatorProps, ref: ForwardedRef) { let { dropIndicatorProps, @@ -1397,24 +1452,21 @@ function TableDropIndicator(props: TableDropIndicatorProps, ref: ForwardedRef} data-drop-target={isDropTarget || undefined}> -
{renderProps.children} -
-
+ +
-
+ {renderProps.children} -