Add a type discriminator to result types

[gordonbrander]

Description

Adding a type discriminator such as an ok: boolean property to result types such as PostgrestResponseSuccess would make them discriminated unions. This has a number of advantages. You can use if (!res.ok) { ... } to do type narrowing. It also lets the type checker ensure that success and error are mutually exclusive, and there is not some third state of { data: null, error: null }.

Suggested solution

For example,

supabase-js/packages/core/postgrest-js/src/types/types.ts

Line 15 in 8d1e0c5

export interface PostgrestResponseSuccess<T> extends PostgrestResponseBase {

could become:

export interface PostgrestResponseSuccess<T> extends PostgrestResponseBase {
  ok: true
  error: null
  data: T
  count: number | null
}
export interface PostgrestResponseFailure extends PostgrestResponseBase {
  ok: false
  error: PostgrestError
  data: null
  count: null
}

export type PostgrestSingleResponse<T> = PostgrestResponseSuccess<T> | PostgrestResponseFailure

Alternative

No response

Additional context

Old Typescript handbook doc on discriminating unions https://www.typescriptlang.org/docs/handbook/unions-and-intersections.html#discriminating-unions

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that request the same feature to avoid creating a duplicate.

[nancysangani]

Hi! Can I take this issue? Just want to confirm the ok: boolean discriminator approach is still acceptable before I start.

[gordonbrander]

A quick note: I was just digging through Zod's docs and noticed that Zod's .safeParse() result type uses the same field names as supabase-js results (data and error). It uses success as the name of the type discriminator field.

type Result<T, E> = {
  success: true;
  data: T;
} | {
  success: false;
  error: E
};

Could be worth using the same field name as Zod?

[nancysangani]

Thanks for the suggestion! Using success as the discriminator makes sense for consistency with Zod. I can implement the change using success: true | false for the response types. Should I proceed with a PR?

[gordonbrander]

(I'm not part of the Supabase team, but I hope the answer is yes)