diff --git a/src/components/controls/TextArea/TextArea.tsx b/src/components/controls/TextArea/TextArea.tsx index b94ec7635f..e54dbfeab2 100644 --- a/src/components/controls/TextArea/TextArea.tsx +++ b/src/components/controls/TextArea/TextArea.tsx @@ -1,6 +1,6 @@ import React from 'react'; -import {useForkRef, useUniqId} from '../../../hooks'; +import {useControlledState, useForkRef, useUniqId} from '../../../hooks'; import {blockNew} from '../../utils/cn'; import {ClearButton, mapTextInputSizeToButtonSize} from '../common'; import {OuterAdditionalContent} from '../common/OuterAdditionalContent/OuterAdditionalContent'; @@ -69,15 +69,13 @@ export const TextArea = React.forwardRef(functio validationState: validationStateProp, }); - const [uncontrolledValue, setUncontrolledValue] = React.useState(defaultValue ?? ''); + const [inputValue, setInputValue] = useControlledState(value, defaultValue ?? '', onUpdate); const innerControlRef = React.useRef(null); const [hasVerticalScrollbar, setHasVerticalScrollbar] = React.useState(false); const state = getInputControlState(validationState); const handleRef = useForkRef(props.controlRef, innerControlRef); const innerId = useUniqId(); - const isControlled = value !== undefined; - const inputValue = isControlled ? value : uncontrolledValue; const isErrorMsgVisible = validationState === 'invalid' && Boolean(errorMessage); const isClearControlVisible = Boolean(hasClear && !disabled && inputValue); const id = originalId || innerId; @@ -97,16 +95,10 @@ export const TextArea = React.forwardRef(functio tabIndex, name, onChange(event: React.ChangeEvent) { - const newValue = event.target.value; - if (!isControlled) { - setUncontrolledValue(newValue); - } + setInputValue(event.target.value); if (onChange) { onChange(event); } - if (onUpdate) { - onUpdate(newValue); - } }, autoComplete: prepareAutoComplete(autoComplete), controlProps: { @@ -131,15 +123,9 @@ export const TextArea = React.forwardRef(functio if (onChange) { onChange(syntheticEvent); } - - if (onUpdate) { - onUpdate(''); - } } - if (!isControlled) { - setUncontrolledValue(''); - } + setInputValue(''); }; React.useEffect(() => { diff --git a/src/components/controls/TextInput/TextInput.tsx b/src/components/controls/TextInput/TextInput.tsx index 543df3fb12..e94f38b151 100644 --- a/src/components/controls/TextInput/TextInput.tsx +++ b/src/components/controls/TextInput/TextInput.tsx @@ -2,7 +2,7 @@ import React from 'react'; import {TriangleExclamation} from '@gravity-ui/icons'; -import {useForkRef, useUniqId} from '../../../hooks'; +import {useControlledState, useForkRef, useUniqId} from '../../../hooks'; import {useElementSize} from '../../../hooks/private'; import {Icon} from '../../Icon'; import {Popover} from '../../Popover'; @@ -97,15 +97,13 @@ export const TextInput = React.forwardRef(funct validationState: validationStateProp, }); - const [uncontrolledValue, setUncontrolledValue] = React.useState(defaultValue ?? ''); + const [inputValue, setInputValue] = useControlledState(value, defaultValue ?? '', onUpdate); const innerControlRef = React.useRef(null); const handleRef = useForkRef(props.controlRef, innerControlRef); const labelRef = React.useRef(null); const startContentRef = React.useRef(null); const state = getInputControlState(validationState); - const isControlled = value !== undefined; - const inputValue = isControlled ? value : uncontrolledValue; const isLabelVisible = Boolean(label); const isErrorMsgVisible = validationState === 'invalid' && Boolean(errorMessage) && errorPlacement === 'outside'; @@ -149,24 +147,20 @@ export const TextInput = React.forwardRef(funct tabIndex, name, onChange(event: React.ChangeEvent) { - const newValue = event.target.value; - if (!isControlled) { - setUncontrolledValue(newValue); - } + setInputValue(event.target.value); + if (onChange) { onChange(event); } - if (onUpdate) { - onUpdate(newValue); - } }, autoComplete: isAutoCompleteOff ? 'off' : prepareAutoComplete(autoComplete), controlProps, }; const handleClear = (event: React.MouseEvent) => { - const control = innerControlRef.current; + setInputValue(''); + const control = innerControlRef.current; if (control) { control.focus(); @@ -179,14 +173,6 @@ export const TextInput = React.forwardRef(funct if (onChange) { onChange(syntheticEvent); } - - if (onUpdate) { - onUpdate(''); - } - } - - if (!isControlled) { - setUncontrolledValue(''); } }; diff --git a/src/hooks/index.ts b/src/hooks/index.ts index 8c733a9c24..9a98690749 100644 --- a/src/hooks/index.ts +++ b/src/hooks/index.ts @@ -1,6 +1,7 @@ export * from './useActionHandlers'; export * from './useAsyncActionHandler'; export * from './useBodyScrollLock'; +export * from './useControlledState'; export * from './useFileInput'; export * from './useFocusWithin'; export * from './useForkRef'; diff --git a/src/hooks/private/useCheckbox/useCheckbox.ts b/src/hooks/private/useCheckbox/useCheckbox.ts index 78471c2af5..62c777d147 100644 --- a/src/hooks/private/useCheckbox/useCheckbox.ts +++ b/src/hooks/private/useCheckbox/useCheckbox.ts @@ -1,6 +1,6 @@ import React from 'react'; -import {useForkRef} from '../..'; +import {useControlledState, useForkRef} from '../..'; import type {ControlProps} from '../../../components/types'; import {eventBroker} from '../../../components/utils/event-broker'; @@ -27,9 +27,12 @@ export function useCheckbox({ disabled, }: UseCheckboxProps): UseCheckboxResult { const innerControlRef = React.useRef(null); - const [checkedState, setCheckedState] = React.useState(defaultChecked ?? false); - const isControlled = typeof checked === 'boolean'; - const isChecked = isControlled ? checked : checkedState; + const [isChecked, setCheckedState] = useControlledState( + checked, + defaultChecked ?? false, + onUpdate, + ); + const inputChecked = indeterminate ? false : checked; const inputAriaChecked = indeterminate ? 'mixed' : isChecked; @@ -42,17 +45,11 @@ export function useCheckbox({ }, [indeterminate]); const handleChange = (event: React.ChangeEvent) => { - if (!isControlled) { - setCheckedState(event.target.checked); - } + setCheckedState(event.target.checked); if (onChange) { onChange(event); } - - if (onUpdate) { - onUpdate(event.target.checked); - } }; const handleClickCapture = React.useCallback( diff --git a/src/hooks/useControlledState/README.md b/src/hooks/useControlledState/README.md new file mode 100644 index 0000000000..c9cfcd9278 --- /dev/null +++ b/src/hooks/useControlledState/README.md @@ -0,0 +1,26 @@ + + +# useControlledState + + + +```tsx +import {useControlledState} from '@gravity-ui/uikit'; +``` + +The `useControlledState` hook that simplify work with controlled/uncontrolled state. + +## Properties + +| Name | Description | Type | Default | +| :----------- | :---------------------------------------- | :---------------: | :-----: | +| value | if `undefined` then value is uncontrolled | `T \| undefined` | | +| defaultValue | Initial value if value is uncontrolled | `T \| undefined` | | +| onUpdate | Callback on value change | `(v: T) => void` | | + +## Result + +`useControlledState` returns an array with exactly two values: + +1. The current state. +2. The set function that lets you update the state to a different value. diff --git a/src/hooks/useControlledState/index.ts b/src/hooks/useControlledState/index.ts new file mode 100644 index 0000000000..3a9d887d89 --- /dev/null +++ b/src/hooks/useControlledState/index.ts @@ -0,0 +1 @@ +export * from './useControlledState'; diff --git a/src/hooks/useControlledState/useControlledState.test.tsx b/src/hooks/useControlledState/useControlledState.test.tsx new file mode 100644 index 0000000000..7500a21942 --- /dev/null +++ b/src/hooks/useControlledState/useControlledState.test.tsx @@ -0,0 +1,305 @@ +import React from 'react'; + +import {act, render, renderHook, screen} from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import {useControlledState} from './useControlledState'; + +describe('useControlledState tests', function () { + it('can handle default setValue behavior, wont invoke onChange for the same value twice in a row', () => { + const onChangeSpy = jest.fn(); + const {result} = renderHook(() => + useControlledState(undefined, 'defaultValue', onChangeSpy), + ); + let [value, setValue] = result.current; + expect(value).toBe('defaultValue'); + expect(onChangeSpy).not.toHaveBeenCalled(); + act(() => setValue('newValue')); + [value, setValue] = result.current; + expect(value).toBe('newValue'); + expect(onChangeSpy).toHaveBeenLastCalledWith('newValue'); + + act(() => setValue('newValue2')); + [value, setValue] = result.current; + expect(value).toBe('newValue2'); + expect(onChangeSpy).toHaveBeenLastCalledWith('newValue2'); + + onChangeSpy.mockClear(); + + act(() => setValue('newValue2')); + [value, setValue] = result.current; + expect(value).toBe('newValue2'); + expect(onChangeSpy).not.toHaveBeenCalled(); + + // it should call onChange with a new but not immediately previously run value + act(() => setValue('newValue')); + [value, setValue] = result.current; + expect(value).toBe('newValue'); + expect(onChangeSpy).toHaveBeenLastCalledWith('newValue'); + }); + + it('using NaN will only trigger onChange once', () => { + const onChangeSpy = jest.fn(); + const {result} = renderHook(() => + useControlledState(undefined, null, onChangeSpy), + ); + let [value, setValue] = result.current; + expect(value).toBeNull(); + expect(onChangeSpy).not.toHaveBeenCalled(); + act(() => setValue(NaN)); + [value, setValue] = result.current; + expect(value).toBe(NaN); + expect(onChangeSpy).toHaveBeenCalledTimes(1); + expect(onChangeSpy).toHaveBeenLastCalledWith(NaN); + + act(() => setValue(NaN)); + [value, setValue] = result.current; + expect(value).toBe(NaN); + expect(onChangeSpy).toHaveBeenCalledTimes(1); + }); + + it('does not trigger too many renders', async () => { + const renderSpy = jest.fn(); + + const TestComponent = (props: any) => { + const [state, setState] = useControlledState( + props.value, + props.defaultValue, + props.onChange, + ); + React.useEffect(() => renderSpy(), [state]); + return + ); + } + + function TransitionButton({onClick}: any) { + const [isPending, startTransition] = React.useTransition(); + return ( + + ); + } + + const onChange = jest.fn(); + render(); + const value = screen.getByTestId('value'); + const show = screen.getByTestId('show'); + + expect(value).toHaveTextContent('1'); + await userEvent.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 userEvent.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 userEvent.click(value); + expect(value).toHaveTextContent('3'); + expect(onChange).toHaveBeenCalledTimes(2); + expect(onChange).toHaveBeenLastCalledWith(3); + }); + + it('should work with suspense when uncontrolled', async () => { + let resolve: any; + const AsyncChild = React.lazy( + () => + new Promise((r) => { + resolve = r; + }), + ); + function Test(props: any) { + const [value, setValue] = useControlledState(undefined, 1, props.onChange); + const [showChild, setShowChild] = React.useState(false); + const [isPending, startTransition] = React.useTransition(); + + return ( + + + {showChild && } + + ); + } + + function LoadedComponent() { + return
Hello
; + } + + const onChange = jest.fn(); + render(); + const value = screen.getByTestId('value'); + + expect(value).toHaveTextContent('1'); + await userEvent.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 userEvent.click(value); + expect(value).toHaveTextContent('1 (Loading)'); + expect(onChange).toHaveBeenCalledTimes(1); + 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 userEvent.click(value); + expect(value).toHaveTextContent('3'); + expect(onChange).toHaveBeenCalledTimes(2); + expect(onChange).toHaveBeenLastCalledWith(3); + }); +}); diff --git a/src/hooks/useControlledState/useControlledState.ts b/src/hooks/useControlledState/useControlledState.ts new file mode 100644 index 0000000000..9a2cde1fee --- /dev/null +++ b/src/hooks/useControlledState/useControlledState.ts @@ -0,0 +1,58 @@ +import React from 'react'; + +export function useControlledState( + value: Exclude, + defaultValue: Exclude | undefined, + onChange?: (v: C, ...args: any[]) => void, +): [T, (value: C) => void]; +export function useControlledState( + value: Exclude | undefined, + defaultValue: Exclude, + onChange?: (v: C, ...args: any[]) => void, +): [T, (value: C) => void]; +export function useControlledState( + value: T, + defaultValue: T, + onUpdate?: (value: C, ...args: any[]) => void, +) { + const [innerValue, setInnerValue] = React.useState(value ?? defaultValue); + + const isControlledRef = React.useRef(value !== undefined); + const isControlled = value !== undefined; + React.useEffect(() => { + const wasControlled = isControlledRef.current; + if (wasControlled !== isControlled) { + console.warn( + `WARN: A component changed from ${ + wasControlled ? 'controlled' : 'uncontrolled' + } to ${isControlled ? 'controlled' : 'uncontrolled'}.`, + ); + } + isControlledRef.current = isControlled; + }, [isControlled]); + + let currentValue = isControlled ? value : innerValue; + const setState = React.useCallback( + // We do not use setState callback syntax case because of a side effect + // that we call `onUpdate` inside the callback function and onUpdate + // in a controlling component frequently calls setState itself, + // therefore we call `setState` while we're rendering a different component. + (newValue: C, ...args: any[]) => { + if (!Object.is(currentValue, newValue)) { + onUpdate?.(newValue, ...args); + } + 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 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 = newValue; + setInnerValue(newValue); + } + }, + [isControlled, onUpdate, currentValue], + ); + + return [currentValue, setState] as const; +} diff --git a/src/hooks/useSelect/README.md b/src/hooks/useSelect/README.md index 759109be9c..d2f57c7519 100644 --- a/src/hooks/useSelect/README.md +++ b/src/hooks/useSelect/README.md @@ -30,6 +30,6 @@ The `useSelect` hook used to handle items selection in list | handleSelection | Handles item selection | `function` | | open | List container visibility state | `boolean` | | setOpen | (deprecated) use toggleOpen. Sets value for `open` property | `function` | -| toggleOpen | Inver the value of `open` | `function` | +| toggleOpen | Invert the value of `open` | `function` | | activeIndex | Index of active option | `number` | | setActiveIndex | Sets value for `activeIndex` property | `function` | diff --git a/src/hooks/useSelect/useOpenState.ts b/src/hooks/useSelect/useOpenState.ts index 50f0e24d42..8379b60ed0 100644 --- a/src/hooks/useSelect/useOpenState.ts +++ b/src/hooks/useSelect/useOpenState.ts @@ -1,32 +1,33 @@ import React from 'react'; +import {useControlledState} from '../useControlledState/useControlledState'; + import type {UseOpenProps} from './types'; export const useOpenState = (props: UseOpenProps) => { - const [open, setOpenState] = React.useState(props.defaultOpen || false); - const {onOpenChange, onClose} = props; - const isControlled = typeof props.open === 'boolean'; - const openValue = isControlled ? (props.open as boolean) : open; + const [open, setOpenState] = useControlledState( + props.open, + props.defaultOpen ?? false, + props.onOpenChange, + ); + const {onClose} = props; const toggleOpen = React.useCallback( (val?: boolean) => { - const newOpen = typeof val === 'boolean' ? val : !openValue; - if (newOpen !== openValue) { - onOpenChange?.(newOpen); - if (!isControlled) { - setOpenState(newOpen); - } + const newOpen = typeof val === 'boolean' ? val : !open; + if (newOpen !== open) { + setOpenState(newOpen); } if (newOpen === false && onClose) { onClose(); } }, - [openValue, onOpenChange, isControlled, onClose], + [open, setOpenState, onClose], ); return { - open: openValue, + open, toggleOpen, }; }; diff --git a/src/hooks/useSelect/useSelect.ts b/src/hooks/useSelect/useSelect.ts index 7c5c5745ad..01338cce4a 100644 --- a/src/hooks/useSelect/useSelect.ts +++ b/src/hooks/useSelect/useSelect.ts @@ -1,5 +1,7 @@ import React from 'react'; +import {useControlledState} from '../useControlledState'; + import type {UseSelectOption, UseSelectProps, UseSelectResult} from './types'; import {useOpenState} from './useOpenState'; @@ -13,10 +15,8 @@ export const useSelect = ({ multiple, onUpdate, }: UseSelectProps): UseSelectResult => { - const [innerValue, setInnerValue] = React.useState(defaultValue); + const [value, setValue] = useControlledState(valueProps, defaultValue, onUpdate); const [activeIndex, setActiveIndex] = React.useState(); - const value = valueProps || innerValue; - const uncontrolled = !valueProps; const {toggleOpen, ...openState} = useOpenState({ defaultOpen, onClose, @@ -28,16 +28,12 @@ export const useSelect = ({ (option: UseSelectOption) => { if (!value.includes(option.value)) { const nextValue = [option.value]; - onUpdate?.(nextValue); - - if (uncontrolled) { - setInnerValue(nextValue); - } + setValue(nextValue); } toggleOpen(false); }, - [value, uncontrolled, onUpdate, toggleOpen], + [value, setValue, toggleOpen], ); const handleMultipleSelection = React.useCallback( @@ -47,13 +43,9 @@ export const useSelect = ({ ? value.filter((iteratedVal) => iteratedVal !== option.value) : [...value, option.value]; - onUpdate?.(nextValue); - - if (uncontrolled) { - setInnerValue(nextValue); - } + setValue(nextValue); }, - [value, uncontrolled, onUpdate], + [value, setValue], ); const handleSelection = React.useCallback( @@ -68,9 +60,8 @@ export const useSelect = ({ ); const handleClearValue = React.useCallback(() => { - onUpdate?.([]); - setInnerValue([]); - }, [onUpdate]); + setValue([]); + }, [setValue]); return { value,