Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ts] selectedItem type definition for controlled usage #1088

Closed
mattrothenberg opened this issue Jun 17, 2020 · 4 comments · Fixed by #1090 or #1104
Closed

[ts] selectedItem type definition for controlled usage #1088

mattrothenberg opened this issue Jun 17, 2020 · 4 comments · Fixed by #1090 or #1104

Comments

@mattrothenberg
Copy link

  • downshift version: latest
  • node version: 12.31.
  • npm (or yarn) version: 1.21.

Relevant code or config

interface Item = {
  label: string;
  value: string;
}
const [selectedItem, setSelectedItem] = React.useState<Item|null>(null);
const props = useCombobox({selectedItem});

What you did:
In trying to control a Combobox by passing a null value to it, I get a TS error since the type definition of selectedItem expects either an Item or undefined. As far as I understand from reading the docs, passing an undefined value here suggests that you want to use the combobox in an uncontrolled manner, which is not what I'm after.

https://github.com/downshift-js/downshift/blob/master/typings/index.d.ts#L417

 selectedItem?: Item // Null values are not allowed here, meaning you get a TS error when you try to control this prop.

What happened:

Type '{ value: string; label: string; } | null' is not assignable to type 'ItemBase | undefined'.
  Type 'null' is not assignable to type 'ItemBase | undefined'.ts(2322)

Reproduction repository:
https://codesandbox.io/s/unruffled-elion-1fx0s?file=/src/App.tsx

Suggested solution:
Should the type definition for selectedItem be expanded to include null types as well?

@silviuaavram
Copy link
Collaborator

Makes sense, all typings for the hooks in this case should be updated. Will this be a breaking change?

@mattrothenberg
Copy link
Author

@silviuaavram Great question. It's difficult to say whether it will be a breaking change, since the current type definitions are arguably already "broken" for Typescript users who wish to control the selectedItem prop and have probably resorted to things like // @ts-ignore in order to suppress these compilation errors.

Let me reach out to some TS wizards to see what the ideal Type definition should be here. Something feels off about simply expanding the type thusly, but I'm admittedly not super knowledgable here 👇

interface UseComboboxParams {
    selectedItem?: Item | null;
}

@silviuaavram
Copy link
Collaborator

That would be awesome. Thanks @mattrothenberg !

@silviuaavram silviuaavram mentioned this issue Jun 27, 2020
5 tasks
silviuaavram added a commit that referenced this issue Jul 21, 2020
**What**:

Update downshift to v6.

**Why**:

Introduce breaking changes that fix the issues below.
Fixes #1088.
Fixes #1015.
Fixes #1010.
Fixes #719.

**How**:

**The list of breaking changes:**

BREAKING CHANGE: Update TS typings for `selectedItem` to accept `null` in both `useSelect` and `useCombobox`.

To migrate to the new change, update your types or code if necessary. `selectedItem`, `defaultSelectedItem` and `initialSelectedItem` now have `Item | null` instead of `Item` type. PR with the changes: #1090


BREAKING CHANGE: Update TS typings for `itemToString` to accept `null` for the `item` parameter, in `useSelect` and `useCombobox` + in `Downshift` where this was missing. `useMultipleSelection` type for `itemToString` stays the same as it can't receive `null` as `item`.

To migrate to the new change, update your types or code if necessary. `itemToString: (item: Item) => string` -> `itemToString: (item: Item | null) => string}`. PR with the changes: #1075 #1105


BREAKING CHANGE: Pass `type` to the onChange (onInputValueChange, onHighlightedIndexChange, onSelectedItemChange, onIsOpenChange) handler parameters, as specified in the documentation. Also updated the TS typings to reflect this + `onStateChange` - the `type` parameter was passed but it was not reflected in the TS types.

To migrate to the new change, update your types or code if necessary, better to view the changes in the PR: #985.


BREAKING BEHAVIOUR [useCombobox]: When an item is highlighted by keyboard and user closes the menu using mouse/touch, the item is not selected anymore. The only selection on Blur happens using either Tab / Shift+Tab. PR with the changes: #1109


BREAKING BEHAVIOUR [useCombobox & downshift]: When pressing Escape and the menu is open, only close the menu. When the menu is closed and there is an item selected and/or text in the input, clear the selectedItem and the inputValue. PR with the changes: #719
@silviuaavram
Copy link
Collaborator

🎉 This issue has been resolved in version 6.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants