supabase / postgrest-js

Isomorphic JavaScript client for PostgREST.
https://supabase.com
MIT License
1.04k stars 133 forks source link

Improve return types for `throwOnError` #364

Open Cryrivers opened 1 year ago

Cryrivers commented 1 year ago

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 https://github.com/supabase/supabase-js/issues/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 commented 1 year ago

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

TomasHubelbauer commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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