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

Improve return types for throwOnError #364

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Cryrivers
Copy link

@Cryrivers Cryrivers commented Nov 8, 2022

What kind of change does this PR introduce?

This PR improves return types for throwOnError, which excludes null for failure responses in the return type. as throwOnError raises exceptions instead of providing a failure response.

What is the current behavior?

Currently throwOnError still types the return data as Data | null, whereas the null (failure response) case is impossible to happen.

Screenshot 2022-11-08 at 12 34 35 PM

CleanShot 2022-11-08 at 12 28 45@2x

What is the new behavior?

throwOnError just types the return data as Data (null is still kept if the data is nullable), if error happens, it throws an exception.

Screenshot 2022-11-08 at 12 37 19 PM

Screenshot 2022-11-08 at 12 37 55 PM

Additional context

This PR addresses supabase/supabase-js#554

TypeScript issue

TypeScript (4.8, 4.9) complains the circular references, however the types can be correctly inferred in Visual Studio Code.

src/PostgrestBuilder.ts:59:19 - error TS2577: Return type annotation circularly references itself.

59   throwOnError(): EnableThrowOnError<this> {

@Cryrivers
Copy link
Author

i'm still figuring out how to type it correctly.

@TomasHubelbauer
Copy link
Contributor

@Cryrivers Is there anything I can do to help get this over the line? I would also like to see this fix. Your last message says you're still figuring out the types. What is left to figure out to make the result unwrapping work after throwOnError?

@Cryrivers
Copy link
Author

@Cryrivers Is there anything I can do to help get this over the line? I would also like to see this fix. Your last message says you're still figuring out the types. What is left to figure out to make the result unwrapping work after throwOnError?

right now the blocker is the TypeScript error: Return type annotation circularly references itself. I tried other workarounds with no luck. I'm not sure how to bypass the limitation of TypeScript at the moment. :(

@khill-fbmc
Copy link

I don't know much, but could this help at all with the types?

It claims to be super fast, recursive, and 1:1 with TypeScript

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

Successfully merging this pull request may close these issues.

3 participants