Skip to content

Commit

Permalink
Make the Combobox component nullable by default (#3064)
Browse files Browse the repository at this point in the history 
* remove `nullable` prop

* prevent selecting active option on blur

+ cleanup and adjust comments

* remove nullable from comments

* bump TypeScript to 5.4

This gives us `NoInfer<T>`!

* simplify types of `Combobox`

Now that `nullable` is gone, we can take another look at the type
definition. This in combination with the new `NoInfer` type makes types
drastically simpler and more correct.

* re-add `nullable` to prevent type issues

But let's mark it as deprecated to hint that something changed.

* update changelog

* improve `ByComparator` type

If we are just checking for `T extends null`, then
`{id:1,name:string}|null` will also be true and therefore we would
eventually return `string` instead of `"id" | "name"`.

To solve this, we first check if `NonNullable<T> extends never`, this
would be the case if `T` is `null`.

Otherwise, we know it's not just `null` but it can be something else
with or without `null`. To be sure, we use `keyof NonNullable<null>` to
get rid of the `null` part and to only keep the rest of the object (if
it's an object).

* ensure the `by` prop type handles `multiple` values correctly

This way the `by` prop will still compare single values that are present
inside the array.

This now also solves a pending TypeScript issue that we used to `//
@ts-expect-error` before.

* type uncontrolled `Combobox` components correctly

We have some tests that use uncontrolled components which means that we
can't infer the type from the `value` type.

* simplify `onChange` calls

Now that we don't infer the type when using the generic inside of
`onChange`, it means that we can use `onChange={setValue}` directly
because we don't have to worry about the updater function of `setValue`
anymore.

* correctly type `onChange`, by adding `null`

If you are in single value mode, then the `onChange` can (and will)
receive `null` as a value (when you clear the input field). We never
properly typed it so this fixes that.

In multiple value mode this won't happen, if anything the value will be
`[]` but not `null`.

* remove `nullable` prop from playground

* drop `nullable` mentions in tests
  • Loading branch information
@RobinMalfait
RobinMalfait authored Mar 29, 2024
1 parent 6d44a8d commit d03fbb1
Show file tree
Hide file tree
Showing 7 changed files with 104 additions and 162 deletions.
20 changes: 17 additions & 3 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,6 @@
"prettier-plugin-tailwindcss": "^0.5.7",
"rimraf": "^3.0.2",
"tslib": "^2.3.1",
"typescript": "^5.3.2"
"typescript": "^5.4.3"
}
}
1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed

- Attempt form submission when pressing `Enter` on the `<Listbox.Button />` component ([#2972](https://github.com/tailwindlabs/headlessui/pull/2972))
- Make the `Combobox` component `nullable` by default ([#3064](https://github.com/tailwindlabs/headlessui/pull/3064))

### Added

Expand Down
71 changes: 28 additions & 43 deletions packages/@headlessui-react/src/components/combobox/combobox.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,10 +387,13 @@ describe('Rendering', () => {
'should be possible to use completely new objects while rendering (single mode)',
suppressConsoleLogs(async () => {
function Example() {
let [value, setValue] = useState({ id: 2, name: 'Bob' })
let [value, setValue] = useState<{ id: number; name: string } | null>({
id: 2,
name: 'Bob',
})

return (
<Combobox value={value} onChange={(value) => setValue(value)} by="id">
<Combobox value={value} onChange={setValue} by="id">
<Combobox.Button>Trigger</Combobox.Button>
<Combobox.Options>
<Combobox.Option value={{ id: 1, name: 'alice' }}>alice</Combobox.Option>
Expand Down Expand Up @@ -432,7 +435,7 @@ describe('Rendering', () => {
let [value, setValue] = useState([{ id: 2, name: 'Bob' }])

return (
<Combobox value={value} onChange={(value) => setValue(value)} by="id" multiple>
<Combobox value={value} onChange={setValue} by="id" multiple>
<Combobox.Button>Trigger</Combobox.Button>
<Combobox.Options>
<Combobox.Option value={{ id: 1, name: 'alice' }}>alice</Combobox.Option>
Expand Down Expand Up @@ -472,7 +475,7 @@ describe('Rendering', () => {
]

render(
<Combobox name="assignee" by="id">
<Combobox<(typeof data)[number]> name="assignee" by="id">
<Combobox.Input
displayValue={(value: { name: string }) => value.name}
onChange={NOOP}
Expand All @@ -499,7 +502,7 @@ describe('Rendering', () => {
]

function Example() {
let [person, setPerson] = useState(data[1])
let [person, setPerson] = useState<(typeof data)[number] | null>(data[1])

return (
<Combobox value={person} onChange={setPerson} name="assignee" by="id">
Expand Down Expand Up @@ -546,7 +549,7 @@ describe('Rendering', () => {
]

render(
<Combobox name="assignee" by="id">
<Combobox<(typeof data)[number]> name="assignee" by="id">
<Combobox.Input onChange={NOOP} />
<Combobox.Button />
<Combobox.Options>
Expand Down Expand Up @@ -647,7 +650,7 @@ describe('Rendering', () => {
'selecting an option puts the display value into Combobox.Input when displayValue is provided (when value is undefined)',
suppressConsoleLogs(async () => {
function Example() {
let [value, setValue] = useState(undefined)
let [value, setValue] = useState<null | undefined>(undefined)

return (
<Combobox value={value} onChange={setValue}>
Expand Down Expand Up @@ -692,7 +695,7 @@ describe('Rendering', () => {

return (
<>
<Combobox value={value} onChange={setValue} nullable>
<Combobox value={value} onChange={setValue}>
<Combobox.Input
onChange={NOOP}
displayValue={(str?: string) =>
Expand Down Expand Up @@ -766,7 +769,7 @@ describe('Rendering', () => {
'should reflect the value in the input when the value changes and when you are typing',
suppressConsoleLogs(async () => {
function Example() {
let [value, setValue] = useState('bob')
let [value, setValue] = useState<string | null>('bob')
let [_query, setQuery] = useState('')

return (
Expand Down Expand Up @@ -1608,7 +1611,11 @@ describe('Rendering', () => {
handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement)))
}}
>
<Combobox name="assignee" defaultValue={{ id: 2, name: 'bob', label: 'Bob' }} by="id">
<Combobox<(typeof data)[number]>
name="assignee"
defaultValue={{ id: 2, name: 'bob', label: 'Bob' }}
by="id"
>
<Combobox.Button>{({ value }) => value?.name ?? 'Trigger'}</Combobox.Button>
<Combobox.Input
onChange={NOOP}
Expand Down Expand Up @@ -4093,42 +4100,20 @@ describe.each([{ virtual: true }, { virtual: false }])(

describe('`Backspace` key', () => {
it(
'should reset the value when the last character is removed, when in `nullable` mode',
'should reset the value when the last character is removed',
suppressConsoleLogs(async () => {
let handleChange = jest.fn()
function Example() {
let [value, setValue] = useState<string | null>('bob')
let [, setQuery] = useState<string>('')

// return (
// <MyCombobox
// options={[
// { value: 'alice', children: 'Alice' },
// { value: 'bob', children: 'Bob' },
// { value: 'charlie', children: 'Charlie' },
// ]}
// comboboxProps={{
// value,
// onChange: (value: any) => {
// setValue(value)
// handleChange(value)
// },
// nullable: true,
// }}
// inputProps={{
// onChange: (event: any) => setQuery(event.target.value),
// }}
// />
// )

return (
<Combobox
value={value}
onChange={(value) => {
setValue(value)
handleChange(value)
}}
nullable
>
<Combobox.Input onChange={(event) => setQuery(event.target.value)} />
<Combobox.Button>Trigger</Combobox.Button>
Expand Down Expand Up @@ -4175,7 +4160,7 @@ describe.each([{ virtual: true }, { virtual: false }])(
await press(Keys.Backspace)
expect(getComboboxInput()?.value).toBe('')

// Verify that we don't have an selected option anymore since we are in `nullable` mode
// Verify that we don't have an selected option anymore
assertNotActiveComboboxOption(options[1])
assertNoSelectedComboboxOption()

Expand All @@ -4189,7 +4174,7 @@ describe.each([{ virtual: true }, { virtual: false }])(
describe('`Any` key aka search', () => {
type Option = { value: string; name: string; disabled: boolean }
function Example(props: { people: { value: string; name: string; disabled: boolean }[] }) {
let [value, setValue] = useState<Option | undefined>(undefined)
let [value, setValue] = useState<Option | null | undefined>(undefined)
let [query, setQuery] = useState<string>('')
let filteredPeople =
query === ''
Expand All @@ -4207,7 +4192,7 @@ describe.each([{ virtual: true }, { virtual: false }])(
}}
value={value}
by="value"
onChange={(value) => setValue(value)}
onChange={setValue}
>
<Combobox.Input onChange={(event) => setQuery(event.target.value)} />
<Combobox.Button>Trigger</Combobox.Button>
Expand Down Expand Up @@ -5502,7 +5487,7 @@ describe('Multi-select', () => {
let [value, setValue] = useState<string[]>(['bob', 'charlie'])

return (
<Combobox value={value} onChange={(value) => setValue(value)} multiple>
<Combobox value={value} onChange={setValue} multiple>
<Combobox.Input onChange={() => {}} />
<Combobox.Button>Trigger</Combobox.Button>
<Combobox.Options>
Expand Down Expand Up @@ -5538,7 +5523,7 @@ describe('Multi-select', () => {
let [value, setValue] = useState<string[]>(['bob', 'charlie'])

return (
<Combobox value={value} onChange={(value) => setValue(value)} multiple>
<Combobox value={value} onChange={setValue} multiple>
<Combobox.Input onChange={() => {}} />
<Combobox.Button>Trigger</Combobox.Button>
<Combobox.Options>
Expand Down Expand Up @@ -5567,7 +5552,7 @@ describe('Multi-select', () => {
let [value, setValue] = useState<string[]>(['bob', 'charlie'])

return (
<Combobox value={value} onChange={(value) => setValue(value)} multiple>
<Combobox value={value} onChange={setValue} multiple>
<Combobox.Input onChange={() => {}} />
<Combobox.Button>Trigger</Combobox.Button>
<Combobox.Options>
Expand Down Expand Up @@ -5600,7 +5585,7 @@ describe('Multi-select', () => {
let [value, setValue] = useState<string[]>(['bob', 'charlie'])

return (
<Combobox value={value} onChange={(value) => setValue(value)} multiple>
<Combobox value={value} onChange={setValue} multiple>
<Combobox.Input onChange={() => {}} />
<Combobox.Button>Trigger</Combobox.Button>
<Combobox.Options>
Expand Down Expand Up @@ -5648,7 +5633,7 @@ describe('Multi-select', () => {
let [value, setValue] = useState<string[]>([])

return (
<Combobox value={value} onChange={(value) => setValue(value)} multiple>
<Combobox value={value} onChange={setValue} multiple>
<Combobox.Input onChange={() => {}} />
<Combobox.Button>Trigger</Combobox.Button>
<Combobox.Options>
Expand Down Expand Up @@ -5798,7 +5783,7 @@ describe('Form compatibility', () => {
let submits = jest.fn()

function Example() {
let [value, setValue] = useState('home-delivery')
let [value, setValue] = useState<string | null>('home-delivery')
return (
<form
onSubmit={(event) => {
Expand Down Expand Up @@ -5860,7 +5845,7 @@ describe('Form compatibility', () => {
]

function Example() {
let [value, setValue] = useState(options[0])
let [value, setValue] = useState<(typeof options)[number] | null>(options[0])

return (
<form
Expand Down
Loading

0 comments on commit d03fbb1

Please sign in to comment.