Skip to content

Commit

Permalink
Fix regression where displayValue crashes (#2087)
Browse files Browse the repository at this point in the history
* fix regression where `displayValue` crashes

It regressed in the sense that it now uses `displayValue` for the
`defaultValue` as well, but if nothing is passed it would crash.

Right now, it makes sure to only run the displayValue value on the
actual value and the actual default value if they are not undefined.

Note: if your displayValue is implemented like `(value) => value.name`,
and your `value` is passed as `null`, it will still crash (as expected)
because then you are in charge of rendering something else than null. If
we would "fix" this, then no value can be rendered instead of `null`.

Fixes: #2084

* update changelog
  • Loading branch information
RobinMalfait committed Dec 12, 2022
1 parent 208c6fd commit 46754e6
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 6 deletions.
4 changes: 3 additions & 1 deletion packages/@headlessui-react/CHANGELOG.md
Expand Up @@ -7,7 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

- Nothing yet!
### Fixed

- Fix regression where `displayValue` crashes ([#2087](https://github.com/tailwindlabs/headlessui/pull/2087))

## [1.7.5] - 2022-12-08

Expand Down
Expand Up @@ -412,6 +412,30 @@ describe('Rendering', () => {
})
)
})

it(
'should not crash when a defaultValue is not given',
suppressConsoleLogs(async () => {
let data = [
{ id: 1, name: 'alice', label: 'Alice' },
{ id: 2, name: 'bob', label: 'Bob' },
{ id: 3, name: 'charlie', label: 'Charlie' },
]

render(
<Combobox name="assignee" by="id">
<Combobox.Input displayValue={(value: { name: string }) => value.name} />
<Combobox.Options>
{data.map((person) => (
<Combobox.Option key={person.id} value={person}>
{person.label}
</Combobox.Option>
))}
</Combobox.Options>
</Combobox>
)
})
)
})

describe('Combobox.Input', () => {
Expand Down
Expand Up @@ -705,7 +705,7 @@ let Input = forwardRefWithAs(function Input<
// you don't have to use this at all, a more common UI is a "tag" based UI, which you can render
// yourself using the selected option(s).
let currentValue = useMemo(() => {
if (typeof displayValue === 'function') {
if (typeof displayValue === 'function' && data.value !== undefined) {
return displayValue(data.value as unknown as TType) ?? ''
} else if (typeof data.value === 'string') {
return data.value
Expand Down Expand Up @@ -909,7 +909,9 @@ let Input = forwardRefWithAs(function Input<
'aria-labelledby': labelledby,
defaultValue:
props.defaultValue ??
displayValue?.(data.defaultValue as unknown as TType) ??
(data.defaultValue !== undefined
? displayValue?.(data.defaultValue as unknown as TType)
: null) ??
data.defaultValue,
disabled: data.disabled,
onCompositionStart: handleCompositionStart,
Expand Down
4 changes: 3 additions & 1 deletion packages/@headlessui-vue/CHANGELOG.md
Expand Up @@ -7,7 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

- Nothing yet!
### Fixed

- Fix regression where `displayValue` crashes ([#2087](https://github.com/tailwindlabs/headlessui/pull/2087))

## [1.7.5] - 2022-12-08

Expand Down
25 changes: 25 additions & 0 deletions packages/@headlessui-vue/src/components/combobox/combobox.test.ts
Expand Up @@ -465,6 +465,31 @@ describe('Rendering', () => {
})
)
})

it(
'should not crash when a defaultValue is not given',
suppressConsoleLogs(async () => {
let data = [
{ id: 1, name: 'alice', label: 'Alice' },
{ id: 2, name: 'bob', label: 'Bob' },
{ id: 3, name: 'charlie', label: 'Charlie' },
]

renderTemplate({
template: html`
<Combobox name="assignee" by="id">
<ComboboxInput :displayValue="(value) => value.name" />
<ComboboxOptions>
<ComboboxOption v-for="person in data" :key="person.id" :value="person">
{{ person.label }}
</ComboboxOption>
<ComboboxOptions>
</Combobox>
`,
setup: () => ({ data }),
})
})
)
})

describe('ComboboxInput', () => {
Expand Down
6 changes: 4 additions & 2 deletions packages/@headlessui-vue/src/components/combobox/combobox.ts
Expand Up @@ -676,7 +676,7 @@ export let ComboboxInput = defineComponent({
let value = api.value.value
if (!dom(api.inputRef)) return ''

if (typeof props.displayValue !== 'undefined') {
if (typeof props.displayValue !== 'undefined' && value !== undefined) {
return props.displayValue(value as unknown) ?? ''
} else if (typeof value === 'string') {
return value
Expand Down Expand Up @@ -874,7 +874,9 @@ export let ComboboxInput = defineComponent({
let defaultValue = computed(() => {
return (
props.defaultValue ??
props.displayValue?.(api.defaultValue.value) ??
(api.defaultValue.value !== undefined
? props.displayValue?.(api.defaultValue.value)
: null) ??
api.defaultValue.value ??
''
)
Expand Down

0 comments on commit 46754e6

Please sign in to comment.