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 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/checkbox/src/useCheckboxGroupState.ts b/packages/@react-stately/checkbox/src/useCheckboxGroupState.ts index b1f1d2db9fa..3a06fdd83b4 100644 --- a/packages/@react-stately/checkbox/src/useCheckboxGroupState.ts +++ b/packages/@react-stately/checkbox/src/useCheckboxGroupState.ts @@ -98,10 +98,12 @@ export function useCheckboxGroupState(props: CheckboxGroupProps = {}): CheckboxG if (props.isReadOnly || props.isDisabled) { return; } - if (!selectedValues.includes(value)) { - selectedValues = selectedValues.concat(value); - setValue(selectedValues); - } + 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..0e52b499b6b 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] { +// 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]; +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; - } - }; + useEarlyEffect(() => { + 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); }); });