From a0bcbae93ac57219b87f9797fb2c9228c3f6ff98 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 2 Dec 2022 15:18:31 +0100 Subject: [PATCH] Fix crash when using `multiple` mode without `value` prop (uncontrolled) for `Listbox` and `Combobox` components (#2058) * ensure `value` in `multiple` mode is always an array In case nothing or `undefined` was passed. * update changelog --- packages/@headlessui-react/CHANGELOG.md | 1 + .../src/components/combobox/combobox.test.tsx | 23 +++++++++++ .../src/components/combobox/combobox.tsx | 2 +- .../src/components/listbox/listbox.test.tsx | 23 +++++++++++ .../src/components/listbox/listbox.tsx | 6 ++- packages/@headlessui-vue/CHANGELOG.md | 1 + .../src/components/combobox/combobox.test.ts | 39 +++++++++++++++++++ .../src/components/combobox/combobox.ts | 9 ++++- .../src/components/listbox/listbox.test.tsx | 39 +++++++++++++++++++ .../src/components/listbox/listbox.ts | 9 ++++- 10 files changed, 148 insertions(+), 4 deletions(-) diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index 26d6b9e08f..53f7c00b8d 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Ensure Popover doesn't crash when `focus` is going to `window` ([#2019](https://github.com/tailwindlabs/headlessui/pull/2019)) - Ensure `shift+home` and `shift+end` works as expected in the `Combobox.Input` component ([#2024](https://github.com/tailwindlabs/headlessui/pull/2024)) - Improve syncing of the `Combobox.Input` value ([#2042](https://github.com/tailwindlabs/headlessui/pull/2042)) +- Fix crash when using `multiple` mode without `value` prop (uncontrolled) for `Listbox` and `Combobox` components ([#2058](https://github.com/tailwindlabs/headlessui/pull/2058)) ## [1.7.4] - 2022-11-03 diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx index d4e65258e7..e3be24ce25 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx @@ -172,6 +172,29 @@ describe('Rendering', () => { }) ) + it( + 'should not crash in multiple mode', + suppressConsoleLogs(async () => { + render( + + Trigger + + alice + bob + charlie + + + ) + + await click(getComboboxButton()) + let [alice, bob, charlie] = getComboboxOptions() + + await click(alice) + await click(bob) + await click(charlie) + }) + ) + describe('Equality', () => { let options = [ { id: 1, name: 'Alice' }, diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 31776faa27..0d5f573b3c 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -403,7 +403,7 @@ function ComboboxFn( + let [value = multiple ? [] : undefined, theirOnChange] = useControllable( controlledValue, controlledOnChange, defaultValue diff --git a/packages/@headlessui-react/src/components/listbox/listbox.test.tsx b/packages/@headlessui-react/src/components/listbox/listbox.test.tsx index 84f415d3c9..45feef66dc 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.test.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.test.tsx @@ -163,6 +163,29 @@ describe('Rendering', () => { }) ) + it( + 'should not crash in multiple mode', + suppressConsoleLogs(async () => { + render( + + Trigger + + alice + bob + charlie + + + ) + + await click(getListboxButton()) + let [alice, bob, charlie] = getListboxOptions() + + await click(alice) + await click(bob) + await click(charlie) + }) + ) + describe('Equality', () => { let options = [ { id: 1, name: 'Alice' }, diff --git a/packages/@headlessui-react/src/components/listbox/listbox.tsx b/packages/@headlessui-react/src/components/listbox/listbox.tsx index e7c52bc5a9..f78a7231a8 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.tsx @@ -358,7 +358,11 @@ let ListboxRoot = forwardRefWithAs(function Listbox< const orientation = horizontal ? 'horizontal' : 'vertical' let listboxRef = useSyncRefs(ref) - let [value, theirOnChange] = useControllable(controlledValue, controlledOnChange, defaultValue) + let [value = multiple ? [] : undefined, theirOnChange] = useControllable( + controlledValue, + controlledOnChange, + defaultValue + ) let [state, dispatch] = useReducer(stateReducer, { dataRef: createRef(), diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index 6a26cd303a..3bb627ba87 100644 --- a/packages/@headlessui-vue/CHANGELOG.md +++ b/packages/@headlessui-vue/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Ensure Popover doesn't crash when `focus` is going to `window` ([#2019](https://github.com/tailwindlabs/headlessui/pull/2019)) - Ensure `shift+home` and `shift+end` works as expected in the `ComboboxInput` component ([#2024](https://github.com/tailwindlabs/headlessui/pull/2024)) - Improve syncing of the `ComboboxInput` value ([#2042](https://github.com/tailwindlabs/headlessui/pull/2042)) +- Fix crash when using `multiple` mode without `value` prop (uncontrolled) for `Listbox` and `Combobox` components ([#2058](https://github.com/tailwindlabs/headlessui/pull/2058)) ## [1.7.4] - 2022-11-03 diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts index 8b09e55462..4c608887df 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts @@ -203,6 +203,45 @@ describe('Rendering', () => { }) ) + it( + 'should not crash in multiple mode', + suppressConsoleLogs(async () => { + let options = [ + { id: 1, name: 'Alice' }, + { id: 2, name: 'Bob' }, + { id: 3, name: 'Charlie' }, + ] + + renderTemplate({ + template: html` + + Trigger + + {{ JSON.stringify(data) }} + + + `, + setup: () => { + let value = ref(options[1]) + return { options, value } + }, + }) + + await click(getComboboxButton()) + let [alice, bob, charlie] = getComboboxOptions() + + await click(alice) + await click(bob) + await click(charlie) + }) + ) + describe('Equality', () => { let options = [ { id: 1, name: 'Alice' }, diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.ts b/packages/@headlessui-vue/src/components/combobox/combobox.ts index 47dda4b54a..d9786c9750 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.ts @@ -176,7 +176,14 @@ export let Combobox = defineComponent({ let mode = computed(() => (props.multiple ? ValueMode.Multi : ValueMode.Single)) let nullable = computed(() => props.nullable) let [value, theirOnChange] = useControllable( - computed(() => props.modelValue), + computed(() => + props.modelValue === undefined + ? match(mode.value, { + [ValueMode.Multi]: [], + [ValueMode.Single]: undefined, + }) + : props.modelValue + ), (value: unknown) => emit('update:modelValue', value), computed(() => props.defaultValue) ) diff --git a/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx b/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx index 210019190f..0a5360a96e 100644 --- a/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx +++ b/packages/@headlessui-vue/src/components/listbox/listbox.test.tsx @@ -183,6 +183,45 @@ describe('Rendering', () => { }) ) + it( + 'should not crash in multiple mode', + suppressConsoleLogs(async () => { + let options = [ + { id: 1, name: 'Alice' }, + { id: 2, name: 'Bob' }, + { id: 3, name: 'Charlie' }, + ] + + renderTemplate({ + template: html` + + Trigger + + {{ JSON.stringify(data) }} + + + `, + setup: () => { + let value = ref(options[1]) + return { options, value } + }, + }) + + await click(getListboxButton()) + let [alice, bob, charlie] = getListboxOptions() + + await click(alice) + await click(bob) + await click(charlie) + }) + ) + describe('Equality', () => { let options = [ { id: 1, name: 'Alice' }, diff --git a/packages/@headlessui-vue/src/components/listbox/listbox.ts b/packages/@headlessui-vue/src/components/listbox/listbox.ts index 8ba88190cd..8644562339 100644 --- a/packages/@headlessui-vue/src/components/listbox/listbox.ts +++ b/packages/@headlessui-vue/src/components/listbox/listbox.ts @@ -168,7 +168,14 @@ export let Listbox = defineComponent({ let mode = computed(() => (props.multiple ? ValueMode.Multi : ValueMode.Single)) let [value, theirOnChange] = useControllable( - computed(() => props.modelValue), + computed(() => + props.modelValue === undefined + ? match(mode.value, { + [ValueMode.Multi]: [], + [ValueMode.Single]: undefined, + }) + : props.modelValue + ), (value: unknown) => emit('update:modelValue', value), computed(() => props.defaultValue) )