supabase / postgrest-js

Isomorphic JavaScript client for PostgREST.
https://supabase.com
MIT License
964 stars 128 forks source link

fix: throwOnError function call should return response success type always #454

Open dogukanakkaya opened 11 months ago

dogukanakkaya commented 11 months ago

What kind of change does this PR introduce?

This PR improves the typing of .throwOnError function. Which should return PostgrestResponseSuccess<T> type.

What is the current behavior?

Right now .throwOnError is returning nullable data.

What is the new behavior?

If .throwOnError called on the query it will always return PostgrestResponseSuccess<T> instead of PostgrestResponseSuccess<T> | PostgrestResponseFailure

Additional context

This is a breaking change because previously you could call other functions after .throwOnError because it does not execute the query but only sets the shouldThrowOnError property to true to be used later on when executing the query.

With this change, chaining more functions after .throwOnError is not possible since it executes the query and check the result and throws in case of error or return the PostgrestResponseSuccess in case of success.

I saw this PR right after I finished working on that. Which resolves the same issue with Typescript only instead, which I also tried at first since it gives developers more flexibility since we don't necessarily have to execute the query after .throwOnError. But looks like that PR is freezed/stuck since Nov 2022 because of a Typescript issue.

Also related with: https://github.com/supabase/supabase-js/issues/801

wyozi commented 10 months ago

In my codebase I use a helper called dataOrThrow. It does what it says, i.e. do throwOnError but also change the return value to the .data property. It would be a potential option if avoiding breaking changes was a goal, although I think it would be a good addition to the codebase in the first place