types: exclude null and undefined from useResult pick fn and generics #1233
Conversation
This file contains hidden or 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
|
P.S. I still feel like I'm learning how to write good PR descriptions. I'd appreciate if there's any feedback on how I wrote this. Mainly concerned about being too wordy, or not giving enough info that's helpful. |
|
I'm assuming the build failure isn't an issue since the target branch's build is still failing too for the same reason |
this allows you to not have to write useResult<ResultType | undefined>(...), because we can assume the result type is undefined until it is fetched. If the result type already has undefined, then this is a noop and should be fine.
now it'll work better with useSubscription too, since that is typed as `TResult | null | undefined`
ferm10n
changed the title
Akryum
reviewed
Sep 20, 2021
| @@ -17,7 +17,7 @@ export type UseResultReturn<T> = Readonly<Ref<Readonly<T>>> | |||
| * @returns Readonly ref with `undefined` or the resolved `result`. | |||
| */ | |||
| export function useResult<TResult, TResultKey extends keyof NonNullable<TResult> = keyof NonNullable<TResult>> ( | |||
| result: Ref<TResult> | |||
| result: Ref<TResult | null | undefined> | |||
|
Thanks for the PR! I fixed the original issue in a different way in 4475805 |
|
Thanks for taking a look at this. I'm concerned with the DeepNonNullable type though. Isn't it possible for a query to return a null somewhere nested in the response? I haven't verified this yet this is actually an issue yet though. |
|
That's what the try catch is for |
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.
I was excited to learn about #1062 which fixed the potentially undefined result.value, but ran into an issue with the types in
useResult(). Here's the exampleTo get the types to work, I have to provide values for the generics which is a bit verbose, but fine:
But now that the
result.valueis correctly typed, it reveals that the type of thedataargument in the pick function needs to be fixed. The pick function will never be called if its value is undefined, so data should exclude undefined from its type. This PR is just a small change to implement that.