fix(types): type result for throwOnError responses (#590)

* fix(types): type result for throwOnError responses

When using throwOnError(), the response type is now more strictly typed:
- Data is guaranteed to be non-null
- Error field is removed from response type
- Response type is controlled by generic ThrowOnError boolean parameter

Fixes #563

* chore: re-use generic types

* fix: return this to comply with PostgresFilterBuilder

* chore: fix test to check inheritance and not equality

src/PostgrestBuilder.ts

@@ -1,11 +1,14 @@
 // @ts-ignore
 import nodeFetch from '@supabase/node-fetch'
 
-import type { Fetch, PostgrestSingleResponse } from './types'
+import type { Fetch, PostgrestSingleResponse, PostgrestResponseSuccess } from './types'
 import PostgrestError from './PostgrestError'
 
-export default abstract class PostgrestBuilder<Result>
-  implements PromiseLike<PostgrestSingleResponse<Result>>
+export default abstract class PostgrestBuilder<Result, ThrowOnError extends boolean = false>
+  implements
+    PromiseLike<
+      ThrowOnError extends true ? PostgrestResponseSuccess<Result> : PostgrestSingleResponse<Result>
+    >
 {
   protected method: 'GET' | 'HEAD' | 'POST' | 'PATCH' | 'DELETE'
   protected url: URL
@@ -42,9 +45,9 @@ export default abstract class PostgrestBuilder<Result>
    *
    * {@link https://github.com/supabase/supabase-js/issues/92}
    */
-  throwOnError(): this {
+  throwOnError(): this & PostgrestBuilder<Result, true> {
     this.shouldThrowOnError = true
-    return this
+    return this as this & PostgrestBuilder<Result, true>
   }
 
   /**
@@ -56,9 +59,18 @@ export default abstract class PostgrestBuilder<Result>
     return this
   }
 
-  then<TResult1 = PostgrestSingleResponse<Result>, TResult2 = never>(
+  then<
+    TResult1 = ThrowOnError extends true
+      ? PostgrestResponseSuccess<Result>
+      : PostgrestSingleResponse<Result>,
+    TResult2 = never
+  >(
     onfulfilled?:
-      | ((value: PostgrestSingleResponse<Result>) => TResult1 | PromiseLike<TResult1>)
+      | ((
+          value: ThrowOnError extends true
+            ? PostgrestResponseSuccess<Result>
+            : PostgrestSingleResponse<Result>
+        ) => TResult1 | PromiseLike<TResult1>)
       | undefined
       | null,
     onrejected?: ((reason: any) => TResult2 | PromiseLike<TResult2>) | undefined | null

src/PostgrestTransformBuilder.ts

@@ -192,7 +192,7 @@ export default class PostgrestTransformBuilder<
     ResultOne = Result extends (infer ResultOne)[] ? ResultOne : never
   >(): PostgrestBuilder<ResultOne> {
     this.headers['Accept'] = 'application/vnd.pgrst.object+json'
-    return this as PostgrestBuilder<ResultOne>
+    return this as unknown as PostgrestBuilder<ResultOne>
   }
 
   /**
@@ -212,23 +212,23 @@ export default class PostgrestTransformBuilder<
       this.headers['Accept'] = 'application/vnd.pgrst.object+json'
     }
     this.isMaybeSingle = true
-    return this as PostgrestBuilder<ResultOne | null>
+    return this as unknown as PostgrestBuilder<ResultOne | null>
   }
 
   /**
    * Return `data` as a string in CSV format.
    */
   csv(): PostgrestBuilder<string> {
     this.headers['Accept'] = 'text/csv'
-    return this as PostgrestBuilder<string>
+    return this as unknown as PostgrestBuilder<string>
   }
 
   /**
    * Return `data` as an object in [GeoJSON](https://geojson.org) format.
    */
   geojson(): PostgrestBuilder<Record<string, unknown>> {
     this.headers['Accept'] = 'application/geo+json'
-    return this as PostgrestBuilder<Record<string, unknown>>
+    return this as unknown as PostgrestBuilder<Record<string, unknown>>
   }
 
   /**
@@ -285,8 +285,8 @@ export default class PostgrestTransformBuilder<
     this.headers[
       'Accept'
     ] = `application/vnd.pgrst.plan+${format}; for="${forMediatype}"; options=${options};`
-    if (format === 'json') return this as PostgrestBuilder<Record<string, unknown>[]>
-    else return this as PostgrestBuilder<string>
+    if (format === 'json') return this as unknown as PostgrestBuilder<Record<string, unknown>[]>
+    else return this as unknown as PostgrestBuilder<string>
   }
 
   /**

test/index.test-d.ts

@@ -1,5 +1,6 @@
+import { TypeEqual } from 'ts-expect'
 import { expectError, expectType } from 'tsd'
-import { PostgrestClient } from '../src/index'
+import { PostgrestClient, PostgrestError } from '../src/index'
 import { Prettify } from '../src/types'
 import { Database, Json } from './types'
 
@@ -208,6 +209,70 @@ const postgrest = new PostgrestClient<Database>(REST_URL)
   const x = postgrest.from('channels').select()
   const y = x.throwOnError()
   const z = x.setHeader('', '')
-  expectType<typeof x>(y)
-  expectType<typeof x>(z)
+  expectType<typeof y extends typeof x ? true : false>(true)
+  expectType<typeof z extends typeof x ? true : false>(true)
+}
+
+// Should have nullable data and error field
+{
+  const result = await postgrest.from('users').select('username, messages(id, message)').limit(1)
+  let expected:
+    | {
+        username: string
+        messages: {
+          id: number
+          message: string | null
+        }[]
+      }[]
+    | null
+  const { data } = result
+  const { error } = result
+  expectType<TypeEqual<typeof data, typeof expected>>(true)
+  let err: PostgrestError | null
+  expectType<TypeEqual<typeof error, typeof err>>(true)
+}
+
+// Should have non nullable data and no error fields if throwOnError is added
+{
+  const result = await postgrest
+    .from('users')
+    .select('username, messages(id, message)')
+    .limit(1)
+    .throwOnError()
+  const { data } = result
+  const { error } = result
+  let expected:
+    | {
+        username: string
+        messages: {
+          id: number
+          message: string | null
+        }[]
+      }[]
+  expectType<TypeEqual<typeof data, typeof expected>>(true)
+  expectType<TypeEqual<typeof error, null>>(true)
+  error
+}
+
+// Should work with throwOnError middle of the chaining
+{
+  const result = await postgrest
+    .from('users')
+    .select('username, messages(id, message)')
+    .throwOnError()
+    .eq('username', 'test')
+    .limit(1)
+  const { data } = result
+  const { error } = result
+  let expected:
+    | {
+        username: string
+        messages: {
+          id: number
+          message: string | null
+        }[]
+      }[]
+  expectType<TypeEqual<typeof data, typeof expected>>(true)
+  expectType<TypeEqual<typeof error, null>>(true)
+  error
 }