Skip to content

Commit

Permalink
Fix XYZPropsWeControl and cleanup internal TypeScript types (#2329)
Browse files Browse the repository at this point in the history
* cleanup `XYZPropsWeControl`

The idea behind the `PropsWeControl` is that we can omit all the fields
that we are controlling entirely. In this case, passing a prop like
`role`, but if we already set the role ourselves then the prop won't do
anything at all. This is why we want to alert the end user that it is an
"error".

It can also happen that we "control" the value by default, but keep
incoming props into account. For example we generate a unique ID for
most components, but you can provide your own to override it. In this
case we _don't_ want to include the ID in the `XYZPropsWeControl`.

Additionally, we introduced some functionality months ago where we call
event callbacks (`onClick`, ...) from the incoming props before our own
callbacks. This means that by definition all `onXYZ` callbacks can be
provided.

* improve defining types

Whenever we explicitly provide custom types for certain props, then we
make sure to omit those keys first from the original props (of let's say
an `input`). This is important so that TypeScript doesn't try to "merge"
those types together.

* cleanup: move `useEffect`

* add `defaultValue` explicitly

* ensure tests are not using `any` because of `onChange={console.log}`

The `console.log` is typed as `(...args: any[]) => void` which means
that it will incorrectly mark its incoming data as `any` as well.
Converting it to `x => console.log(x)` makes TypeScript happy. Or in
this case, angry since it found a bug.

This is required because it _can_ be that your value (e.g.: the value of
a Combobox) is an object (e.g.: a `User`), but it is also nullable.

Therefore we can provide the value `null`. This would mean that
eventually this resolves to `keyof null` which is `never`, but we just
want a string in this case.

```diff
-export type ByComparator<T> = (keyof T & string) | ((a: T, b: T) => boolean)
+export type ByComparator<T> =
+  | (T extends null ? string : keyof T & string)
+  | ((a: T, b: T) => boolean)
```

* improve the internal types of the `Combobox` component

* improve the internal types of the `Disclosure` component

* improve the internal types of the `Listbox` component

* improve the internal types of the `Menu` component

* improve the internal types of the `Popover` component

* improve the internal types of the `Tabs` component

* improve the internal types of the `Transition` component

* use `Override` in `Hidden` as well

* cleanup unused code

* don't check the `useSyncExternalStoreShimClient`

* don't check the `useSyncExternalStoreShimServer`

* improve types in the render tests

* fix `Ref<TTag>` to be `Ref<HTMLElement>`

* improve internal types of the `Transition` component (Vue)

+ add `attrs.class` as well

* use different type for `AnyComponent`

* update changelog
  • Loading branch information
RobinMalfait authored Mar 2, 2023
1 parent 948ae73 commit 989cd6b
Show file tree
Hide file tree
Showing 23 changed files with 386 additions and 400 deletions.
1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Ensure `Transition` component completes if nothing is transitioning ([#2318](https://github.com/tailwindlabs/headlessui/pull/2318))
- Enable native label behavior for `<Switch>` where possible ([#2265](https://github.com/tailwindlabs/headlessui/pull/2265))
- Allow root containers from the `Dialog` component in the `FocusTrap` component ([#2322](https://github.com/tailwindlabs/headlessui/pull/2322))
- Fix `XYZPropsWeControl` and cleanup internal TypeScript types ([#2329](https://github.com/tailwindlabs/headlessui/pull/2329))

## [1.7.12] - 2023-02-24

Expand Down
218 changes: 109 additions & 109 deletions packages/@headlessui-react/src/components/combobox/combobox.test.tsx

Large diffs are not rendered by default.

78 changes: 37 additions & 41 deletions packages/@headlessui-react/src/components/combobox/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import React, {
createRef,
useCallback,
useContext,
useEffect,
useMemo,
useReducer,
useRef,
Expand All @@ -14,7 +15,6 @@ import React, {
MouseEvent as ReactMouseEvent,
MutableRefObject,
Ref,
useEffect,
} from 'react'
import { ByComparator, EnsureArray, Expand, Props } from '../../types'

Expand Down Expand Up @@ -385,31 +385,31 @@ export type ComboboxProps<

function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_TAG>(
props: ComboboxProps<TValue, true, true, TTag>,
ref: Ref<TTag>
ref: Ref<HTMLElement>
): JSX.Element
function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_TAG>(
props: ComboboxProps<TValue, true, false, TTag>,
ref: Ref<TTag>
ref: Ref<HTMLElement>
): JSX.Element
function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_TAG>(
props: ComboboxProps<TValue, false, false, TTag>,
ref: Ref<TTag>
ref: Ref<HTMLElement>
): JSX.Element
function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_TAG>(
props: ComboboxProps<TValue, false, true, TTag>,
ref: Ref<TTag>
ref: Ref<HTMLElement>
): JSX.Element

function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_TAG>(
props: ComboboxProps<TValue, boolean | undefined, boolean | undefined, TTag>,
ref: Ref<TTag>
ref: Ref<HTMLElement>
) {
let {
value: controlledValue,
defaultValue,
onChange: controlledOnChange,
name,
by = (a: any, z: any) => a === z,
by = (a: TValue, z: TValue) => a === z,
disabled = false,
__demoMode = false,
nullable = false,
Expand Down Expand Up @@ -440,16 +440,18 @@ function ComboboxFn<TValue, TTag extends ElementType = typeof DEFAULT_COMBOBOX_T
let buttonRef = useRef<_Data['buttonRef']['current']>(null)
let optionsRef = useRef<_Data['optionsRef']['current']>(null)

type TActualValue = true extends typeof multiple ? EnsureArray<TValue>[number] : TValue
let compare = useEvent(
// @ts-expect-error Eventually we'll want to tackle this, but for now this will do.
typeof by === 'string'
? (a, z) => {
let property = by as unknown as keyof TValue
? (a: TActualValue, z: TActualValue) => {
let property = by as unknown as keyof TActualValue
return a?.[property] === z?.[property]
}
: by
)

let isSelected: (value: unknown) => boolean = useCallback(
let isSelected: (value: TValue) => boolean = useCallback(
(compareValue) =>
match(data.mode, {
[ValueMode.Multi]: () =>
Expand Down Expand Up @@ -695,23 +697,24 @@ interface InputRenderPropArg {
disabled: boolean
}
type InputPropsWeControl =
| 'role'
| 'aria-labelledby'
| 'aria-expanded'
| 'aria-activedescendant'
| 'aria-autocomplete'
| 'onKeyDown'
| 'onChange'
| 'displayValue'
| 'aria-controls'
| 'aria-expanded'
| 'aria-labelledby'
| 'disabled'
| 'role'

export type ComboboxInputProps<TTag extends ElementType, TType> = Props<
TTag,
InputRenderPropArg,
InputPropsWeControl
> & {
displayValue?(item: TType): string
onChange?(event: React.ChangeEvent<HTMLInputElement>): void
}
InputPropsWeControl,
{
defaultValue?: TType
displayValue?(item: TType): string
onChange?(event: React.ChangeEvent<HTMLInputElement>): void
}
>

function InputFn<
TTag extends ElementType = typeof DEFAULT_INPUT_TAG,
Expand Down Expand Up @@ -1010,15 +1013,12 @@ interface ButtonRenderPropArg {
value: any
}
type ButtonPropsWeControl =
// | 'type' // While we do "control" this prop we allow it to be overridden
| 'tabIndex'
| 'aria-haspopup'
| 'aria-controls'
| 'aria-expanded'
| 'aria-haspopup'
| 'aria-labelledby'
| 'disabled'
| 'onClick'
| 'onKeyDown'
| 'tabIndex'

export type ComboboxButtonProps<TTag extends ElementType> = Props<
TTag,
Expand Down Expand Up @@ -1131,13 +1131,8 @@ interface LabelRenderPropArg {
open: boolean
disabled: boolean
}
type LabelPropsWeControl = 'ref' | 'onClick'

export type ComboboxLabelProps<TTag extends ElementType> = Props<
TTag,
LabelRenderPropArg,
LabelPropsWeControl
>
export type ComboboxLabelProps<TTag extends ElementType> = Props<TTag, LabelRenderPropArg>

function LabelFn<TTag extends ElementType = typeof DEFAULT_LABEL_TAG>(
props: ComboboxLabelProps<TTag>,
Expand Down Expand Up @@ -1175,18 +1170,18 @@ let DEFAULT_OPTIONS_TAG = 'ul' as const
interface OptionsRenderPropArg {
open: boolean
}
type OptionsPropsWeControl = 'aria-labelledby' | 'hold' | 'onKeyDown' | 'role' | 'tabIndex'
type OptionsPropsWeControl = 'aria-labelledby' | 'aria-multiselectable' | 'role' | 'tabIndex'

let OptionsRenderFeatures = Features.RenderStrategy | Features.Static

export type ComboboxOptionsProps<TTag extends ElementType> = Props<
TTag,
OptionsRenderPropArg,
OptionsPropsWeControl
> &
OptionsPropsWeControl,
PropsForFeatures<typeof OptionsRenderFeatures> & {
hold?: boolean
}
>

function OptionsFn<TTag extends ElementType = typeof DEFAULT_OPTIONS_TAG>(
props: ComboboxOptionsProps<TTag>,
Expand Down Expand Up @@ -1263,16 +1258,17 @@ interface OptionRenderPropArg {
selected: boolean
disabled: boolean
}
type ComboboxOptionPropsWeControl = 'role' | 'tabIndex' | 'aria-disabled' | 'aria-selected'
type OptionPropsWeControl = 'role' | 'tabIndex' | 'aria-disabled' | 'aria-selected'

export type ComboboxOptionProps<TTag extends ElementType, TType> = Props<
TTag,
OptionRenderPropArg,
ComboboxOptionPropsWeControl | 'value'
> & {
disabled?: boolean
value: TType
}
OptionPropsWeControl,
{
disabled?: boolean
value: TType
}
>

function OptionFn<
TTag extends ElementType = typeof DEFAULT_OPTION_TAG,
Expand Down
10 changes: 5 additions & 5 deletions packages/@headlessui-react/src/components/dialog/dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -118,21 +118,21 @@ let DEFAULT_DIALOG_TAG = 'div' as const
interface DialogRenderPropArg {
open: boolean
}
type DialogPropsWeControl = 'role' | 'aria-modal' | 'aria-describedby' | 'aria-labelledby'
type DialogPropsWeControl = 'role' | 'aria-describedby' | 'aria-labelledby' | 'aria-modal'

let DialogRenderFeatures = Features.RenderStrategy | Features.Static

export type DialogProps<TTag extends ElementType> = Props<
TTag,
DialogRenderPropArg,
DialogPropsWeControl
> &
DialogPropsWeControl,
PropsForFeatures<typeof DialogRenderFeatures> & {
open?: boolean
onClose(value: boolean): void
initialFocus?: MutableRefObject<HTMLElement | null>
__demoMode?: boolean
}
>

function DialogFn<TTag extends ElementType = typeof DEFAULT_DIALOG_TAG>(
props: DialogProps<TTag>,
Expand Down Expand Up @@ -402,7 +402,7 @@ let DEFAULT_OVERLAY_TAG = 'div' as const
interface OverlayRenderPropArg {
open: boolean
}
type OverlayPropsWeControl = 'aria-hidden' | 'onClick'
type OverlayPropsWeControl = 'aria-hidden'

export type DialogOverlayProps<TTag extends ElementType> = Props<
TTag,
Expand Down Expand Up @@ -454,7 +454,7 @@ let DEFAULT_BACKDROP_TAG = 'div' as const
interface BackdropRenderPropArg {
open: boolean
}
type BackdropPropsWeControl = 'aria-hidden' | 'onClick'
type BackdropPropsWeControl = 'aria-hidden'

export type DialogBackdropProps<TTag extends ElementType> = Props<
TTag,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ export type DisclosureProps<TTag extends ElementType> = Props<TTag, DisclosureRe

function DisclosureFn<TTag extends ElementType = typeof DEFAULT_DISCLOSURE_TAG>(
props: DisclosureProps<TTag>,
ref: Ref<TTag>
ref: Ref<HTMLElement>
) {
let { defaultOpen = false, ...theirProps } = props
let internalDisclosureRef = useRef<HTMLElement | null>(null)
Expand Down Expand Up @@ -247,14 +247,15 @@ let DEFAULT_BUTTON_TAG = 'button' as const
interface ButtonRenderPropArg {
open: boolean
}
type ButtonPropsWeControl =
// | 'type' // We allow this to be overridden
'aria-expanded' | 'aria-controls' | 'onKeyDown' | 'onClick'
type ButtonPropsWeControl = 'aria-controls' | 'aria-expanded'

export type DisclosureButtonProps<TTag extends ElementType> = Props<
TTag,
ButtonRenderPropArg,
ButtonPropsWeControl
ButtonPropsWeControl,
{
disabled?: boolean
}
>

function ButtonFn<TTag extends ElementType = typeof DEFAULT_BUTTON_TAG>(
Expand Down
Loading

0 comments on commit 989cd6b

Please sign in to comment.