Commit 31b3e47
This PR fixes an issue where the `Combobox` only inferred the type of
the internal value based on the `value` prop.
We only wanted to infer the type of the Combobox based on the `value`
prop to make it less confusing where values came from. But if you are
using an uncontrolled component then the `value` is not provided to the
`Combobox`.
In most cases this isn't an actual issue, but it is if you _also_ want
to us the `by` prop to improve comparing values based on a certain
property.
Without this change, the only way of typing the `by` prop correctly is
by using an explicit type on the `Combobox`:
```diff
- <Combobox
+ <Combobox<typeof people[number]>
```
With this change, the internal value type will be inferred by the
`onChange` meaning that the `by` prop is properly typed now.
## Test plan
Given a reproduction where the `by` prop is **correct** if you look at
the type of the `onChange`:
```tsx
import * as React from 'react'
import { Combobox } from './combobox'
export function App() {
function handleChange(person: { id: string; name: string } | null) {
console.log(person)
}
return <Combobox by="name" onChange={handleChange}></Combobox>
}
```
**Before:**
<img width="2282" height="1381" alt="image"
src="https://github.com/user-attachments/assets/8a095931-6fc3-4b67-9d83-165017b93d72"
/>
**After:**
<img width="895" height="143" alt="image"
src="https://github.com/user-attachments/assets/57cd7a66-b830-4899-92dc-99bf65e5d8a5"
/>
There is no output because there are no errors.
---
Given a reproduction where the `by` prop is **incorrect** if you look at
the type of the `onChange`:
```tsx
import * as React from 'react'
import { Combobox } from './combobox'
export function App() {
function handleChange(person: { id: string; name: string } | null) {
console.log(person)
}
return <Combobox by="username" onChange={handleChange}></Combobox>
}
```
**Before:**
<img width="2306" height="1378" alt="image"
src="https://github.com/user-attachments/assets/45a15fa0-d59a-40ca-a498-7de9ad82633e"
/>
**After:**
<img width="2103" height="818" alt="image"
src="https://github.com/user-attachments/assets/05124186-ea85-4023-b9e0-8cf019a87995"
/>
The error type is much simpler and easier to reason about. It's not as
clean as a simple `'id' | 'name'` because the `by` prop can _also_ be a
function. But at least there isn't a wall of TypeScript errors you have
to reason about. Maybe we can improve this part in a future PR.
Fixes: #3636
---------
Co-authored-by: Jordan Pittman <thecrypticace@gmail.com>
1 parent b0615ad commit 31b3e47
File tree
2 files changed
+2
-3
lines changed- packages/@headlessui-react
- src/components/combobox
2 files changed
+2
-3
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| 15 | + | |
15 | 16 | | |
16 | 17 | | |
17 | 18 | | |
| |||
Lines changed: 1 addition & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
257 | 257 | | |
258 | 258 | | |
259 | 259 | | |
260 | | - | |
261 | | - | |
262 | | - | |
| 260 | + | |
263 | 261 | | |
264 | 262 | | |
265 | 263 | | |
| |||
0 commit comments