From 6f49e2cce56229c3ebacab00043e834bd3abc496 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 16:40:47 +0100 Subject: [PATCH 01/21] cleanup `XYZPropsWeControl` The idea behind the `PropsWeControl` is that we can omit all the fields that we are controlling entirely. In this case, passing a prop like `role`, but if we already set the role ourselves then the prop won't do anything at all. This is why we want to alert the end user that it is an "error". It can also happen that we "control" the value by default, but keep incoming props into account. For example we generate a unique ID for most components, but you can provide your own to override it. In this case we _don't_ want to include the ID in the `XYZPropsWeControl`. Additionally, we introduced some functionality months ago where we call event callbacks (`onClick`, ...) from the incoming props before our own callbacks. This means that by definition all `onXYZ` callbacks can be provided. --- .../src/components/combobox/combobox.tsx | 43 +++++++++---------- .../src/components/dialog/dialog.tsx | 13 ++++-- .../src/components/disclosure/disclosure.tsx | 4 +- .../src/components/listbox/listbox.tsx | 30 ++++--------- .../src/components/menu/menu.tsx | 23 ++-------- .../src/components/popover/popover.tsx | 13 +++--- .../components/radio-group/radio-group.tsx | 24 +++++++---- .../src/components/switch/switch.tsx | 17 +++++--- .../src/components/tabs/tabs.tsx | 6 +-- 9 files changed, 78 insertions(+), 95 deletions(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 69513abbc6..14bff38ccc 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -695,19 +695,21 @@ interface InputRenderPropArg { disabled: boolean } type InputPropsWeControl = - | 'role' - | 'aria-labelledby' - | 'aria-expanded' | 'aria-activedescendant' | 'aria-autocomplete' - | 'onKeyDown' - | 'onChange' - | 'displayValue' + | 'aria-controls' + | 'aria-expanded' + | 'aria-labelledby' + | 'disabled' + | 'role' export type ComboboxInputProps = Props< TTag, InputRenderPropArg, - InputPropsWeControl + | InputPropsWeControl + // Props we gave a new type + | 'displayValue' + | 'onChange' > & { displayValue?(item: TType): string onChange?(event: React.ChangeEvent): void @@ -1010,15 +1012,12 @@ interface ButtonRenderPropArg { value: any } type ButtonPropsWeControl = - // | 'type' // While we do "control" this prop we allow it to be overridden - | 'tabIndex' - | 'aria-haspopup' | 'aria-controls' | 'aria-expanded' + | 'aria-haspopup' | 'aria-labelledby' | 'disabled' - | 'onClick' - | 'onKeyDown' + | 'tabIndex' export type ComboboxButtonProps = Props< TTag, @@ -1131,13 +1130,8 @@ interface LabelRenderPropArg { open: boolean disabled: boolean } -type LabelPropsWeControl = 'ref' | 'onClick' -export type ComboboxLabelProps = Props< - TTag, - LabelRenderPropArg, - LabelPropsWeControl -> +export type ComboboxLabelProps = Props function LabelFn( props: ComboboxLabelProps, @@ -1175,14 +1169,16 @@ let DEFAULT_OPTIONS_TAG = 'ul' as const interface OptionsRenderPropArg { open: boolean } -type OptionsPropsWeControl = 'aria-labelledby' | 'hold' | 'onKeyDown' | 'role' | 'tabIndex' +type OptionsPropsWeControl = 'aria-labelledby' | 'aria-multiselectable' | 'role' | 'tabIndex' let OptionsRenderFeatures = Features.RenderStrategy | Features.Static export type ComboboxOptionsProps = Props< TTag, OptionsRenderPropArg, - OptionsPropsWeControl + | OptionsPropsWeControl + // Props we gave a new type + | 'hold' > & PropsForFeatures & { hold?: boolean @@ -1263,12 +1259,15 @@ interface OptionRenderPropArg { selected: boolean disabled: boolean } -type ComboboxOptionPropsWeControl = 'role' | 'tabIndex' | 'aria-disabled' | 'aria-selected' +type OptionPropsWeControl = 'role' | 'tabIndex' | 'aria-disabled' | 'aria-selected' export type ComboboxOptionProps = Props< TTag, OptionRenderPropArg, - ComboboxOptionPropsWeControl | 'value' + | OptionPropsWeControl + // Props we gave a new type + | 'disabled' + | 'value' > & { disabled?: boolean value: TType diff --git a/packages/@headlessui-react/src/components/dialog/dialog.tsx b/packages/@headlessui-react/src/components/dialog/dialog.tsx index 096c215782..74901c52a0 100644 --- a/packages/@headlessui-react/src/components/dialog/dialog.tsx +++ b/packages/@headlessui-react/src/components/dialog/dialog.tsx @@ -118,14 +118,19 @@ let DEFAULT_DIALOG_TAG = 'div' as const interface DialogRenderPropArg { open: boolean } -type DialogPropsWeControl = 'role' | 'aria-modal' | 'aria-describedby' | 'aria-labelledby' +type DialogPropsWeControl = 'role' | 'aria-describedby' | 'aria-labelledby' | 'aria-modal' let DialogRenderFeatures = Features.RenderStrategy | Features.Static export type DialogProps = Props< TTag, DialogRenderPropArg, - DialogPropsWeControl + | DialogPropsWeControl + // Props we gave a new type + | 'open' + | 'onClose' + | 'initialFocus' + | '__demoMode' > & PropsForFeatures & { open?: boolean @@ -402,7 +407,7 @@ let DEFAULT_OVERLAY_TAG = 'div' as const interface OverlayRenderPropArg { open: boolean } -type OverlayPropsWeControl = 'aria-hidden' | 'onClick' +type OverlayPropsWeControl = 'aria-hidden' export type DialogOverlayProps = Props< TTag, @@ -454,7 +459,7 @@ let DEFAULT_BACKDROP_TAG = 'div' as const interface BackdropRenderPropArg { open: boolean } -type BackdropPropsWeControl = 'aria-hidden' | 'onClick' +type BackdropPropsWeControl = 'aria-hidden' export type DialogBackdropProps = Props< TTag, diff --git a/packages/@headlessui-react/src/components/disclosure/disclosure.tsx b/packages/@headlessui-react/src/components/disclosure/disclosure.tsx index cbebe24246..fc7f19a142 100644 --- a/packages/@headlessui-react/src/components/disclosure/disclosure.tsx +++ b/packages/@headlessui-react/src/components/disclosure/disclosure.tsx @@ -247,9 +247,7 @@ let DEFAULT_BUTTON_TAG = 'button' as const interface ButtonRenderPropArg { open: boolean } -type ButtonPropsWeControl = - // | 'type' // We allow this to be overridden - 'aria-expanded' | 'aria-controls' | 'onKeyDown' | 'onClick' +type ButtonPropsWeControl = 'aria-controls' | 'aria-expanded' export type DisclosureButtonProps = Props< TTag, diff --git a/packages/@headlessui-react/src/components/listbox/listbox.tsx b/packages/@headlessui-react/src/components/listbox/listbox.tsx index 12a3923c04..00a5504799 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.tsx @@ -586,14 +586,11 @@ interface ButtonRenderPropArg { value: any } type ButtonPropsWeControl = - // | 'type' // We allow this to be overridden - | 'aria-haspopup' | 'aria-controls' | 'aria-expanded' + | 'aria-haspopup' | 'aria-labelledby' | 'disabled' - | 'onKeyDown' - | 'onClick' export type ListboxButtonProps = Props< TTag, @@ -703,13 +700,8 @@ interface LabelRenderPropArg { open: boolean disabled: boolean } -type LabelPropsWeControl = 'ref' | 'onClick' -export type ListboxLabelProps = Props< - TTag, - LabelRenderPropArg, - LabelPropsWeControl -> +export type ListboxLabelProps = Props function LabelFn( props: ListboxLabelProps, @@ -749,8 +741,8 @@ interface OptionsRenderPropArg { type OptionsPropsWeControl = | 'aria-activedescendant' | 'aria-labelledby' + | 'aria-multiselectable' | 'aria-orientation' - | 'onKeyDown' | 'role' | 'tabIndex' @@ -906,21 +898,15 @@ interface OptionRenderPropArg { selected: boolean disabled: boolean } -type ListboxOptionPropsWeControl = - | 'role' - | 'tabIndex' - | 'aria-disabled' - | 'aria-selected' - | 'onPointerLeave' - | 'onMouseLeave' - | 'onPointerMove' - | 'onMouseMove' - | 'onFocus' +type OptionPropsWeControl = 'aria-disabled' | 'aria-selected' | 'role' | 'tabIndex' export type ListboxOptionProps = Props< TTag, OptionRenderPropArg, - ListboxOptionPropsWeControl | 'value' + | OptionPropsWeControl + // Props we gave a new type + | 'disabled' + | 'value' > & { disabled?: boolean value: TType diff --git a/packages/@headlessui-react/src/components/menu/menu.tsx b/packages/@headlessui-react/src/components/menu/menu.tsx index 2245090752..5ec628cdf6 100644 --- a/packages/@headlessui-react/src/components/menu/menu.tsx +++ b/packages/@headlessui-react/src/components/menu/menu.tsx @@ -308,9 +308,7 @@ let DEFAULT_BUTTON_TAG = 'button' as const interface ButtonRenderPropArg { open: boolean } -type ButtonPropsWeControl = - // | 'type' // We allow this to be overridden - 'aria-haspopup' | 'aria-controls' | 'aria-expanded' | 'onKeyDown' | 'onClick' +type ButtonPropsWeControl = 'aria-controls' | 'aria-expanded' | 'aria-haspopup' export type MenuButtonProps = Props< TTag, @@ -405,12 +403,7 @@ let DEFAULT_ITEMS_TAG = 'div' as const interface ItemsRenderPropArg { open: boolean } -type ItemsPropsWeControl = - | 'aria-activedescendant' - | 'aria-labelledby' - | 'onKeyDown' - | 'role' - | 'tabIndex' +type ItemsPropsWeControl = 'aria-activedescendant' | 'aria-labelledby' | 'role' | 'tabIndex' let ItemsRenderFeatures = Features.RenderStrategy | Features.Static @@ -586,20 +579,12 @@ interface ItemRenderPropArg { disabled: boolean close: () => void } -type MenuItemPropsWeControl = - | 'role' - | 'tabIndex' - | 'aria-disabled' - | 'onPointerLeave' - | 'onPointerMove' - | 'onMouseLeave' - | 'onMouseMove' - | 'onFocus' +type ItemPropsWeControl = 'aria-disabled' | 'role' | 'tabIndex' export type MenuItemProps = Props< TTag, ItemRenderPropArg, - MenuItemPropsWeControl + ItemPropsWeControl > & { disabled?: boolean } diff --git a/packages/@headlessui-react/src/components/popover/popover.tsx b/packages/@headlessui-react/src/components/popover/popover.tsx index 3a11217db8..5a836f9d0e 100644 --- a/packages/@headlessui-react/src/components/popover/popover.tsx +++ b/packages/@headlessui-react/src/components/popover/popover.tsx @@ -385,9 +385,7 @@ let DEFAULT_BUTTON_TAG = 'button' as const interface ButtonRenderPropArg { open: boolean } -type ButtonPropsWeControl = - // | 'type' // We allow this to be overridden - 'aria-expanded' | 'aria-controls' | 'onKeyDown' | 'onClick' +type ButtonPropsWeControl = 'aria-controls' | 'aria-expanded' export type PopoverButtonProps = Props< TTag, @@ -620,7 +618,7 @@ let DEFAULT_OVERLAY_TAG = 'div' as const interface OverlayRenderPropArg { open: boolean } -type OverlayPropsWeControl = 'aria-hidden' | 'onClick' +type OverlayPropsWeControl = 'aria-hidden' let OverlayRenderFeatures = Features.RenderStrategy | Features.Static @@ -684,14 +682,17 @@ interface PanelRenderPropArg { open: boolean close: (focusableElement?: HTMLElement | MutableRefObject) => void } -type PanelPropsWeControl = 'onKeyDown' let PanelRenderFeatures = Features.RenderStrategy | Features.Static +type PanelPropsWeControl = 'tabIndex' + export type PopoverPanelProps = Props< TTag, PanelRenderPropArg, - PanelPropsWeControl + | PanelPropsWeControl + // Props we gave a new type + | 'focus' > & PropsForFeatures & { focus?: boolean diff --git a/packages/@headlessui-react/src/components/radio-group/radio-group.tsx b/packages/@headlessui-react/src/components/radio-group/radio-group.tsx index dc977aaa7d..ec2957a964 100644 --- a/packages/@headlessui-react/src/components/radio-group/radio-group.tsx +++ b/packages/@headlessui-react/src/components/radio-group/radio-group.tsx @@ -140,7 +140,14 @@ type RadioGroupPropsWeControl = 'role' | 'aria-labelledby' | 'aria-describedby' export type RadioGroupProps = Props< TTag, RadioGroupRenderPropArg, - RadioGroupPropsWeControl | 'value' | 'defaultValue' | 'onChange' | 'disabled' | 'name' | 'by' + | RadioGroupPropsWeControl + // Props we gave a new type + | 'by' + | 'defaultValue' + | 'disabled' + | 'name' + | 'onChange' + | 'value' > & { value?: TType defaultValue?: TType @@ -161,7 +168,7 @@ function RadioGroupFn a === z, + by = (a: TType, z: TType) => a === z, disabled = false, ...theirProps } = props @@ -374,19 +381,20 @@ interface OptionRenderPropArg { active: boolean disabled: boolean } -type RadioPropsWeControl = +type OptionPropsWeControl = | 'aria-checked' - | 'onBlur' - | 'onClick' - | 'onFocus' - | 'ref' + | 'aria-describedby' + | 'aria-lablledby' | 'role' | 'tabIndex' export type RadioOptionProps = Props< TTag, OptionRenderPropArg, - RadioPropsWeControl | 'value' | 'disabled' + | OptionPropsWeControl + // Props we gave a new type + | 'value' + | 'disabled' > & { value: TType disabled?: boolean diff --git a/packages/@headlessui-react/src/components/switch/switch.tsx b/packages/@headlessui-react/src/components/switch/switch.tsx index ddf7fe927c..971df5603c 100644 --- a/packages/@headlessui-react/src/components/switch/switch.tsx +++ b/packages/@headlessui-react/src/components/switch/switch.tsx @@ -96,19 +96,22 @@ interface SwitchRenderPropArg { checked: boolean } type SwitchPropsWeControl = - | 'role' - | 'tabIndex' | 'aria-checked' - | 'aria-labelledby' | 'aria-describedby' - | 'onClick' - | 'onKeyUp' - | 'onKeyPress' + | 'aria-labelledby' + | 'role' + | 'tabIndex' export type SwitchProps = Props< TTag, SwitchRenderPropArg, - SwitchPropsWeControl | 'checked' | 'defaultChecked' | 'onChange' | 'name' | 'value' + | SwitchPropsWeControl + // Props we gave a new type + | 'checked' + | 'defaultChecked' + | 'onChange' + | 'name' + | 'value' > & { checked?: boolean defaultChecked?: boolean diff --git a/packages/@headlessui-react/src/components/tabs/tabs.tsx b/packages/@headlessui-react/src/components/tabs/tabs.tsx index 57ead334bb..873a8836a3 100644 --- a/packages/@headlessui-react/src/components/tabs/tabs.tsx +++ b/packages/@headlessui-react/src/components/tabs/tabs.tsx @@ -329,7 +329,7 @@ let DEFAULT_LIST_TAG = 'div' as const interface ListRenderPropArg { selectedIndex: number } -type ListPropsWeControl = 'role' | 'aria-orientation' +type ListPropsWeControl = 'aria-orientation' | 'role' export type TabListProps = Props< TTag, @@ -370,9 +370,7 @@ let DEFAULT_TAB_TAG = 'button' as const interface TabRenderPropArg { selected: boolean } -type TabPropsWeControl = - // | 'type' // We allow this to be overridden - 'role' | 'aria-controls' | 'aria-selected' | 'tabIndex' +type TabPropsWeControl = 'aria-controls' | 'aria-selected' | 'role' | 'tabIndex' export type TabProps = Props< TTag, From 9f8c6851c91176761a293e40e74fdac4af63d54d Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 16:56:25 +0100 Subject: [PATCH 02/21] improve defining types Whenever we explicitly provide custom types for certain props, then we make sure to omit those keys first from the original props (of let's say an `input`). This is important so that TypeScript doesn't try to "merge" those types together. --- .../src/components/combobox/combobox.tsx | 34 +++++++--------- .../src/components/dialog/dialog.tsx | 9 +---- .../src/components/listbox/listbox.tsx | 14 +++---- .../src/components/popover/popover.tsx | 6 +-- .../components/radio-group/radio-group.tsx | 40 ++++++++----------- .../src/components/switch/switch.tsx | 23 +++++------ packages/@headlessui-react/src/types.ts | 8 +++- 7 files changed, 55 insertions(+), 79 deletions(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 14bff38ccc..a1a10464ae 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -706,14 +706,12 @@ type InputPropsWeControl = export type ComboboxInputProps = Props< TTag, InputRenderPropArg, - | InputPropsWeControl - // Props we gave a new type - | 'displayValue' - | 'onChange' -> & { - displayValue?(item: TType): string - onChange?(event: React.ChangeEvent): void -} + InputPropsWeControl, + { + displayValue?(item: TType): string + onChange?(event: React.ChangeEvent): void + } +> function InputFn< TTag extends ElementType = typeof DEFAULT_INPUT_TAG, @@ -1176,13 +1174,11 @@ let OptionsRenderFeatures = Features.RenderStrategy | Features.Static export type ComboboxOptionsProps = Props< TTag, OptionsRenderPropArg, - | OptionsPropsWeControl - // Props we gave a new type - | 'hold' -> & + OptionsPropsWeControl, PropsForFeatures & { hold?: boolean } +> function OptionsFn( props: ComboboxOptionsProps, @@ -1264,14 +1260,12 @@ type OptionPropsWeControl = 'role' | 'tabIndex' | 'aria-disabled' | 'aria-select export type ComboboxOptionProps = Props< TTag, OptionRenderPropArg, - | OptionPropsWeControl - // Props we gave a new type - | 'disabled' - | 'value' -> & { - disabled?: boolean - value: TType -} + OptionPropsWeControl, + { + disabled?: boolean + value: TType + } +> function OptionFn< TTag extends ElementType = typeof DEFAULT_OPTION_TAG, diff --git a/packages/@headlessui-react/src/components/dialog/dialog.tsx b/packages/@headlessui-react/src/components/dialog/dialog.tsx index 74901c52a0..43a10da8df 100644 --- a/packages/@headlessui-react/src/components/dialog/dialog.tsx +++ b/packages/@headlessui-react/src/components/dialog/dialog.tsx @@ -125,19 +125,14 @@ let DialogRenderFeatures = Features.RenderStrategy | Features.Static export type DialogProps = Props< TTag, DialogRenderPropArg, - | DialogPropsWeControl - // Props we gave a new type - | 'open' - | 'onClose' - | 'initialFocus' - | '__demoMode' -> & + DialogPropsWeControl, PropsForFeatures & { open?: boolean onClose(value: boolean): void initialFocus?: MutableRefObject __demoMode?: boolean } +> function DialogFn( props: DialogProps, diff --git a/packages/@headlessui-react/src/components/listbox/listbox.tsx b/packages/@headlessui-react/src/components/listbox/listbox.tsx index 00a5504799..3cd3223cca 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.tsx @@ -903,14 +903,12 @@ type OptionPropsWeControl = 'aria-disabled' | 'aria-selected' | 'role' | 'tabInd export type ListboxOptionProps = Props< TTag, OptionRenderPropArg, - | OptionPropsWeControl - // Props we gave a new type - | 'disabled' - | 'value' -> & { - disabled?: boolean - value: TType -} + OptionPropsWeControl, + { + disabled?: boolean + value: TType + } +> function OptionFn< TTag extends ElementType = typeof DEFAULT_OPTION_TAG, diff --git a/packages/@headlessui-react/src/components/popover/popover.tsx b/packages/@headlessui-react/src/components/popover/popover.tsx index 5a836f9d0e..1fa305afdf 100644 --- a/packages/@headlessui-react/src/components/popover/popover.tsx +++ b/packages/@headlessui-react/src/components/popover/popover.tsx @@ -690,13 +690,11 @@ type PanelPropsWeControl = 'tabIndex' export type PopoverPanelProps = Props< TTag, PanelRenderPropArg, - | PanelPropsWeControl - // Props we gave a new type - | 'focus' -> & + PanelPropsWeControl, PropsForFeatures & { focus?: boolean } +> function PanelFn( props: PopoverPanelProps, diff --git a/packages/@headlessui-react/src/components/radio-group/radio-group.tsx b/packages/@headlessui-react/src/components/radio-group/radio-group.tsx index ec2957a964..d9c34f9d24 100644 --- a/packages/@headlessui-react/src/components/radio-group/radio-group.tsx +++ b/packages/@headlessui-react/src/components/radio-group/radio-group.tsx @@ -140,22 +140,16 @@ type RadioGroupPropsWeControl = 'role' | 'aria-labelledby' | 'aria-describedby' export type RadioGroupProps = Props< TTag, RadioGroupRenderPropArg, - | RadioGroupPropsWeControl - // Props we gave a new type - | 'by' - | 'defaultValue' - | 'disabled' - | 'name' - | 'onChange' - | 'value' -> & { - value?: TType - defaultValue?: TType - onChange?(value: TType): void - by?: (keyof TType & string) | ((a: TType, z: TType) => boolean) - disabled?: boolean - name?: string -} + RadioGroupPropsWeControl, + { + value?: TType + defaultValue?: TType + onChange?(value: TType): void + by?: (keyof TType & string) | ((a: TType, z: TType) => boolean) + disabled?: boolean + name?: string + } +> function RadioGroupFn( props: RadioGroupProps, @@ -391,14 +385,12 @@ type OptionPropsWeControl = export type RadioOptionProps = Props< TTag, OptionRenderPropArg, - | OptionPropsWeControl - // Props we gave a new type - | 'value' - | 'disabled' -> & { - value: TType - disabled?: boolean -} + OptionPropsWeControl, + { + value: TType + disabled?: boolean + } +> function OptionFn< TTag extends ElementType = typeof DEFAULT_OPTION_TAG, diff --git a/packages/@headlessui-react/src/components/switch/switch.tsx b/packages/@headlessui-react/src/components/switch/switch.tsx index 971df5603c..3d6f3975fd 100644 --- a/packages/@headlessui-react/src/components/switch/switch.tsx +++ b/packages/@headlessui-react/src/components/switch/switch.tsx @@ -105,20 +105,15 @@ type SwitchPropsWeControl = export type SwitchProps = Props< TTag, SwitchRenderPropArg, - | SwitchPropsWeControl - // Props we gave a new type - | 'checked' - | 'defaultChecked' - | 'onChange' - | 'name' - | 'value' -> & { - checked?: boolean - defaultChecked?: boolean - onChange?(checked: boolean): void - name?: string - value?: string -} + SwitchPropsWeControl, + { + checked?: boolean + defaultChecked?: boolean + onChange?(checked: boolean): void + name?: string + value?: string + } +> function SwitchFn( props: SwitchProps, diff --git a/packages/@headlessui-react/src/types.ts b/packages/@headlessui-react/src/types.ts index caf933cbbe..64bc16be2c 100644 --- a/packages/@headlessui-react/src/types.ts +++ b/packages/@headlessui-react/src/types.ts @@ -48,8 +48,12 @@ type ClassNameOverride = export type Props< TTag extends ReactTag, TSlot = {}, - TOmitableProps extends PropertyKey = never -> = CleanProps & OurProps & ClassNameOverride + TOmitableProps extends PropertyKey = never, + Overrides = {} +> = CleanProps & + OurProps & + ClassNameOverride & + Overrides type Without = { [P in Exclude]?: never } export type XOR = T | U extends __ From 46efcfc32e6d2b8834d3ab3da947ef2d33ac0f0c Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 17:05:49 +0100 Subject: [PATCH 03/21] cleanup: move `useEffect` --- packages/@headlessui-react/src/components/combobox/combobox.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index a1a10464ae..55995cc87c 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -4,6 +4,7 @@ import React, { createRef, useCallback, useContext, + useEffect, useMemo, useReducer, useRef, @@ -14,7 +15,6 @@ import React, { MouseEvent as ReactMouseEvent, MutableRefObject, Ref, - useEffect, } from 'react' import { ByComparator, EnsureArray, Expand, Props } from '../../types' From 797d5f14808212cdeef6dd31152ecb0c98a50de2 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 17:06:11 +0100 Subject: [PATCH 04/21] add `defaultValue` explicitly --- packages/@headlessui-react/src/components/combobox/combobox.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 55995cc87c..1c21981723 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -708,6 +708,7 @@ export type ComboboxInputProps = Props< InputRenderPropArg, InputPropsWeControl, { + defaultValue?: TType displayValue?(item: TType): string onChange?(event: React.ChangeEvent): void } From e2511a9e779f3ce3350c4cd4e084715562ef2bd2 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 17:13:59 +0100 Subject: [PATCH 05/21] ensure tests are not using `any` because of `onChange={console.log}` The `console.log` is typed as `(...args: any[]) => void` which means that it will incorrectly mark its incoming data as `any` as well. Converting it to `x => console.log(x)` makes TypeScript happy. Or in this case, angry since it found a bug. This is required because it _can_ be that your value (e.g.: the value of a Combobox) is an object (e.g.: a `User`), but it is also nullable. Therefore we can provide the value `null`. This would mean that eventually this resolves to `keyof null` which is `never`, but we just want a string in this case. ```diff -export type ByComparator = (keyof T & string) | ((a: T, b: T) => boolean) +export type ByComparator = + | (T extends null ? string : keyof T & string) + | ((a: T, b: T) => boolean) ``` --- .../src/components/combobox/combobox.test.tsx | 218 +++++++++--------- packages/@headlessui-react/src/types.ts | 4 +- 2 files changed, 112 insertions(+), 110 deletions(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx index 3fc8957555..fcc94ff4e7 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx @@ -74,7 +74,7 @@ describe('safeguards', () => { 'should be possible to render a Combobox without crashing', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -100,7 +100,7 @@ describe('Rendering', () => { 'should be possible to render a Combobox using a render prop', suppressConsoleLogs(async () => { render( - + console.log(x)}> {({ open }) => ( <> @@ -137,7 +137,7 @@ describe('Rendering', () => { 'should be possible to disable a Combobox', suppressConsoleLogs(async () => { render( - + console.log(x)} disabled> Trigger @@ -206,7 +206,7 @@ describe('Rendering', () => { 'should use object equality by default', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger {options.map((option) => ( @@ -236,7 +236,7 @@ describe('Rendering', () => { 'should be possible to compare null values by a field', suppressConsoleLogs(async () => { render( - + console.log(x)} by="id"> Trigger {options.map((option) => ( @@ -274,7 +274,7 @@ describe('Rendering', () => { 'should be possible to compare objects by a field', suppressConsoleLogs(async () => { render( - + console.log(x)} by="id"> Trigger {options.map((option) => ( @@ -306,7 +306,7 @@ describe('Rendering', () => { render( console.log(x)} by={(a, z) => a.id === z.id} > Trigger @@ -687,7 +687,7 @@ describe('Rendering', () => { 'should be possible to render a Combobox.Label using a render prop', suppressConsoleLogs(async () => { render( - + console.log(x)}> {JSON.stringify} Trigger @@ -725,7 +725,7 @@ describe('Rendering', () => { 'should be possible to link Input/Button and Label if Label is rendered last', suppressConsoleLogs(async () => { render( - + console.log(x)}> Label @@ -741,7 +741,7 @@ describe('Rendering', () => { 'should be possible to render a Combobox.Label using a render prop and an `as` prop', suppressConsoleLogs(async () => { render( - + console.log(x)}> {JSON.stringify} Trigger @@ -776,7 +776,7 @@ describe('Rendering', () => { 'should be possible to render a Combobox.Button using a render prop', suppressConsoleLogs(async () => { render( - + console.log(x)}> {JSON.stringify} @@ -809,7 +809,7 @@ describe('Rendering', () => { 'should be possible to render a Combobox.Button using a render prop and an `as` prop', suppressConsoleLogs(async () => { render( - + console.log(x)}> {JSON.stringify} @@ -844,7 +844,7 @@ describe('Rendering', () => { 'should be possible to render a Combobox.Button and a Combobox.Label and see them linked together', suppressConsoleLogs(async () => { render( - + console.log(x)}> Label Trigger @@ -871,7 +871,7 @@ describe('Rendering', () => { describe('`type` attribute', () => { it('should set the `type` to "button" by default', async () => { render( - + console.log(x)}> Trigger @@ -882,7 +882,7 @@ describe('Rendering', () => { it('should not set the `type` to "button" if it already contains a `type`', async () => { render( - + console.log(x)}> Trigger @@ -897,7 +897,7 @@ describe('Rendering', () => { )) render( - + console.log(x)}> Trigger @@ -908,7 +908,7 @@ describe('Rendering', () => { it('should not set the type if the "as" prop is not a "button"', async () => { render( - + console.log(x)}> Trigger @@ -923,7 +923,7 @@ describe('Rendering', () => { )) render( - + console.log(x)}> Trigger @@ -939,7 +939,7 @@ describe('Rendering', () => { 'should be possible to render Combobox.Options using a render prop', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -974,7 +974,7 @@ describe('Rendering', () => { it('should be possible to always render the Combobox.Options if we provide it a `static` prop', () => { render( - + console.log(x)}> Trigger @@ -991,7 +991,7 @@ describe('Rendering', () => { it('should be possible to use a different render strategy for the Combobox.Options', async () => { render( - + console.log(x)}> Trigger @@ -1016,7 +1016,7 @@ describe('Rendering', () => { 'should be possible to render a Combobox.Option using a render prop', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -1049,7 +1049,7 @@ describe('Rendering', () => { function Example({ hide = false }) { return ( <> - + console.log(x)}> Trigger @@ -1405,7 +1405,7 @@ describe('Rendering composition', () => { 'should be possible to conditionally render classNames (aka className can be a function?!)', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -1466,7 +1466,7 @@ describe('Rendering composition', () => { 'should be possible to swap the Combobox option with a button for example', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -1501,7 +1501,7 @@ describe('Rendering composition', () => { 'should mark all the elements between Combobox.Options and Combobox.Option with role none', suppressConsoleLogs(async () => { render( - + console.log(x)}>
@@ -1560,7 +1560,7 @@ describe('Composition', () => { suppressConsoleLogs(async () => { let orderFn = jest.fn() render( - + console.log(x)}> Trigger @@ -1618,7 +1618,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the combobox with Enter', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -1667,7 +1667,7 @@ describe('Keyboard interactions', () => { 'should not be possible to open the combobox with Enter when the button is disabled', suppressConsoleLogs(async () => { render( - + console.log(x)} disabled> Trigger @@ -1703,7 +1703,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the combobox with Enter, and focus the selected option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -1752,7 +1752,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the combobox with Enter, and focus the selected option (when using the `hidden` render strategy)', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -1823,7 +1823,7 @@ describe('Keyboard interactions', () => { ] let selectedOption = myOptions[1] render( - + console.log(x)}> Trigger @@ -1874,7 +1874,7 @@ describe('Keyboard interactions', () => { 'should have no active combobox option when there are no combobox options at all', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -1905,7 +1905,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the combobox with Space', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -1952,7 +1952,7 @@ describe('Keyboard interactions', () => { 'should not be possible to open the combobox with Space when the button is disabled', suppressConsoleLogs(async () => { render( - + console.log(x)} disabled> Trigger @@ -1988,7 +1988,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the combobox with Space, and focus the selected option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2036,7 +2036,7 @@ describe('Keyboard interactions', () => { 'should have no active combobox option when there are no combobox options at all', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2063,7 +2063,7 @@ describe('Keyboard interactions', () => { 'should have no active combobox option upon Space key press, when there are no non-disabled combobox options', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2104,7 +2104,7 @@ describe('Keyboard interactions', () => { 'should be possible to close an open combobox with Escape', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2149,7 +2149,7 @@ describe('Keyboard interactions', () => { let handleKeyDown = jest.fn() render(
- + console.log(x)}> Trigger @@ -2178,7 +2178,7 @@ describe('Keyboard interactions', () => { let handleKeyDown = jest.fn() render(
- + console.log(x)}> Trigger @@ -2207,7 +2207,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the combobox with ArrowDown', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2253,7 +2253,7 @@ describe('Keyboard interactions', () => { 'should not be possible to open the combobox with ArrowDown when the button is disabled', suppressConsoleLogs(async () => { render( - + console.log(x)} disabled> Trigger @@ -2289,7 +2289,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the combobox with ArrowDown, and focus the selected option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2335,7 +2335,7 @@ describe('Keyboard interactions', () => { 'should have no active combobox option when there are no combobox options at all', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2362,7 +2362,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the combobox with ArrowUp and the last option should be active', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2408,7 +2408,7 @@ describe('Keyboard interactions', () => { 'should not be possible to open the combobox with ArrowUp and the last option should be active when the button is disabled', suppressConsoleLogs(async () => { render( - + console.log(x)} disabled> Trigger @@ -2444,7 +2444,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the combobox with ArrowUp, and focus the selected option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2490,7 +2490,7 @@ describe('Keyboard interactions', () => { 'should have no active combobox option when there are no combobox options at all', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2515,7 +2515,7 @@ describe('Keyboard interactions', () => { 'should be possible to use ArrowUp to navigate the combobox options and jump to the first non-disabled one', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2851,7 +2851,7 @@ describe('Keyboard interactions', () => { 'should be possible to close an open combobox with Escape', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2890,7 +2890,7 @@ describe('Keyboard interactions', () => { 'should bubble escape when using `static` on Combobox.Options', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2941,7 +2941,7 @@ describe('Keyboard interactions', () => { 'should bubble escape when not using Combobox.Options at all', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2987,7 +2987,7 @@ describe('Keyboard interactions', () => { 'should sync the input field correctly and reset it when pressing Escape', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3022,7 +3022,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the combobox with ArrowDown', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3068,7 +3068,7 @@ describe('Keyboard interactions', () => { 'should not be possible to open the combobox with ArrowDown when the button is disabled', suppressConsoleLogs(async () => { render( - + console.log(x)} disabled> Trigger @@ -3104,7 +3104,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the combobox with ArrowDown, and focus the selected option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3150,7 +3150,7 @@ describe('Keyboard interactions', () => { 'should have no active combobox option when there are no combobox options at all', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3175,7 +3175,7 @@ describe('Keyboard interactions', () => { 'should be possible to use ArrowDown to navigate the combobox options', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3220,7 +3220,7 @@ describe('Keyboard interactions', () => { 'should be possible to use ArrowDown to navigate the combobox options and skip the first disabled one', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3258,7 +3258,7 @@ describe('Keyboard interactions', () => { 'should be possible to use ArrowDown to navigate the combobox options and jump to the first non-disabled one', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3298,7 +3298,7 @@ describe('Keyboard interactions', () => { 'should be possible to go to the next item if no value is set', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3337,7 +3337,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the combobox with ArrowUp and the last option should be active', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3383,7 +3383,7 @@ describe('Keyboard interactions', () => { 'should not be possible to open the combobox with ArrowUp and the last option should be active when the button is disabled', suppressConsoleLogs(async () => { render( - + console.log(x)} disabled> Trigger @@ -3419,7 +3419,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the combobox with ArrowUp, and focus the selected option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3465,7 +3465,7 @@ describe('Keyboard interactions', () => { 'should have no active combobox option when there are no combobox options at all', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3490,7 +3490,7 @@ describe('Keyboard interactions', () => { 'should be possible to use ArrowUp to navigate the combobox options and jump to the first non-disabled one', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3529,7 +3529,7 @@ describe('Keyboard interactions', () => { 'should not be possible to navigate up or down if there is only a single non-disabled option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3576,7 +3576,7 @@ describe('Keyboard interactions', () => { 'should be possible to use ArrowUp to navigate the combobox options', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3634,7 +3634,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the End key to go to the last combobox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3663,7 +3663,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the End key to go to the last non disabled combobox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3697,7 +3697,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the End key to go to the first combobox option if that is the only non-disabled combobox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3734,7 +3734,7 @@ describe('Keyboard interactions', () => { 'should have no active combobox option upon End key press, when there are no non-disabled combobox options', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3773,7 +3773,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the PageDown key to go to the last combobox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3802,7 +3802,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the PageDown key to go to the last non disabled combobox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3839,7 +3839,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the PageDown key to go to the first combobox option if that is the only non-disabled combobox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3876,7 +3876,7 @@ describe('Keyboard interactions', () => { 'should have no active combobox option upon PageDown key press, when there are no non-disabled combobox options', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3915,7 +3915,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the Home key to go to the first combobox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3947,7 +3947,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the Home key to go to the first non disabled combobox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3983,7 +3983,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the Home key to go to the last combobox option if that is the only non-disabled combobox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -4020,7 +4020,7 @@ describe('Keyboard interactions', () => { 'should have no active combobox option upon Home key press, when there are no non-disabled combobox options', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -4059,7 +4059,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the PageUp key to go to the first combobox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -4091,7 +4091,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the PageUp key to go to the first non disabled combobox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -4126,7 +4126,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the PageUp key to go to the last combobox option if that is the only non-disabled combobox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -4163,7 +4163,7 @@ describe('Keyboard interactions', () => { 'should have no active combobox option upon PageUp key press, when there are no non-disabled combobox options', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -4436,7 +4436,7 @@ describe('Mouse interactions', () => { 'should focus the Combobox.Input when we click the Combobox.Label', suppressConsoleLogs(async () => { render( - + console.log(x)}> Label Trigger @@ -4463,7 +4463,7 @@ describe('Mouse interactions', () => { 'should not focus the Combobox.Input when we right click the Combobox.Label', suppressConsoleLogs(async () => { render( - + console.log(x)}> Label Trigger @@ -4490,7 +4490,7 @@ describe('Mouse interactions', () => { 'should be possible to open the combobox on click', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -4530,7 +4530,7 @@ describe('Mouse interactions', () => { 'should not be possible to open the combobox on right click', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -4559,7 +4559,7 @@ describe('Mouse interactions', () => { 'should not be possible to open the combobox on click when the button is disabled', suppressConsoleLogs(async () => { render( - + console.log(x)} disabled> Trigger @@ -4592,7 +4592,7 @@ describe('Mouse interactions', () => { 'should be possible to open the combobox on click, and focus the selected option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -4635,7 +4635,7 @@ describe('Mouse interactions', () => { 'should be possible to close a combobox on click', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -4665,7 +4665,7 @@ describe('Mouse interactions', () => { 'should be a no-op when we click outside of a closed combobox', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -4694,7 +4694,7 @@ describe('Mouse interactions', () => { suppressConsoleLogs(async () => { render( <> - + console.log(x)}> Trigger @@ -4730,7 +4730,7 @@ describe('Mouse interactions', () => { suppressConsoleLogs(async () => { render(
- + console.log(x)}> Trigger @@ -4740,7 +4740,7 @@ describe('Mouse interactions', () => { - + console.log(x)}> Trigger @@ -4775,7 +4775,7 @@ describe('Mouse interactions', () => { 'should be possible to click outside of the combobox which should close the combobox (even if we press the combobox button)', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -4808,7 +4808,7 @@ describe('Mouse interactions', () => { let focusFn = jest.fn() render(
- + console.log(x)}> Trigger @@ -4848,7 +4848,7 @@ describe('Mouse interactions', () => { 'should be possible to hover an option and make it active', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -4881,7 +4881,7 @@ describe('Mouse interactions', () => { 'should be possible to hover an option and make it active when using `static`', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -4911,7 +4911,7 @@ describe('Mouse interactions', () => { 'should make a combobox option active when you move the mouse over it', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -4936,7 +4936,7 @@ describe('Mouse interactions', () => { 'should be a no-op when we move the mouse and the combobox option is already active', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -4967,7 +4967,7 @@ describe('Mouse interactions', () => { 'should be a no-op when we move the mouse and the combobox option is disabled', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -4994,7 +4994,7 @@ describe('Mouse interactions', () => { 'should not be possible to hover an option that is disabled', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -5024,7 +5024,7 @@ describe('Mouse interactions', () => { 'should be possible to mouse leave an option and make it inactive', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -5067,7 +5067,7 @@ describe('Mouse interactions', () => { 'should be possible to mouse leave a disabled option and be a no-op', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -5244,7 +5244,7 @@ describe('Mouse interactions', () => { 'should not be possible to focus a combobox option which is disabled', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -5274,7 +5274,7 @@ describe('Mouse interactions', () => { 'should be possible to hold the last active option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -5797,7 +5797,7 @@ describe('Form compatibility', () => { }} > - + console.log(x)} /> Trigger Pizza Delivery diff --git a/packages/@headlessui-react/src/types.ts b/packages/@headlessui-react/src/types.ts index 64bc16be2c..9a5e38ddf1 100644 --- a/packages/@headlessui-react/src/types.ts +++ b/packages/@headlessui-react/src/types.ts @@ -66,5 +66,7 @@ export type XOR = T | U extends __ ? (Without & U) | (Without & T) : T | U -export type ByComparator = (keyof T & string) | ((a: T, b: T) => boolean) +export type ByComparator = + | (T extends null ? string : keyof T & string) + | ((a: T, b: T) => boolean) export type EnsureArray = T extends any[] ? T : Expand[] From 3043aeda281a373a4da2791f08ff89a71a28c011 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 17:24:18 +0100 Subject: [PATCH 06/21] improve the internal types of the `Combobox` component --- .../src/components/combobox/combobox.tsx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 1c21981723..3bb0ac3fcb 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -409,7 +409,7 @@ function ComboboxFn a === z, + by = (a: TValue, z: TValue) => a === z, disabled = false, __demoMode = false, nullable = false, @@ -440,16 +440,18 @@ function ComboboxFn(null) let optionsRef = useRef<_Data['optionsRef']['current']>(null) + type TActualValue = true extends typeof multiple ? EnsureArray[number] : TValue let compare = useEvent( + // @ts-expect-error Eventually we'll want to tackle this, but for now this will do. typeof by === 'string' - ? (a, z) => { - let property = by as unknown as keyof TValue + ? (a: TActualValue, z: TActualValue) => { + let property = by as unknown as keyof TActualValue return a?.[property] === z?.[property] } : by ) - let isSelected: (value: unknown) => boolean = useCallback( + let isSelected: (value: TValue) => boolean = useCallback( (compareValue) => match(data.mode, { [ValueMode.Multi]: () => From bbb0df5268d128c72a29f65497278d55262f645a Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 17:25:10 +0100 Subject: [PATCH 07/21] improve the internal types of the `Disclosure` component --- .../src/components/disclosure/disclosure.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/@headlessui-react/src/components/disclosure/disclosure.tsx b/packages/@headlessui-react/src/components/disclosure/disclosure.tsx index fc7f19a142..8a368ff26f 100644 --- a/packages/@headlessui-react/src/components/disclosure/disclosure.tsx +++ b/packages/@headlessui-react/src/components/disclosure/disclosure.tsx @@ -252,7 +252,10 @@ type ButtonPropsWeControl = 'aria-controls' | 'aria-expanded' export type DisclosureButtonProps = Props< TTag, ButtonRenderPropArg, - ButtonPropsWeControl + ButtonPropsWeControl, + { + disabled?: boolean + } > function ButtonFn( From 9c741853530d137772693769b8a377af0da86571 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 17:28:39 +0100 Subject: [PATCH 08/21] improve the internal types of the `Listbox` component --- .../src/components/listbox/listbox.test.tsx | 212 +++++++++--------- .../src/components/listbox/listbox.tsx | 8 +- 2 files changed, 110 insertions(+), 110 deletions(-) diff --git a/packages/@headlessui-react/src/components/listbox/listbox.test.tsx b/packages/@headlessui-react/src/components/listbox/listbox.test.tsx index cd0b90b1ec..8ed46da4a8 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.test.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.test.tsx @@ -68,7 +68,7 @@ describe('safeguards', () => { 'should be possible to render a Listbox without crashing', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -93,7 +93,7 @@ describe('Rendering', () => { 'should be possible to render a Listbox using a render prop', suppressConsoleLogs(async () => { render( - + console.log(x)}> {({ open }) => ( <> Trigger @@ -129,7 +129,7 @@ describe('Rendering', () => { 'should be possible to disable a Listbox', suppressConsoleLogs(async () => { render( - + console.log(x)} disabled> Trigger Option A @@ -197,7 +197,7 @@ describe('Rendering', () => { 'should use object equality by default', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger {options.map((option) => ( @@ -227,7 +227,7 @@ describe('Rendering', () => { 'should be possible to compare objects by a field', suppressConsoleLogs(async () => { render( - + console.log(x)} by="id"> Trigger {options.map((option) => ( @@ -259,7 +259,7 @@ describe('Rendering', () => { render( console.log(x)} by={(a, z) => a.id === z.id} > Trigger @@ -461,7 +461,7 @@ describe('Rendering', () => { 'null should be a valid value for the Listbox', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -487,7 +487,7 @@ describe('Rendering', () => { 'should be possible to render a Listbox.Label using a render prop', suppressConsoleLogs(async () => { render( - + console.log(x)}> {JSON.stringify} Trigger @@ -524,7 +524,7 @@ describe('Rendering', () => { 'should be possible to render a Listbox.Label using a render prop and an `as` prop', suppressConsoleLogs(async () => { render( - + console.log(x)}> {JSON.stringify} Trigger @@ -558,7 +558,7 @@ describe('Rendering', () => { 'should be possible to render a Listbox.Button using a render prop', suppressConsoleLogs(async () => { render( - + console.log(x)}> {JSON.stringify} Option A @@ -590,7 +590,7 @@ describe('Rendering', () => { 'should be possible to render a Listbox.Button using a render prop and an `as` prop', suppressConsoleLogs(async () => { render( - + console.log(x)}> {JSON.stringify} @@ -624,7 +624,7 @@ describe('Rendering', () => { 'should be possible to render a Listbox.Button and a Listbox.Label and see them linked together', suppressConsoleLogs(async () => { render( - + console.log(x)}> Label Trigger @@ -650,7 +650,7 @@ describe('Rendering', () => { describe('`type` attribute', () => { it('should set the `type` to "button" by default', async () => { render( - + console.log(x)}> Trigger ) @@ -660,7 +660,7 @@ describe('Rendering', () => { it('should not set the `type` to "button" if it already contains a `type`', async () => { render( - + console.log(x)}> Trigger ) @@ -674,7 +674,7 @@ describe('Rendering', () => { )) render( - + console.log(x)}> Trigger ) @@ -684,7 +684,7 @@ describe('Rendering', () => { it('should not set the type if the "as" prop is not a "button"', async () => { render( - + console.log(x)}> Trigger ) @@ -698,7 +698,7 @@ describe('Rendering', () => { )) render( - + console.log(x)}> Trigger ) @@ -713,7 +713,7 @@ describe('Rendering', () => { 'should be possible to render Listbox.Options using a render prop', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger {(data) => ( @@ -747,7 +747,7 @@ describe('Rendering', () => { it('should be possible to always render the Listbox.Options if we provide it a `static` prop', () => { render( - + console.log(x)}> Trigger Option A @@ -763,7 +763,7 @@ describe('Rendering', () => { it('should be possible to use a different render strategy for the Listbox.Options', async () => { render( - + console.log(x)}> Trigger Option A @@ -787,7 +787,7 @@ describe('Rendering', () => { 'should be possible to render a Listbox.Option using a render prop', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger {JSON.stringify} @@ -819,7 +819,7 @@ describe('Rendering', () => { function Example({ hide = false }) { return ( <> - + console.log(x)}> Trigger Option 1 @@ -1163,7 +1163,7 @@ describe('Rendering composition', () => { 'should be possible to conditionally render classNames (aka className can be a function?!)', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger JSON.stringify(bag)}> @@ -1238,7 +1238,7 @@ describe('Rendering composition', () => { 'should be possible to swap the Listbox option with a button for example', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -1285,7 +1285,7 @@ describe('Composition', () => { suppressConsoleLogs(async () => { let orderFn = jest.fn() render( - + console.log(x)}> Trigger @@ -1341,7 +1341,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the listbox with Enter', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -1387,7 +1387,7 @@ describe('Keyboard interactions', () => { 'should not be possible to open the listbox with Enter when the button is disabled', suppressConsoleLogs(async () => { render( - + console.log(x)} disabled> Trigger Option A @@ -1422,7 +1422,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the listbox with Enter, and focus the selected option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -1467,7 +1467,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the listbox with Enter, and focus the selected option (when using the `hidden` render strategy)', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -1534,7 +1534,7 @@ describe('Keyboard interactions', () => { ] let selectedOption = myOptions[1] render( - + console.log(x)}> Trigger {myOptions.map((myOption) => ( @@ -1581,7 +1581,7 @@ describe('Keyboard interactions', () => { 'should have no active listbox option when there are no listbox options at all', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -1605,7 +1605,7 @@ describe('Keyboard interactions', () => { 'should focus the first non disabled listbox option when opening with Enter', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -1640,7 +1640,7 @@ describe('Keyboard interactions', () => { 'should focus the first non disabled listbox option when opening with Enter (jump over multiple disabled ones)', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -1677,7 +1677,7 @@ describe('Keyboard interactions', () => { 'should have no active listbox option upon Enter key press, when there are no non-disabled listbox options', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -1713,7 +1713,7 @@ describe('Keyboard interactions', () => { 'should be possible to close the listbox with Enter when there is no active listboxoption', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -1819,7 +1819,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the listbox with Space', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -1862,7 +1862,7 @@ describe('Keyboard interactions', () => { 'should not be possible to open the listbox with Space when the button is disabled', suppressConsoleLogs(async () => { render( - + console.log(x)} disabled> Trigger Option A @@ -1897,7 +1897,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the listbox with Space, and focus the selected option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -1942,7 +1942,7 @@ describe('Keyboard interactions', () => { 'should have no active listbox option when there are no listbox options at all', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -1966,7 +1966,7 @@ describe('Keyboard interactions', () => { 'should focus the first non disabled listbox option when opening with Space', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2001,7 +2001,7 @@ describe('Keyboard interactions', () => { 'should focus the first non disabled listbox option when opening with Space (jump over multiple disabled ones)', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2038,7 +2038,7 @@ describe('Keyboard interactions', () => { 'should have no active listbox option upon Space key press, when there are no non-disabled listbox options', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2142,7 +2142,7 @@ describe('Keyboard interactions', () => { 'should be possible to close an open listbox with Escape', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -2185,7 +2185,7 @@ describe('Keyboard interactions', () => { 'should focus trap when we use Tab', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -2236,7 +2236,7 @@ describe('Keyboard interactions', () => { 'should focus trap when we use Shift+Tab', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -2289,7 +2289,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the listbox with ArrowDown', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -2334,7 +2334,7 @@ describe('Keyboard interactions', () => { 'should not be possible to open the listbox with ArrowDown when the button is disabled', suppressConsoleLogs(async () => { render( - + console.log(x)} disabled> Trigger Option A @@ -2369,7 +2369,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the listbox with ArrowDown, and focus the selected option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -2414,7 +2414,7 @@ describe('Keyboard interactions', () => { 'should have no active listbox option when there are no listbox options at all', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2438,7 +2438,7 @@ describe('Keyboard interactions', () => { 'should be possible to use ArrowDown to navigate the listbox options', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -2484,7 +2484,7 @@ describe('Keyboard interactions', () => { 'should be possible to use ArrowDown to navigate the listbox options and skip the first disabled one', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2524,7 +2524,7 @@ describe('Keyboard interactions', () => { 'should be possible to use ArrowDown to navigate the listbox options and jump to the first non-disabled one', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2564,7 +2564,7 @@ describe('Keyboard interactions', () => { 'should be possible to use ArrowRight to navigate the listbox options', suppressConsoleLogs(async () => { render( - + console.log(x)} horizontal> Trigger Option A @@ -2612,7 +2612,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the listbox with ArrowUp and the last option should be active', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -2657,7 +2657,7 @@ describe('Keyboard interactions', () => { 'should not be possible to open the listbox with ArrowUp and the last option should be active when the button is disabled', suppressConsoleLogs(async () => { render( - + console.log(x)} disabled> Trigger Option A @@ -2692,7 +2692,7 @@ describe('Keyboard interactions', () => { 'should be possible to open the listbox with ArrowUp, and focus the selected option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -2737,7 +2737,7 @@ describe('Keyboard interactions', () => { 'should have no active listbox option when there are no listbox options at all', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2761,7 +2761,7 @@ describe('Keyboard interactions', () => { 'should be possible to use ArrowUp to navigate the listbox options and jump to the first non-disabled one', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -2799,7 +2799,7 @@ describe('Keyboard interactions', () => { 'should not be possible to navigate up or down if there is only a single non-disabled option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -2845,7 +2845,7 @@ describe('Keyboard interactions', () => { 'should be possible to use ArrowUp to navigate the listbox options', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -2902,7 +2902,7 @@ describe('Keyboard interactions', () => { 'should be possible to use ArrowLeft to navigate the listbox options', suppressConsoleLogs(async () => { render( - + console.log(x)} horizontal> Trigger Option A @@ -2960,7 +2960,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the End key to go to the last listbox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -2991,7 +2991,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the End key to go to the last non disabled listbox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -3027,7 +3027,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the End key to go to the first listbox option if that is the only non-disabled listbox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -3062,7 +3062,7 @@ describe('Keyboard interactions', () => { 'should have no active listbox option upon End key press, when there are no non-disabled listbox options', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3100,7 +3100,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the PageDown key to go to the last listbox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -3131,7 +3131,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the PageDown key to go to the last non disabled listbox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -3167,7 +3167,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the PageDown key to go to the first listbox option if that is the only non-disabled listbox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -3202,7 +3202,7 @@ describe('Keyboard interactions', () => { 'should have no active listbox option upon PageDown key press, when there are no non-disabled listbox options', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3240,7 +3240,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the Home key to go to the first listbox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -3271,7 +3271,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the Home key to go to the first non disabled listbox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3306,7 +3306,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the Home key to go to the last listbox option if that is the only non-disabled listbox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3341,7 +3341,7 @@ describe('Keyboard interactions', () => { 'should have no active listbox option upon Home key press, when there are no non-disabled listbox options', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3379,7 +3379,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the PageUp key to go to the first listbox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -3410,7 +3410,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the PageUp key to go to the first non disabled listbox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3445,7 +3445,7 @@ describe('Keyboard interactions', () => { 'should be possible to use the PageUp key to go to the last listbox option if that is the only non-disabled listbox option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3480,7 +3480,7 @@ describe('Keyboard interactions', () => { 'should have no active listbox option upon PageUp key press, when there are no non-disabled listbox options', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger @@ -3518,7 +3518,7 @@ describe('Keyboard interactions', () => { 'should be possible to type a full word that has a perfect match', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger alice @@ -3551,7 +3551,7 @@ describe('Keyboard interactions', () => { 'should be possible to type a partial of a word', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger alice @@ -3590,7 +3590,7 @@ describe('Keyboard interactions', () => { 'should be possible to type words with spaces', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger value a @@ -3629,7 +3629,7 @@ describe('Keyboard interactions', () => { 'should not be possible to search for a disabled option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger alice @@ -3664,7 +3664,7 @@ describe('Keyboard interactions', () => { 'should be possible to search for a word (case insensitive)', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger alice @@ -3697,7 +3697,7 @@ describe('Keyboard interactions', () => { 'should be possible to search for the next occurence', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger alice @@ -3731,7 +3731,7 @@ describe('Keyboard interactions', () => { 'should stay on the same item while keystrokes still match', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger alice @@ -3807,7 +3807,7 @@ describe('Mouse interactions', () => { 'should focus the Listbox.Button when we click the Listbox.Label', suppressConsoleLogs(async () => { render( - + console.log(x)}> Label Trigger @@ -3833,7 +3833,7 @@ describe('Mouse interactions', () => { 'should not focus the Listbox.Button when we right click the Listbox.Label', suppressConsoleLogs(async () => { render( - + console.log(x)}> Label Trigger @@ -3859,7 +3859,7 @@ describe('Mouse interactions', () => { 'should be possible to open the listbox on click', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -3898,7 +3898,7 @@ describe('Mouse interactions', () => { 'should not be possible to open the listbox on right click', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Item A @@ -3926,7 +3926,7 @@ describe('Mouse interactions', () => { 'should not be possible to open the listbox on click when the button is disabled', suppressConsoleLogs(async () => { render( - + console.log(x)} disabled> Trigger Option A @@ -3958,7 +3958,7 @@ describe('Mouse interactions', () => { 'should be possible to open the listbox on click, and focus the selected option', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -4000,7 +4000,7 @@ describe('Mouse interactions', () => { 'should be possible to close a listbox on click', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger Option A @@ -4029,7 +4029,7 @@ describe('Mouse interactions', () => { 'should be a no-op when we click outside of a closed listbox', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger alice @@ -4054,7 +4054,7 @@ describe('Mouse interactions', () => { 'should be possible to click outside of the listbox which should close the listbox', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger alice @@ -4085,7 +4085,7 @@ describe('Mouse interactions', () => { suppressConsoleLogs(async () => { render(
- + console.log(x)}> Trigger alice @@ -4094,7 +4094,7 @@ describe('Mouse interactions', () => { - + console.log(x)}> Trigger alice @@ -4128,7 +4128,7 @@ describe('Mouse interactions', () => { 'should be possible to click outside of the listbox which should close the listbox (even if we press the listbox button)', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger alice @@ -4160,7 +4160,7 @@ describe('Mouse interactions', () => { let focusFn = jest.fn() render(
- + console.log(x)}> Trigger alice @@ -4199,7 +4199,7 @@ describe('Mouse interactions', () => { 'should be possible to hover an option and make it active', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger alice @@ -4231,7 +4231,7 @@ describe('Mouse interactions', () => { 'should make a listbox option active when you move the mouse over it', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger alice @@ -4255,7 +4255,7 @@ describe('Mouse interactions', () => { 'should be a no-op when we move the mouse and the listbox option is already active', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger alice @@ -4285,7 +4285,7 @@ describe('Mouse interactions', () => { 'should be a no-op when we move the mouse and the listbox option is disabled', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger alice @@ -4311,7 +4311,7 @@ describe('Mouse interactions', () => { 'should not be possible to hover an option that is disabled', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger alice @@ -4340,7 +4340,7 @@ describe('Mouse interactions', () => { 'should be possible to mouse leave an option and make it inactive', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger alice @@ -4382,7 +4382,7 @@ describe('Mouse interactions', () => { 'should be possible to mouse leave a disabled option and be a no-op', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger alice @@ -4516,7 +4516,7 @@ describe('Mouse interactions', () => { 'should be possible focus a listbox option, so that it becomes active', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger alice @@ -4546,7 +4546,7 @@ describe('Mouse interactions', () => { 'should not be possible to focus a listbox option which is disabled', suppressConsoleLogs(async () => { render( - + console.log(x)}> Trigger alice diff --git a/packages/@headlessui-react/src/components/listbox/listbox.tsx b/packages/@headlessui-react/src/components/listbox/listbox.tsx index 3cd3223cca..a7249f328e 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.tsx @@ -357,7 +357,7 @@ function ListboxFn< defaultValue, name, onChange: controlledOnChange, - by = (a, z) => a === z, + by = (a: TActualType, z: TActualType) => a === z, disabled = false, horizontal = false, multiple = false, @@ -366,7 +366,7 @@ function ListboxFn< const orientation = horizontal ? 'horizontal' : 'vertical' let listboxRef = useSyncRefs(ref) - let [value = multiple ? [] : undefined, theirOnChange] = useControllable( + let [value = multiple ? [] : undefined, theirOnChange] = useControllable( controlledValue, controlledOnChange, defaultValue @@ -397,12 +397,12 @@ function ListboxFn< : by ) - let isSelected: (value: unknown) => boolean = useCallback( + let isSelected: (value: TActualType) => boolean = useCallback( (compareValue) => match(data.mode, { [ValueMode.Multi]: () => (value as unknown as EnsureArray).some((option) => compare(option, compareValue)), - [ValueMode.Single]: () => compare(value as TType, compareValue), + [ValueMode.Single]: () => compare(value as TActualType, compareValue), }), [value] ) From 8007cedf217164ce8cd726dcdc28e6332b4212dc Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 17:29:31 +0100 Subject: [PATCH 09/21] improve the internal types of the `Menu` component --- packages/@headlessui-react/src/components/menu/menu.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/@headlessui-react/src/components/menu/menu.tsx b/packages/@headlessui-react/src/components/menu/menu.tsx index 5ec628cdf6..00077db0cd 100644 --- a/packages/@headlessui-react/src/components/menu/menu.tsx +++ b/packages/@headlessui-react/src/components/menu/menu.tsx @@ -313,7 +313,10 @@ type ButtonPropsWeControl = 'aria-controls' | 'aria-expanded' | 'aria-haspopup' export type MenuButtonProps = Props< TTag, ButtonRenderPropArg, - ButtonPropsWeControl + ButtonPropsWeControl, + { + disabled?: boolean + } > function ButtonFn( From 70f40c05be26beea9f53d5bc2495be36c8c2a33f Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 17:31:36 +0100 Subject: [PATCH 10/21] improve the internal types of the `Popover` component --- .../@headlessui-react/src/components/popover/popover.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/@headlessui-react/src/components/popover/popover.tsx b/packages/@headlessui-react/src/components/popover/popover.tsx index 1fa305afdf..61b2cf083e 100644 --- a/packages/@headlessui-react/src/components/popover/popover.tsx +++ b/packages/@headlessui-react/src/components/popover/popover.tsx @@ -390,7 +390,10 @@ type ButtonPropsWeControl = 'aria-controls' | 'aria-expanded' export type PopoverButtonProps = Props< TTag, ButtonRenderPropArg, - ButtonPropsWeControl + ButtonPropsWeControl, + { + disabled?: boolean + } > function ButtonFn( From c071a5294324e44316eb8134af3c3fcd6326f55f Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 17:43:27 +0100 Subject: [PATCH 11/21] improve the internal types of the `Tabs` component --- .../src/components/tabs/tabs.tsx | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/packages/@headlessui-react/src/components/tabs/tabs.tsx b/packages/@headlessui-react/src/components/tabs/tabs.tsx index 873a8836a3..21976565b5 100644 --- a/packages/@headlessui-react/src/components/tabs/tabs.tsx +++ b/packages/@headlessui-react/src/components/tabs/tabs.tsx @@ -209,13 +209,18 @@ interface TabsRenderPropArg { selectedIndex: number } -export type TabGroupProps = Props & { - defaultIndex?: number - onChange?: (index: number) => void - selectedIndex?: number - vertical?: boolean - manual?: boolean -} +export type TabGroupProps = Props< + TTag, + TabsRenderPropArg, + never, + { + defaultIndex?: number + onChange?: (index: number) => void + selectedIndex?: number + vertical?: boolean + manual?: boolean + } +> function GroupFn( props: TabGroupProps, @@ -334,10 +339,11 @@ type ListPropsWeControl = 'aria-orientation' | 'role' export type TabListProps = Props< TTag, ListRenderPropArg, - ListPropsWeControl -> & { - // -} + ListPropsWeControl, + { + // + } +> function ListFn( props: TabListProps, @@ -545,9 +551,9 @@ let PanelRenderFeatures = Features.RenderStrategy | Features.Static export type TabPanelProps = Props< TTag, PanelRenderPropArg, - PanelPropsWeControl -> & - PropsForFeatures + PanelPropsWeControl, + PropsForFeatures & { id?: string; tabIndex?: number } +> function PanelFn( props: TabPanelProps, From 459c2f947be188e712081794474c7af709b78efd Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 17:52:22 +0100 Subject: [PATCH 12/21] improve the internal types of the `Transition` component --- .../src/components/transitions/transition.tsx | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/@headlessui-react/src/components/transitions/transition.tsx b/packages/@headlessui-react/src/components/transitions/transition.tsx index 79dbde1cb6..fe20bf69a8 100644 --- a/packages/@headlessui-react/src/components/transitions/transition.tsx +++ b/packages/@headlessui-react/src/components/transitions/transition.tsx @@ -74,13 +74,14 @@ export interface TransitionEvents { afterLeave?: () => void } -export type TransitionChildProps = Omit< - Props, - 'ref' -> & +export type TransitionChildProps = Props< + TTag, + TransitionChildRenderPropArg, + never, PropsForFeatures & - TransitionClasses & - TransitionEvents & { appear?: boolean } + TransitionClasses & + TransitionEvents & { appear?: boolean } +> function useTransitionContext() { let context = useContext(TransitionContext) @@ -573,8 +574,10 @@ function ChildFn return ( <> {!hasTransitionContext && hasOpenClosedContext ? ( + // @ts-expect-error This is an object ) : ( + // @ts-expect-error This is an object )} From 17a77ef34de883fb29184e124d193940fd8ec838 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 17:43:18 +0100 Subject: [PATCH 13/21] use `Override` in `Hidden` as well --- packages/@headlessui-react/src/internal/hidden.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/@headlessui-react/src/internal/hidden.tsx b/packages/@headlessui-react/src/internal/hidden.tsx index 96a981e0ae..c0d922dd50 100644 --- a/packages/@headlessui-react/src/internal/hidden.tsx +++ b/packages/@headlessui-react/src/internal/hidden.tsx @@ -15,9 +15,7 @@ export enum Features { Hidden = 1 << 2, } -export type HiddenProps = Props & { - features?: Features -} +export type HiddenProps = Props function VisuallyHidden( props: HiddenProps, From 0677ab380abfebfe19949fda4938c027bc6becd1 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 18:02:16 +0100 Subject: [PATCH 14/21] cleanup unused code --- packages/@headlessui-react/src/test-utils/interactions.ts | 3 --- packages/@headlessui-vue/src/test-utils/interactions.ts | 3 --- 2 files changed, 6 deletions(-) diff --git a/packages/@headlessui-react/src/test-utils/interactions.ts b/packages/@headlessui-react/src/test-utils/interactions.ts index acdefb5180..747ef523fa 100644 --- a/packages/@headlessui-react/src/test-utils/interactions.ts +++ b/packages/@headlessui-react/src/test-utils/interactions.ts @@ -1,9 +1,6 @@ import { fireEvent } from '@testing-library/react' -import { disposables } from '../utils/disposables' import { pointer } from './fake-pointer' -let d = disposables() - function nextFrame(cb: Function): void { setImmediate(() => { setImmediate(() => { diff --git a/packages/@headlessui-vue/src/test-utils/interactions.ts b/packages/@headlessui-vue/src/test-utils/interactions.ts index 0780ee89b4..57c2ac7f32 100644 --- a/packages/@headlessui-vue/src/test-utils/interactions.ts +++ b/packages/@headlessui-vue/src/test-utils/interactions.ts @@ -1,9 +1,6 @@ import { fireEvent } from '@testing-library/dom' -import { disposables } from '../utils/disposables' import { pointer } from './fake-pointer' -let d = disposables() - function nextFrame(cb: Function): void { setImmediate(() => setImmediate(() => { From 4e814564e5362663dda9e4822af00d29bd4bc44c Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 18:03:00 +0100 Subject: [PATCH 15/21] don't check the `useSyncExternalStoreShimClient` --- .../useSyncExternalStoreShimClient.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/@headlessui-react/src/use-sync-external-store-shim/useSyncExternalStoreShimClient.ts b/packages/@headlessui-react/src/use-sync-external-store-shim/useSyncExternalStoreShimClient.ts index 58f967ce72..2ebb849344 100644 --- a/packages/@headlessui-react/src/use-sync-external-store-shim/useSyncExternalStoreShimClient.ts +++ b/packages/@headlessui-react/src/use-sync-external-store-shim/useSyncExternalStoreShimClient.ts @@ -1,3 +1,5 @@ +// @ts-nocheck + /** * Copyright (c) Facebook, Inc. and its affiliates. * From 7991ec5136d2d550482444c15ebff4d259ed9340 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 18:04:29 +0100 Subject: [PATCH 16/21] don't check the `useSyncExternalStoreShimServer` --- .../useSyncExternalStoreShimServer.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/@headlessui-react/src/use-sync-external-store-shim/useSyncExternalStoreShimServer.ts b/packages/@headlessui-react/src/use-sync-external-store-shim/useSyncExternalStoreShimServer.ts index 96b8dbc554..2d550daec5 100644 --- a/packages/@headlessui-react/src/use-sync-external-store-shim/useSyncExternalStoreShimServer.ts +++ b/packages/@headlessui-react/src/use-sync-external-store-shim/useSyncExternalStoreShimServer.ts @@ -1,3 +1,5 @@ +// @ts-nocheck + /** * Copyright (c) Facebook, Inc. and its affiliates. * From 4a027e3ab834864d9fd147eebc3c806b48b288bf Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 18:07:15 +0100 Subject: [PATCH 17/21] improve types in the render tests --- packages/@headlessui-react/src/utils/render.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@headlessui-react/src/utils/render.test.tsx b/packages/@headlessui-react/src/utils/render.test.tsx index 64f40a85f2..336197198d 100644 --- a/packages/@headlessui-react/src/utils/render.test.tsx +++ b/packages/@headlessui-react/src/utils/render.test.tsx @@ -68,12 +68,12 @@ describe('Default functionality', () => { }) it('should be possible to add a ref with a different name', () => { - let ref = createRef() + let ref = createRef() function MyComponent({ innerRef, ...props - }: Props & { innerRef: Ref }) { + }: Props }>) { return
} From 8508fafa29166206fbf0ed03564af799b385c120 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 18:38:00 +0100 Subject: [PATCH 18/21] fix `Ref` to be `Ref` --- .../src/components/combobox/combobox.tsx | 10 +++++----- .../src/components/disclosure/disclosure.tsx | 2 +- .../src/components/listbox/listbox.tsx | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 3bb0ac3fcb..0302d2cc3f 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -385,24 +385,24 @@ export type ComboboxProps< function ComboboxFn( props: ComboboxProps, - ref: Ref + ref: Ref ): JSX.Element function ComboboxFn( props: ComboboxProps, - ref: Ref + ref: Ref ): JSX.Element function ComboboxFn( props: ComboboxProps, - ref: Ref + ref: Ref ): JSX.Element function ComboboxFn( props: ComboboxProps, - ref: Ref + ref: Ref ): JSX.Element function ComboboxFn( props: ComboboxProps, - ref: Ref + ref: Ref ) { let { value: controlledValue, diff --git a/packages/@headlessui-react/src/components/disclosure/disclosure.tsx b/packages/@headlessui-react/src/components/disclosure/disclosure.tsx index 8a368ff26f..22f9329d3e 100644 --- a/packages/@headlessui-react/src/components/disclosure/disclosure.tsx +++ b/packages/@headlessui-react/src/components/disclosure/disclosure.tsx @@ -162,7 +162,7 @@ export type DisclosureProps = Props( props: DisclosureProps, - ref: Ref + ref: Ref ) { let { defaultOpen = false, ...theirProps } = props let internalDisclosureRef = useRef(null) diff --git a/packages/@headlessui-react/src/components/listbox/listbox.tsx b/packages/@headlessui-react/src/components/listbox/listbox.tsx index a7249f328e..db0b36a5be 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.tsx @@ -351,7 +351,7 @@ function ListboxFn< TTag extends ElementType = typeof DEFAULT_LISTBOX_TAG, TType = string, TActualType = TType extends (infer U)[] ? U : TType ->(props: ListboxProps, ref: Ref) { +>(props: ListboxProps, ref: Ref) { let { value: controlledValue, defaultValue, From 2d0094322597e6fe4fc1213cac61e93372c208d5 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 22:24:04 +0100 Subject: [PATCH 19/21] improve internal types of the `Transition` component (Vue) + add `attrs.class` as well --- .../src/components/transitions/transition.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/@headlessui-vue/src/components/transitions/transition.ts b/packages/@headlessui-vue/src/components/transitions/transition.ts index 119dbcf065..64e2e88100 100644 --- a/packages/@headlessui-vue/src/components/transitions/transition.ts +++ b/packages/@headlessui-vue/src/components/transitions/transition.ts @@ -357,7 +357,13 @@ export let TransitionChild = defineComponent({ ...(appear && show && env.isServer ? { // Already apply the `enter` and `enterFrom` on the server if required - class: normalizeClass([rest.class, ...enterClasses, ...enterFromClasses]), + class: normalizeClass([ + attrs.class, + // @ts-expect-error not explicitly defined + rest.class, + ...enterClasses, + ...enterFromClasses, + ]), } : {}), } From 7355460abc3afcd0954b79cc73b78963bbc91f1b Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 22:27:00 +0100 Subject: [PATCH 20/21] use different type for `AnyComponent` --- .../@headlessui-vue/src/test-utils/vue-testing-library.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@headlessui-vue/src/test-utils/vue-testing-library.ts b/packages/@headlessui-vue/src/test-utils/vue-testing-library.ts index 8a65b62095..fa91d06cf7 100644 --- a/packages/@headlessui-vue/src/test-utils/vue-testing-library.ts +++ b/packages/@headlessui-vue/src/test-utils/vue-testing-library.ts @@ -1,6 +1,6 @@ import { mount } from '@vue/test-utils' import { logDOM, fireEvent, screen } from '@testing-library/dom' -import { DefineComponent, ComponentOptionsWithoutProps, defineComponent } from 'vue' +import { ComponentOptionsWithoutProps, defineComponent } from 'vue' let mountedWrappers = new Set() @@ -16,7 +16,7 @@ function resolveContainer(): HTMLElement { // It's not the most elegant type // but Props and Emits need to be typed as any and not `{}` -type AnyComponent = DefineComponent +type AnyComponent = ReturnType export function createRenderTemplate(defaultComponents: Record) { return (input: string | ComponentOptionsWithoutProps) => { From 478c18eda5f4e0b1d77c4872606b657f5c742693 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Mar 2023 18:41:42 +0100 Subject: [PATCH 21/21] update changelog --- packages/@headlessui-react/CHANGELOG.md | 1 + packages/@headlessui-vue/CHANGELOG.md | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index 7f224379fd..9d3e9edff2 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Ensure `Transition` component completes if nothing is transitioning ([#2318](https://github.com/tailwindlabs/headlessui/pull/2318)) - Enable native label behavior for `` where possible ([#2265](https://github.com/tailwindlabs/headlessui/pull/2265)) - Allow root containers from the `Dialog` component in the `FocusTrap` component ([#2322](https://github.com/tailwindlabs/headlessui/pull/2322)) +- Fix `XYZPropsWeControl` and cleanup internal TypeScript types ([#2329](https://github.com/tailwindlabs/headlessui/pull/2329)) ## [1.7.12] - 2023-02-24 diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index 7f87bb29e4..03c6b4a7e2 100644 --- a/packages/@headlessui-vue/CHANGELOG.md +++ b/packages/@headlessui-vue/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Enable native label behavior for `` where possible ([#2265](https://github.com/tailwindlabs/headlessui/pull/2265)) - Allow root containers from the `Dialog` component in the `FocusTrap` component ([#2322](https://github.com/tailwindlabs/headlessui/pull/2322)) +- Cleanup internal TypeScript types ([#2329](https://github.com/tailwindlabs/headlessui/pull/2329)) ## [1.7.11] - 2023-02-24