Improve the types of the Combobox component (#1761)

* improve types of `Combobox` Now given the `multiple` and/or `nullable` props we ensure that the types for the `value`, `defaultValue`, `onChange`, `by`, render prop, ... are all correct. You will also be able to easily tell which type to use instead of inferring it by doing something like this: ```tsx <Combobox<ExplicitTypeHere> value={...} onChange={...} ... > ... </Combobox> ``` * update changelog
tailwindlabs · Aug 11, 2022 · 486ac80 · 486ac80
1 parent b28d177
commit 486ac80
Show file tree

Hide file tree

Showing 6 changed files with 140 additions and 61 deletions.
diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md
@@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Improve `Combobox` re-opening keyboard issue on mobile ([#1732](https://github.com/tailwindlabs/headlessui/pull/1732))
 - Ensure `Disclosure.Panel` is properly linked ([#1747](https://github.com/tailwindlabs/headlessui/pull/1747))
 - Only select the active option when using "singular" mode when pressing `<tab>` in the `Combobox` component ([#1750](https://github.com/tailwindlabs/headlessui/pull/1750))
+- Improve the types of the `Combobox` component ([#1761](https://github.com/tailwindlabs/headlessui/pull/1761))
 
 ## Changed
 

diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx
@@ -280,7 +280,7 @@ describe('Rendering', () => {
             let [value, setValue] = useState({ id: 2, name: 'Bob' })
 
             return (
-              <Combobox value={value} onChange={setValue} by="id">
+              <Combobox value={value} onChange={(value) => setValue(value)} by="id">
                 <Combobox.Button>Trigger</Combobox.Button>
                 <Combobox.Options>
                   <Combobox.Option value={{ id: 1, name: 'alice' }}>alice</Combobox.Option>
@@ -322,7 +322,7 @@ describe('Rendering', () => {
             let [value, setValue] = useState([{ id: 2, name: 'Bob' }])
 
             return (
-              <Combobox value={value} onChange={setValue} by="id" multiple>
+              <Combobox value={value} onChange={(value) => setValue(value)} by="id" multiple>
                 <Combobox.Button>Trigger</Combobox.Button>
                 <Combobox.Options>
                   <Combobox.Option value={{ id: 1, name: 'alice' }}>alice</Combobox.Option>
@@ -2231,7 +2231,7 @@ describe('Keyboard interactions', () => {
       suppressConsoleLogs(async () => {
         let handleChange = jest.fn()
         function Example() {
-          let [value, setValue] = useState<string>('bob')
+          let [value, setValue] = useState<string | null>('bob')
           let [, setQuery] = useState<string>('')
 
           return (
@@ -5095,7 +5095,7 @@ describe('Multi-select', () => {
         let [value, setValue] = useState<string[]>(['bob', 'charlie'])
 
         return (
-          <Combobox value={value} onChange={setValue} multiple>
+          <Combobox value={value} onChange={(value) => setValue(value)} multiple>
             <Combobox.Input onChange={() => {}} />
             <Combobox.Button>Trigger</Combobox.Button>
             <Combobox.Options>
@@ -5131,7 +5131,7 @@ describe('Multi-select', () => {
         let [value, setValue] = useState<string[]>(['bob', 'charlie'])
 
         return (
-          <Combobox value={value} onChange={setValue} multiple>
+          <Combobox value={value} onChange={(value) => setValue(value)} multiple>
             <Combobox.Input onChange={() => {}} />
             <Combobox.Button>Trigger</Combobox.Button>
             <Combobox.Options>
@@ -5160,7 +5160,7 @@ describe('Multi-select', () => {
         let [value, setValue] = useState<string[]>(['bob', 'charlie'])
 
         return (
-          <Combobox value={value} onChange={setValue} multiple>
+          <Combobox value={value} onChange={(value) => setValue(value)} multiple>
             <Combobox.Input onChange={() => {}} />
             <Combobox.Button>Trigger</Combobox.Button>
             <Combobox.Options>
@@ -5193,7 +5193,7 @@ describe('Multi-select', () => {
         let [value, setValue] = useState<string[]>(['bob', 'charlie'])
 
         return (
-          <Combobox value={value} onChange={setValue} multiple>
+          <Combobox value={value} onChange={(value) => setValue(value)} multiple>
             <Combobox.Input onChange={() => {}} />
             <Combobox.Button>Trigger</Combobox.Button>
             <Combobox.Options>

diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx
@@ -15,7 +15,7 @@ import React, {
   MutableRefObject,
   Ref,
 } from 'react'
-import { Props } from '../../types'
+import { ByComparator, EnsureArray, Expand, Props } from '../../types'
 
 import { useComputed } from '../../hooks/use-computed'
 import { useDisposables } from '../../hooks/use-disposables'
@@ -303,49 +303,106 @@ interface ComboboxRenderPropArg<T> {
   value: T
 }
 
-let ComboboxRoot = forwardRefWithAs(function Combobox<
-  TTag extends ElementType = typeof DEFAULT_COMBOBOX_TAG,
-  TType = string,
-  TActualType = TType extends (infer U)[] ? U : TType
->(
-  props: Props<
-    TTag,
-    ComboboxRenderPropArg<TType>,
-    'value' | 'defaultValue' | 'onChange' | 'by' | 'disabled' | 'name' | 'nullable' | 'multiple'
-  > & {
-    value?: TType
-    defaultValue?: TType
-    onChange?(value: TType): void
-    by?: (keyof TActualType & string) | ((a: TActualType, z: TActualType) => boolean)
-    disabled?: boolean
-    __demoMode?: boolean
-    name?: string
-    nullable?: boolean
-    multiple?: boolean
-  },
+type O = 'value' | 'defaultValue' | 'nullable' | 'multiple' | 'onChange' | 'by'
+
+type ComboboxValueProps<
+  TValue,
+  TNullable extends boolean | undefined,
+  TMultiple extends boolean | undefined,
+  TTag extends ElementType
+> = Extract<
+  | ({
+      value?: EnsureArray<TValue>
+      defaultValue?: EnsureArray<TValue>
+      nullable: true // We ignore `nullable` in multiple mode
+      multiple: true
+      onChange?(value: EnsureArray<TValue>): void
+      by?: ByComparator<TValue>
+    } & Props<TTag, ComboboxRenderPropArg<EnsureArray<TValue>>, O>)
+  | ({
+      value?: TValue | null
+      defaultValue?: TValue | null
+      nullable: true
+      multiple?: false
+      onChange?(value: TValue | null): void
+      by?: ByComparator<TValue | null>
+    } & Expand<Props<TTag, ComboboxRenderPropArg<TValue | null>, O>>)
+  | ({
+      value?: EnsureArray<TValue>
+      defaultValue?: EnsureArray<TValue>
+      nullable?: false
+      multiple: true
+      onChange?(value: EnsureArray<TValue>): void
+      by?: ByComparator<TValue extends Array<infer U> ? U : TValue>
+    } & Expand<Props<TTag, ComboboxRenderPropArg<EnsureArray<TValue>>, O>>)
+  | ({
+      value?: TValue
+      nullable?: false
+      multiple?: false
+      defaultValue?: TValue
+      onChange?(value: TValue): void
+      by?: ByComparator<TValue>
+    } & Props<TTag, ComboboxRenderPropArg<TValue>, O>),
+  { nullable?: TNullable; multiple?: TMultiple }
+>
+
+type ComboboxProps<
+  TValue,
+  TNullable extends boolean | undefined,
+  TMultiple extends boolean | undefined,
+  TTag extends ElementType
+> = ComboboxValueProps<TValue, TNullable, TMultiple, TTag> & {
+  disabled?: boolean
+  __demoMode?: boolean
+  name?: string
+}
+
+function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_TAG>(
+  props: ComboboxProps<TValue, true, true, TTag>,
+  ref: Ref<TTag>
+): JSX.Element
+function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_TAG>(
+  props: ComboboxProps<TValue, true, false, TTag>,
+  ref: Ref<TTag>
+): JSX.Element
+function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_TAG>(
+  props: ComboboxProps<TValue, false, false, TTag>,
+  ref: Ref<TTag>
+): JSX.Element
+function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_TAG>(
+  props: ComboboxProps<TValue, false, true, TTag>,
+  ref: Ref<TTag>
+): JSX.Element
+
+function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_TAG>(
+  props: ComboboxProps<TValue, boolean | undefined, boolean | undefined, TTag>,
   ref: Ref<TTag>
 ) {
   let {
     value: controlledValue,
     defaultValue,
     onChange: controlledOnChange,
     name,
-    by = (a, z) => a === z,
+    by = (a: any, z: any) => a === z,
     disabled = false,
     __demoMode = false,
     nullable = false,
     multiple = false,
     ...theirProps
   } = props
-  let [value, theirOnChange] = useControllable(controlledValue, controlledOnChange, defaultValue)
+  let [value, theirOnChange] = useControllable<any>(
+    controlledValue,
+    controlledOnChange,
+    defaultValue
+  )
 
   let [state, dispatch] = useReducer(stateReducer, {
     dataRef: createRef(),
     comboboxState: __demoMode ? ComboboxState.Open : ComboboxState.Closed,
     options: [],
     activeOptionIndex: null,
     activationTrigger: ActivationTrigger.Other,
-  } as StateDefinition<TType>)
+  } as StateDefinition<TValue>)
 
   let defaultToFirstOption = useRef(false)
 
@@ -358,19 +415,19 @@ let ComboboxRoot = forwardRefWithAs(function Combobox<
 
   let compare = useEvent(
     typeof by === 'string'
-      ? (a: TActualType, z: TActualType) => {
-          let property = by as unknown as keyof TActualType
+      ? (a, z) => {
+          let property = by as unknown as keyof TValue
           return a[property] === z[property]
         }
       : by
   )
 
-  let isSelected: (value: TActualType) => boolean = useCallback(
+  let isSelected: (value: unknown) => boolean = useCallback(
     (compareValue) =>
       match(data.mode, {
         [ValueMode.Multi]: () =>
-          (value as unknown as TActualType[]).some((option) => compare(option, compareValue)),
-        [ValueMode.Single]: () => compare(value as unknown as TActualType, compareValue),
+          (value as EnsureArray<TValue>).some((option) => compare(option, compareValue)),
+        [ValueMode.Single]: () => compare(value as TValue, compareValue),
       }),
     [value]
   )
@@ -422,15 +479,15 @@ let ComboboxRoot = forwardRefWithAs(function Combobox<
     data.comboboxState === ComboboxState.Open
   )
 
-  let slot = useMemo<ComboboxRenderPropArg<TType>>(
+  let slot = useMemo<ComboboxRenderPropArg<unknown>>(
     () => ({
       open: data.comboboxState === ComboboxState.Open,
       disabled,
       activeIndex: data.activeOptionIndex,
       activeOption:
         data.activeOptionIndex === null
           ? null
-          : (data.options[data.activeOptionIndex].dataRef.current.value as TType),
+          : (data.options[data.activeOptionIndex].dataRef.current.value as TValue),
       value,
     }),
     [data, disabled, value]
@@ -482,19 +539,19 @@ let ComboboxRoot = forwardRefWithAs(function Combobox<
   let onChange = useEvent((value: unknown) => {
     return match(data.mode, {
       [ValueMode.Single]() {
-        return theirOnChange?.(value as TType)
+        return theirOnChange?.(value as TValue)
       },
       [ValueMode.Multi]() {
-        let copy = (data.value as TActualType[]).slice()
+        let copy = (data.value as TValue[]).slice()
 
-        let idx = copy.findIndex((item) => compare(item, value as TActualType))
+        let idx = copy.findIndex((item) => compare(item, value as TValue))
         if (idx === -1) {
-          copy.push(value as TActualType)
+          copy.push(value as TValue)
         } else {
           copy.splice(idx, 1)
         }
 
-        return theirOnChange?.(copy as unknown as TType)
+        return theirOnChange?.(copy as unknown as TValue[])
       },
     })
   })
@@ -550,7 +607,8 @@ let ComboboxRoot = forwardRefWithAs(function Combobox<
       </ComboboxDataContext.Provider>
     </ComboboxActionsContext.Provider>
   )
-})
+}
+let ComboboxRoot = forwardRefWithAs(ComboboxFn)
 
 // ---
 

diff --git a/packages/@headlessui-react/src/components/transitions/transition.tsx b/packages/@headlessui-react/src/components/transitions/transition.tsx
@@ -12,7 +12,7 @@ import React, {
   MutableRefObject,
   Ref,
 } from 'react'
-import { Props } from '../../types'
+import { Props, ReactTag } from '../../types'
 import {
   Features,
   forwardRefWithAs,
@@ -68,7 +68,7 @@ export interface TransitionEvents {
   afterLeave?: () => void
 }
 
-type TransitionChildProps<TTag> = Props<TTag, TransitionChildRenderPropArg> &
+type TransitionChildProps<TTag extends ReactTag> = Props<TTag, TransitionChildRenderPropArg> &
   PropsForFeatures<typeof TransitionChildRenderFeatures> &
   TransitionClasses &
   TransitionEvents & { appear?: boolean }

diff --git a/packages/@headlessui-react/src/types.ts b/packages/@headlessui-react/src/types.ts
@@ -1,4 +1,6 @@
-import { ReactNode, ReactElement } from 'react'
+import { ReactNode, ReactElement, JSXElementConstructor } from 'react'
+
+export type ReactTag = keyof JSX.IntrinsicElements | JSXElementConstructor<any>
 
 // A unique placeholder we can use as a default. This is nice because we can use this instead of
 // defaulting to null / never / ... and possibly collide with actual data.
@@ -8,38 +10,48 @@ export type __ = typeof __
 
 export type Expand<T> = T extends infer O ? { [K in keyof O]: O[K] } : never
 
-export type PropsOf<TTag = any> = TTag extends React.ElementType
+export type PropsOf<TTag extends ReactTag> = TTag extends React.ElementType
   ? React.ComponentProps<TTag>
   : never
 
 type PropsWeControl = 'as' | 'children' | 'refName' | 'className'
 
 // Resolve the props of the component, but ensure to omit certain props that we control
-type CleanProps<TTag, TOmitableProps extends keyof any = __> = TOmitableProps extends __
+type CleanProps<
+  TTag extends ReactTag,
+  TOmitableProps extends PropertyKey = __
+> = TOmitableProps extends __
   ? Omit<PropsOf<TTag>, PropsWeControl>
   : Omit<PropsOf<TTag>, TOmitableProps | PropsWeControl>
 
 // Add certain props that we control
-type OurProps<TTag, TSlot = any> = {
+type OurProps<TTag extends ReactTag, TSlot> = {
   as?: TTag
   children?: ReactNode | ((bag: TSlot) => ReactElement)
   refName?: string
 }
 
+type HasProperty<T extends object, K extends PropertyKey> = T extends never
+  ? never
+  : K extends keyof T
+  ? true
+  : never
+
 // Conditionally override the `className`, to also allow for a function
-// if and only if the PropsOf<TTag> already define `className`.
+// if and only if the PropsOf<TTag> already defines `className`.
 // This will allow us to have a TS error on as={Fragment}
-type ClassNameOverride<TTag, TSlot = any> = PropsOf<TTag> extends { className?: any }
-  ? { className?: string | ((bag: TSlot) => string) }
-  : {}
+type ClassNameOverride<TTag extends ReactTag, TSlot = {}> =
+  // Order is important here, because `never extends true` is `true`...
+  true extends HasProperty<PropsOf<TTag>, 'className'>
+    ? { className?: PropsOf<TTag>['className'] | ((bag: TSlot) => string) }
+    : {}
 
 // Provide clean TypeScript props, which exposes some of our custom API's.
-export type Props<TTag, TSlot = any, TOmitableProps extends keyof any = __> = CleanProps<
-  TTag,
-  TOmitableProps
-> &
-  OurProps<TTag, TSlot> &
-  ClassNameOverride<TTag, TSlot>
+export type Props<
+  TTag extends ReactTag,
+  TSlot = {},
+  TOmitableProps extends PropertyKey = __
+> = CleanProps<TTag, TOmitableProps> & OurProps<TTag, TSlot> & ClassNameOverride<TTag, TSlot>
 
 type Without<T, U> = { [P in Exclude<keyof T, keyof U>]?: never }
 export type XOR<T, U> = T | U extends __
@@ -51,3 +63,6 @@ export type XOR<T, U> = T | U extends __
   : T | U extends object
   ? (Without<T, U> & U) | (Without<U, T> & T)
   : T | U
+
+export type ByComparator<T> = (keyof T & string) | ((a: T, b: T) => boolean)
+export type EnsureArray<T> = T extends any[] ? T : Expand<T>[]
diff --git a/packages/playground-react/pages/combobox/multi-select.tsx b/packages/playground-react/pages/combobox/multi-select.tsx
@@ -39,7 +39,12 @@ function MultiPeopleList() {
             console.log([...new FormData(e.currentTarget).entries()])
           }}
         >
-          <Combobox value={activePersons} onChange={setActivePersons} name="people" multiple>
+          <Combobox
+            value={activePersons}
+            onChange={(people) => setActivePersons(people)}
+            name="people"
+            multiple
+          >
             <Combobox.Label className="block text-sm font-medium leading-5 text-gray-700">
               Assigned to
             </Combobox.Label>