From d03fbb19f57ca3311b966ab3cf4603a5b981bf43 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 29 Mar 2024 15:41:12 +0100 Subject: [PATCH] Make the `Combobox` component `nullable` by default (#3064) * 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`! * 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 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` 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 --- package-lock.json | 20 ++- package.json | 2 +- packages/@headlessui-react/CHANGELOG.md | 1 + .../src/components/combobox/combobox.test.tsx | 71 +++----- .../src/components/combobox/combobox.tsx | 169 ++++++------------ .../src/hooks/use-by-comparator.ts | 2 +- .../pages/combobox/combobox-virtualized.tsx | 1 - 7 files changed, 104 insertions(+), 162 deletions(-) diff --git a/package-lock.json b/package-lock.json index 4c2e013223..ed9f8defe9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -30,7 +30,7 @@ "prettier-plugin-tailwindcss": "^0.5.7", "rimraf": "^3.0.2", "tslib": "^2.3.1", - "typescript": "^5.3.2" + "typescript": "^5.4.3" } }, "node_modules/@adobe/css-tools": { @@ -100,6 +100,19 @@ "node": ">=18" } }, + "node_modules/@arethetypeswrong/core/node_modules/typescript": { + "version": "5.3.3", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.3.3.tgz", + "integrity": "sha512-pXWcraxM0uxAS+tN0AG/BF2TyqmHO014Z070UsJ+pFvYuRSq8KH8DmWpnbXe0pEPDHXZV3FcAbJkijJ5oNEnWw==", + "dev": true, + "bin": { + "tsc": "bin/tsc", + "tsserver": "bin/tsserver" + }, + "engines": { + "node": ">=14.17" + } + }, "node_modules/@babel/code-frame": { "version": "7.23.5", "dev": true, @@ -9653,9 +9666,10 @@ } }, "node_modules/typescript": { - "version": "5.3.3", + "version": "5.4.3", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.4.3.tgz", + "integrity": "sha512-KrPd3PKaCLr78MalgiwJnA25Nm8HAmdwN3mYUYZgG/wizIo9EainNVQI9/yDavtVFRN2h3k8uf3GLHuhDMgEHg==", "devOptional": true, - "license": "Apache-2.0", "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" diff --git a/package.json b/package.json index 8f89cf0c7d..68164048dd 100644 --- a/package.json +++ b/package.json @@ -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" } } diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index c8291c4ebc..9086dd5168 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -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 `` 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 diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx index 8e24d69d0e..0e824d8562 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx @@ -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 ( - setValue(value)} by="id"> + Trigger alice @@ -432,7 +435,7 @@ describe('Rendering', () => { let [value, setValue] = useState([{ id: 2, name: 'Bob' }]) return ( - setValue(value)} by="id" multiple> + Trigger alice @@ -472,7 +475,7 @@ describe('Rendering', () => { ] render( - + name="assignee" by="id"> value.name} onChange={NOOP} @@ -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 ( @@ -546,7 +549,7 @@ describe('Rendering', () => { ] render( - + name="assignee" by="id"> @@ -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(undefined) return ( @@ -692,7 +695,7 @@ describe('Rendering', () => { return ( <> - + @@ -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('bob') let [_query, setQuery] = useState('') return ( @@ -1608,7 +1611,11 @@ describe('Rendering', () => { handleSubmission(Object.fromEntries(new FormData(e.target as HTMLFormElement))) }} > - + + name="assignee" + defaultValue={{ id: 2, name: 'bob', label: 'Bob' }} + by="id" + > {({ value }) => value?.name ?? 'Trigger'} { 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('bob') let [, setQuery] = useState('') - // return ( - // { - // setValue(value) - // handleChange(value) - // }, - // nullable: true, - // }} - // inputProps={{ - // onChange: (event: any) => setQuery(event.target.value), - // }} - // /> - // ) - return ( setQuery(event.target.value)} /> Trigger @@ -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() @@ -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