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
fix(types): avoid inferring UiState type from initialUiState #5061
Conversation
This allows someone with custom widgets to still use setUiState etc. without type errors
if you pass initialUiState, that should be asserted by the generic UiState, not automatically accepted. Otherwise you get use cases like the "with function form sets indices state" test where setUiState thinks query is required, just because it has the default given The way this works is by using `NoInfer` from microsoft/TypeScript#14829 (comment) which seems to be quite relied upon This is a follow-up on #5060
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit d51c3ac:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which seems to be quite relied upon
Out of curiosity, how did you measure this?
// this purposely breaks typescript's type inference to ensure it's not used | ||
// as it's used for a default parameter for example | ||
// source: https://github.com/Microsoft/TypeScript/issues/14829#issuecomment-504042546 | ||
type NoInfer<T> = [T][T extends any ? 0 : never]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you see this simplified version microsoft/TypeScript#14829 (comment) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried that last one too, but it made lots of items in the codebase report errors. Not really sure what the difference is though, unfortunately
I measured it by looking at the 20/30 PR references in that typescript issue, which used this method, but also the others in the thread that don't work for this specific use case |
Summary
if you pass initialUiState, that should be asserted by the generic UiState, not automatically accepted. Otherwise you get use cases like the "with function form sets indices state" test where setUiState thinks query is required, just because it has the default given
The way this works is by using
NoInfer
from microsoft/TypeScript#14829 (comment) which seems to be quite relied upon. See also https://stackoverflow.com/a/56688073/3185307This is a follow-up on #5060
Result
ui state is not inferred from initialUiState, but rather enforced by it. This means however that custom widgets are no longer "automatically detected from initialUiState", which I don't think is a breaking change, as it's only a type change