Infer Combobox type based on onChange handler
#3798
Merged
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.
This PR fixes an issue where the
Comboboxonly inferred the type of the internal value based on thevalueprop.We only wanted to infer the type of the Combobox based on the
valueprop to make it less confusing where values came from. But if you are using an uncontrolled component then thevalueis not provided to theCombobox.In most cases this isn't an actual issue, but it is if you also want to us the
byprop to improve comparing values based on a certain property.Without this change, the only way of typing the
byprop correctly is by using an explicit type on theCombobox:With this change, the internal value type will be inferred by the
onChangemeaning that thebyprop is properly typed now.Test plan
Given a reproduction where the
byprop is correct if you look at the type of theonChange:Before:
After:
There is no output because there are no errors.
Given a reproduction where the
byprop is incorrect if you look at the type of theonChange:Before:
After:
The error type is much simpler and easier to reason about. It's not as clean as a simple
'id' | 'name'because thebyprop can also be a function. But at least there isn't a wall of TypeScript errors you have to reason about. Maybe we can improve this part in a future PR.Fixes: #3636