huanld/headlessui
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Warn when changing Combobox between controlled and uncontrolled (#1878)

Browse Source
This commit is contained in:
Jordan Pittman
2022-10-10 10:41:15 -04:00
committed by GitHub
parent 83a5f456c3
commit 1127a55a76
6 changed files with 177 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/main.yml
+3 -2
View File
@@ -1,8 +1,9 @@
name: CI
on:
- push
- pull_request
push:
branches: [main]
pull_request:
concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
packages/@headlessui-react/CHANGELOG.md
+4
View File
@@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fix `<Popover.Button as={Fragment} />` crash ([#1889](https://github.com/tailwindlabs/headlessui/pull/1889))
- Expose `close` function for `Menu` and `Menu.Item` components ([#1897](https://github.com/tailwindlabs/headlessui/pull/1897))
### Added
- Warn when changing components between controlled and uncontrolled ([#1878](https://github.com/tailwindlabs/headlessui/issues/1878))
## [1.7.3] - 2022-09-30
### Fixed
packages/@headlessui-react/src/components/combobox/combobox.test.tsx
+95 -5
View File
@@ -2,7 +2,7 @@ import React, { createElement, useState, useEffect } from 'react'
import { render } from '@testing-library/react'
import { Combobox } from './combobox'
import { suppressConsoleLogs } from '../../test-utils/suppress-console-logs'
import { mockingConsoleLogs, suppressConsoleLogs } from '../../test-utils/suppress-console-logs'
import {
click,
focus,
@@ -396,7 +396,7 @@ describe('Rendering', () => {
'selecting an option puts the value into Combobox.Input when displayValue is not provided',
suppressConsoleLogs(async () => {
function Example() {
let [value, setValue] = useState(undefined)
let [value, setValue] = useState(null)
return (
<Combobox value={value} onChange={setValue}>
@@ -430,7 +430,7 @@ describe('Rendering', () => {
'selecting an option puts the display value into Combobox.Input when displayValue is provided',
suppressConsoleLogs(async () => {
function Example() {
let [value, setValue] = useState(undefined)
let [value, setValue] = useState(null)
return (
<Combobox value={value} onChange={setValue}>
@@ -558,7 +558,7 @@ describe('Rendering', () => {
'should be possible to override the `type` on the input',
suppressConsoleLogs(async () => {
function Example() {
let [value, setValue] = useState(undefined)
let [value, setValue] = useState(null)
return (
<Combobox value={value} onChange={setValue}>
@@ -5155,7 +5155,7 @@ describe('Mouse interactions', () => {
)
it(
'should sync the input field correctly and reset it when resetting the value from outside',
'should sync the input field correctly and reset it when resetting the value from outside (to null)',
suppressConsoleLogs(async () => {
function Example() {
let [value, setValue] = useState<string | null>('bob')
@@ -5196,6 +5196,96 @@ describe('Mouse interactions', () => {
})
)
it(
'should warn when changing the combobox from uncontrolled to controlled',
mockingConsoleLogs(async (spy) => {
function Example() {
let [value, setValue] = useState<string | undefined>(undefined)
return (
<>
<Combobox value={value} onChange={setValue}>
<Combobox.Input onChange={NOOP} />
<Combobox.Button>Trigger</Combobox.Button>
<Combobox.Options>
<Combobox.Option value="alice">alice</Combobox.Option>
<Combobox.Option value="bob">bob</Combobox.Option>
<Combobox.Option value="charlie">charlie</Combobox.Option>
</Combobox.Options>
</Combobox>
<button onClick={() => setValue('bob')}>to controlled</button>
</>
)
}
// Render a uncontrolled combobox
render(<Example />)
// Change to an controlled combobox
await click(getByText('to controlled'))
// Make sure we get a warning
expect(spy).toBeCalledTimes(1)
expect(spy.mock.calls.map((args) => args[0])).toEqual([
'A component is changing from uncontrolled to controlled. This may be caused by the value changing from undefined to a defined value, which should not happen.',
])
// Render a fresh uncontrolled combobox
render(<Example />)
// Change to an controlled combobox
await click(getByText('to controlled'))
// We shouldn't have gotten another warning as we do not want to warn on every render
expect(spy).toBeCalledTimes(1)
})
)
it(
'should warn when changing the combobox from controlled to uncontrolled',
mockingConsoleLogs(async (spy) => {
function Example() {
let [value, setValue] = useState<string | undefined>('bob')
return (
<>
<Combobox value={value} onChange={setValue}>
<Combobox.Input onChange={NOOP} />
<Combobox.Button>Trigger</Combobox.Button>
<Combobox.Options>
<Combobox.Option value="alice">alice</Combobox.Option>
<Combobox.Option value="bob">bob</Combobox.Option>
<Combobox.Option value="charlie">charlie</Combobox.Option>
</Combobox.Options>
</Combobox>
<button onClick={() => setValue(undefined)}>to uncontrolled</button>
</>
)
}
// Render a controlled combobox
render(<Example />)
// Change to an uncontrolled combobox
await click(getByText('to uncontrolled'))
// Make sure we get a warning
expect(spy).toBeCalledTimes(1)
expect(spy.mock.calls.map((args) => args[0])).toEqual([
'A component is changing from controlled to uncontrolled. This may be caused by the value changing from a defined value to undefined, which should not happen.',
])
// Render a fresh controlled combobox
render(<Example />)
// Change to an uncontrolled combobox
await click(getByText('to uncontrolled'))
// We shouldn't have gotten another warning as we do not want to warn on every render
expect(spy).toBeCalledTimes(1)
})
)
it(
'should sync the input field correctly and reset it when resetting the value from outside (when using displayValue)',
suppressConsoleLogs(async () => {
packages/@headlessui-react/src/hooks/use-controllable.ts
+19 -1
View File
@@ -1,4 +1,4 @@
import { useState } from 'react'
import { useRef, useState } from 'react'
import { useEvent } from './use-event'
export function useControllable<T>(
@@ -7,7 +7,25 @@ export function useControllable<T>(
defaultValue?: T
) {
let [internalValue, setInternalValue] = useState(defaultValue)
let isControlled = controlledValue !== undefined
let wasControlled = useRef(isControlled)
let didWarnOnUncontrolledToControlled = useRef(false)
let didWarnOnControlledToUncontrolled = useRef(false)
if (isControlled && !wasControlled.current && !didWarnOnUncontrolledToControlled.current) {
didWarnOnUncontrolledToControlled.current = true
wasControlled.current = isControlled
console.error(
'A component is changing from uncontrolled to controlled. This may be caused by the value changing from undefined to a defined value, which should not happen.'
)
} else if (!isControlled && wasControlled.current && !didWarnOnControlledToUncontrolled.current) {
didWarnOnControlledToUncontrolled.current = true
wasControlled.current = isControlled
console.error(
'A component is changing from controlled to uncontrolled. This may be caused by the value changing from a defined value to undefined, which should not happen.'
)
}
return [
(isControlled ? controlledValue : internalValue)!,
packages/@headlessui-react/src/test-utils/suppress-console-logs.ts
+13
View File
@@ -15,3 +15,16 @@ export function suppressConsoleLogs<T extends unknown[]>(
}).finally(() => spy.mockRestore())
}
}
export function mockingConsoleLogs<T extends unknown[]>(
cb: (spy: jest.SpyInstance, ...args: T) => unknown,
type: FunctionPropertyNames<typeof globalThis.console> = 'error'
) {
return (...args: T) => {
let spy = jest.spyOn(globalThis.console, type).mockImplementation(jest.fn())
return new Promise<unknown>((resolve, reject) => {
Promise.resolve(cb(spy, ...args)).then(resolve, reject)
}).finally(() => spy.mockRestore())
}
}
packages/@headlessui-vue/src/components/combobox/combobox.test.ts
+43 -1
View File
@@ -5381,7 +5381,7 @@ describe('Mouse interactions', () => {
)
it(
'should sync the input field correctly and reset it when resetting the value from outside',
'should sync the input field correctly and reset it when resetting the value from outside (to null)',
suppressConsoleLogs(async () => {
renderTemplate({
template: html`
@@ -5417,6 +5417,48 @@ describe('Mouse interactions', () => {
})
)
it(
'should sync the input field correctly and reset it when resetting the value from outside (to undefined)',
suppressConsoleLogs(async () => {
renderTemplate({
template: html`
<Combobox v-model="value">
<ComboboxInput />
<ComboboxButton>Trigger</ComboboxButton>
<ComboboxOptions>
<ComboboxOption value="alice">alice</ComboboxOption>
<ComboboxOption value="bob">bob</ComboboxOption>
<ComboboxOption value="charlie">charlie</ComboboxOption>
</ComboboxOptions>
</Combobox>
<button @click="value = undefined">reset</button>
`,
setup: () => ({ value: ref('bob') }),
})
// Open combobox
await click(getComboboxButton())
// Verify the input has the selected value
expect(getComboboxInput()?.value).toBe('bob')
// Override the input by typing something
await type(word('alice'), getComboboxInput())
expect(getComboboxInput()?.value).toBe('alice')
// Select the option
await press(Keys.ArrowUp)
await press(Keys.Enter)
expect(getComboboxInput()?.value).toBe('alice')
// Reset from outside
await click(getByText('reset'))
// Verify the input is reset correctly
expect(getComboboxInput()?.value).toBe('')
})
)
it(
'should sync the input field correctly and reset it when resetting the value from outside (when using displayValue)',
suppressConsoleLogs(async () => {