From 8c3499cc8d67960f0002d45d05627841bfe70341 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Sat, 25 May 2024 00:26:51 +0200 Subject: [PATCH] Only handle form reset when `defaultValue` is used (#3240) * add `useDefaultValue` hook This allows us to have a guaranteed `default value` that never changes unless the component re-mounts. Since the hook returns a stable value, we can safely include it in dependency arrays of certain hooks. Before this change, including this is in the dependency arrays it would cause a trigger or change of the hook when the `defaultValue` changes but we never want that. * do not handle `reset` when no `defaultValue` or `defaultChecked` was provided If a `defaultValue` is provided, then the reset will be handled and the `onChange` will be called with this value. If no `defaultValue` was provided, we won't handle the `reset`, otherwise we would call the `onChange` with `undefined` which is incorrect. * update changelog --- packages/@headlessui-react/CHANGELOG.md | 1 + .../src/components/checkbox/checkbox.tsx | 13 ++++++++++--- .../src/components/combobox/combobox.tsx | 7 +++++-- .../src/components/listbox/listbox.tsx | 8 ++++++-- .../src/components/radio-group/radio-group.tsx | 10 ++++++---- .../src/components/switch/switch.tsx | 13 ++++++++++--- .../src/hooks/use-default-value.ts | 13 +++++++++++++ 7 files changed, 51 insertions(+), 14 deletions(-) create mode 100644 packages/@headlessui-react/src/hooks/use-default-value.ts diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index 1f6c347..52b3be2 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Ensure page doesn't scroll down when pressing `Escape` to close the `Dialog` component ([#3218](https://github.com/tailwindlabs/headlessui/pull/3218)) - Fix crash when toggling between `virtual` and non-virtual mode in `Combobox` component ([#3236](https://github.com/tailwindlabs/headlessui/pull/3236)) - Ensure tabbing to a portalled `` component moves focus inside (without using ``) ([#3239](https://github.com/tailwindlabs/headlessui/pull/3239)) +- Only handle form reset when `defaultValue` is used ([#3240](https://github.com/tailwindlabs/headlessui/pull/3240)) ### Deprecated diff --git a/packages/@headlessui-react/src/components/checkbox/checkbox.tsx b/packages/@headlessui-react/src/components/checkbox/checkbox.tsx index a8d743a..2c7e22a 100644 --- a/packages/@headlessui-react/src/components/checkbox/checkbox.tsx +++ b/packages/@headlessui-react/src/components/checkbox/checkbox.tsx @@ -13,6 +13,7 @@ import React, { } from 'react' import { useActivePress } from '../../hooks/use-active-press' import { useControllable } from '../../hooks/use-controllable' +import { useDefaultValue } from '../../hooks/use-default-value' import { useDisposables } from '../../hooks/use-disposables' import { useEvent } from '../../hooks/use-event' import { useId } from '../../hooks/use-id' @@ -85,7 +86,7 @@ function CheckboxFn { + if (defaultChecked === undefined) return return onChange?.(defaultChecked) - }, [onChange /* Explicitly ignoring `defaultChecked` */]) + }, [onChange, defaultChecked]) return ( <> diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 7ff520d..b5828b2 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -24,6 +24,7 @@ import React, { import { useActivePress } from '../../hooks/use-active-press' import { useByComparator, type ByComparator } from '../../hooks/use-by-comparator' import { useControllable } from '../../hooks/use-controllable' +import { useDefaultValue } from '../../hooks/use-default-value' import { useDisposables } from '../../hooks/use-disposables' import { useElementSize } from '../../hooks/use-element-size' import { useEvent } from '../../hooks/use-event' @@ -635,7 +636,7 @@ function ComboboxFn( controlledValue, controlledOnChange, @@ -887,8 +889,9 @@ function ComboboxFn { + if (defaultValue === undefined) return return theirOnChange?.(defaultValue) - }, [theirOnChange /* Explicitly ignoring `defaultValue` */]) + }, [theirOnChange, defaultValue]) return ( ( controlledValue, controlledOnChange, @@ -660,8 +663,9 @@ function ListboxFn< let ourProps = { ref: listboxRef } let reset = useCallback(() => { + if (defaultValue === undefined) return return theirOnChange?.(defaultValue) - }, [theirOnChange /* Explicitly ignoring `defaultValue` */]) + }, [theirOnChange, defaultValue]) return ( ) let options = state.options as Option[] @@ -188,6 +188,7 @@ function RadioGroupFn(null) let radioGroupRef = useSyncRefs(internalRadioGroupRef, ref) + let defaultValue = useDefaultValue(_defaultValue) let [value, onChange] = useControllable(controlledValue, controlledOnChange, defaultValue) let firstOption = useMemo( @@ -304,8 +305,9 @@ function RadioGroupFn ({ value }) satisfies RadioGroupRenderPropArg, [value]) let reset = useCallback(() => { - return triggerChange(defaultValue!) - }, [triggerChange /* Explicitly ignoring `defaultValue` */]) + if (defaultValue === undefined) return + return triggerChange(defaultValue) + }, [triggerChange, defaultValue]) return ( diff --git a/packages/@headlessui-react/src/components/switch/switch.tsx b/packages/@headlessui-react/src/components/switch/switch.tsx index e1080fa..5a17ce0 100644 --- a/packages/@headlessui-react/src/components/switch/switch.tsx +++ b/packages/@headlessui-react/src/components/switch/switch.tsx @@ -17,6 +17,7 @@ import React, { } from 'react' import { useActivePress } from '../../hooks/use-active-press' import { useControllable } from '../../hooks/use-controllable' +import { useDefaultValue } from '../../hooks/use-default-value' import { useDisposables } from '../../hooks/use-disposables' import { useEvent } from '../../hooks/use-event' import { useId } from '../../hooks/use-id' @@ -146,7 +147,7 @@ function SwitchFn( id = providedId || `headlessui-switch-${internalId}`, disabled = providedDisabled || false, checked: controlledChecked, - defaultChecked = false, + defaultChecked: _defaultChecked, onChange: controlledOnChange, name, value, @@ -162,7 +163,12 @@ function SwitchFn( groupContext === null ? null : groupContext.setSwitch ) - let [checked, onChange] = useControllable(controlledChecked, controlledOnChange, defaultChecked) + let defaultChecked = useDefaultValue(_defaultChecked) + let [checked, onChange] = useControllable( + controlledChecked, + controlledOnChange, + defaultChecked ?? false + ) let d = useDisposables() let [changing, setChanging] = useState(false) @@ -232,8 +238,9 @@ function SwitchFn( ) let reset = useCallback(() => { + if (defaultChecked === undefined) return return onChange?.(defaultChecked) - }, [onChange /* Explicitly ignoring `defaultChecked` */]) + }, [onChange, defaultChecked]) return ( <> diff --git a/packages/@headlessui-react/src/hooks/use-default-value.ts b/packages/@headlessui-react/src/hooks/use-default-value.ts new file mode 100644 index 0000000..b7a3ada --- /dev/null +++ b/packages/@headlessui-react/src/hooks/use-default-value.ts @@ -0,0 +1,13 @@ +import { useState } from 'react' + +/** + * Returns a stable value that never changes unless the component is re-mounted. + * + * This ensures that we can use this value in a dependency array without causing + * unnecessary re-renders (because while the incoming `value` can change, the + * returned `defaultValue` won't change). + */ +export function useDefaultValue(value: T) { + let [defaultValue] = useState(value) + return defaultValue +}