Fix crash when using multiple mode without value prop (uncontroll…

…ed) for `Listbox` and `Combobox` components (#2058) * ensure `value` in `multiple` mode is always an array In case nothing or `undefined` was passed. * update changelog
tailwindlabs · Dec 2, 2022 · a0bcbae · a0bcbae
1 parent 4da0b3a
commit a0bcbae
Show file tree

Hide file tree

Showing 10 changed files with 148 additions and 4 deletions.
diff --git 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
@@ -172,6 +172,29 @@ describe('Rendering', () => {
       })
     )
 
+    it(
+      'should not crash in multiple mode',
+      suppressConsoleLogs(async () => {
+        render(
+          <Combobox multiple name="abc">
+            <Combobox.Button>Trigger</Combobox.Button>
+            <Combobox.Options>
+              <Combobox.Option value={{ id: 1, name: 'alice' }}>alice</Combobox.Option>
+              <Combobox.Option value={{ id: 2, name: 'bob' }}>bob</Combobox.Option>
+              <Combobox.Option value={{ id: 3, name: 'charlie' }}>charlie</Combobox.Option>
+            </Combobox.Options>
+          </Combobox>
+        )
+
+        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
@@ -403,7 +403,7 @@ function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_T
     multiple = false,
     ...theirProps
   } = props
-  let [value, theirOnChange] = useControllable<any>(
+  let [value = multiple ? [] : undefined, theirOnChange] = useControllable<any>(
     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
@@ -163,6 +163,29 @@ describe('Rendering', () => {
       })
     )
 
+    it(
+      'should not crash in multiple mode',
+      suppressConsoleLogs(async () => {
+        render(
+          <Listbox multiple name="abc">
+            <Listbox.Button>Trigger</Listbox.Button>
+            <Listbox.Options>
+              <Listbox.Option value={{ id: 1, name: 'alice' }}>alice</Listbox.Option>
+              <Listbox.Option value={{ id: 2, name: 'bob' }}>bob</Listbox.Option>
+              <Listbox.Option value={{ id: 3, name: 'charlie' }}>charlie</Listbox.Option>
+            </Listbox.Options>
+          </Listbox>
+        )
+
+        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
@@ -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
@@ -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
@@ -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`
+            <Combobox multiple name="abc">
+              <ComboboxButton>Trigger</ComboboxButton>
+              <ComboboxOptions>
+                <ComboboxOption
+                  v-for="option in options"
+                  :key="option.id"
+                  :value="option"
+                  v-slot="data"
+                  >{{ JSON.stringify(data) }}</ComboboxOption
+                >
+              </ComboboxOptions>
+            </Combobox>
+          `,
+          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
@@ -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
@@ -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`
+            <Listbox multiple name="abc">
+              <ListboxButton>Trigger</ListboxButton>
+              <ListboxOptions>
+                <ListboxOption
+                  v-for="option in options"
+                  :key="option.id"
+                  :value="option"
+                  v-slot="data"
+                  >{{ JSON.stringify(data) }}</ListboxOption
+                >
+              </ListboxOptions>
+            </Listbox>
+          `,
+          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
@@ -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)
     )