fix(ts): specify that itemToString can receive null #1075
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This makes it so the argument passed to `itemToString` can be `null` for the following interfaces: - `UseSelectProps` - `UseComboboxProps` - `Actions` This makes them behave in line with various documentation, which states that they can receive null: - [`useCombobox#itemToString`](https://github.com/downshift-js/downshift/tree/master/src/hooks/useCombobox#itemtostring) - [`useSelect#itemToString`](https://github.com/downshift-js/downshift/tree/master/src/hooks/useSelect#itemtostring) - [Downshift component](https://github.com/downshift-js/downshift#itemtostring) I didn’t touch `useMultipleSelection`, because its documentation didn’t have this line: [`useMultipleSelection#itemToString`](https://github.com/downshift-js/downshift/tree/master/src/hooks/useMultipleSelection#itemtostring) I was also not sure about `itemToString` passed to the various `A11y`-functions, so I didn’t touch these either. This fix has previously been applied to the main Downshift component: downshift-js#505 This will be a breaking change for people using typescript with `strictNullChecks` turned on if they don’t already check for null. I will leave that decision with you, though.
Codecov Report
@@ Coverage Diff @@
## master #1075 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 17 17
Lines 1319 1319
Branches 241 241
=========================================
Hits 1319 1319 Continue to review full report at Codecov.
|
|
Awesome! Probably we need to have it in |
No, it actually looks like it's not the case in |
|
Oh, you are right. Marked this as a Breaking Change and will add it to the v6 branch when I will start it. Thank you! |
This was referenced Jun 27, 2020
Merged
silviuaavram
added a commit
that referenced
this pull request
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
This was referenced Jun 6, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What:
This makes it so the argument passed to
itemToStringcan benullfor the following interfaces:UseSelectPropsUseComboboxPropsActionsWhy:
This makes them behave in line with various documentation, which states that they can receive null:
useCombobox#itemToStringuseSelect#itemToStringHow:
I looked for all instances for
itemToStringin the typescript types and looked up their documentation to determine which ones have been specified to potentially benull. I then added| nullto the relevant ones, which will make it so the argument can either be of theItemtype ornull.Checklist:
I didn’t touch
useMultipleSelection, because its documentation didn’t have this line:useMultipleSelection#itemToStringI was also not sure about
itemToStringpassed to the variousA11y-functions, so I didn’t touch these either.This fix has previously been applied to the main Downshift component: #505
This will be a breaking change for people using typescript with
strictNullChecksturned on if they don’t already check for null. I will leave that decision with you, though.