From c142420b370e579976b0b2ab43c41e713e93d0de Mon Sep 17 00:00:00 2001 From: avallete Date: Tue, 7 Jan 2025 11:55:22 +0900 Subject: [PATCH 1/4] 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 --- src/PostgrestBuilder.ts | 46 ++++++++++++++++++++++++++++++++++------- test/index.test-d.ts | 45 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 83 insertions(+), 8 deletions(-) diff --git a/src/PostgrestBuilder.ts b/src/PostgrestBuilder.ts index 621785c9..05a9f0d3 100644 --- a/src/PostgrestBuilder.ts +++ b/src/PostgrestBuilder.ts @@ -1,11 +1,22 @@ // @ts-ignore import nodeFetch from '@supabase/node-fetch' -import type { Fetch, PostgrestSingleResponse } from './types' +import type { Fetch } from './types' import PostgrestError from './PostgrestError' -export default abstract class PostgrestBuilder - implements PromiseLike> +export default abstract class PostgrestBuilder + implements + PromiseLike< + ThrowOnError extends true + ? { data: Result; count: number | null; status: number; statusText: string } + : { + data: Result | null + error: PostgrestError | null + count: number | null + status: number + statusText: string + } + > { protected method: 'GET' | 'HEAD' | 'POST' | 'PATCH' | 'DELETE' protected url: URL @@ -42,9 +53,9 @@ export default abstract class PostgrestBuilder * * {@link https://github.com/supabase/supabase-js/issues/92} */ - throwOnError(): this { + throwOnError(): PostgrestBuilder { this.shouldThrowOnError = true - return this + return this as PostgrestBuilder } /** @@ -56,9 +67,30 @@ export default abstract class PostgrestBuilder return this } - then, TResult2 = never>( + then< + TResult1 = ThrowOnError extends true + ? { data: Result; count: number | null; status: number; statusText: string } + : { + data: Result | null + error: PostgrestError | null + count: number | null + status: number + statusText: string + }, + TResult2 = never + >( onfulfilled?: - | ((value: PostgrestSingleResponse) => TResult1 | PromiseLike) + | (( + value: ThrowOnError extends true + ? { data: Result; count: number | null; status: number; statusText: string } + : { + data: Result | null + error: PostgrestError | null + count: number | null + status: number + statusText: string + } + ) => TResult1 | PromiseLike) | undefined | null, onrejected?: ((reason: any) => TResult2 | PromiseLike) | undefined | null diff --git a/test/index.test-d.ts b/test/index.test-d.ts index 50a2444d..bf016d40 100644 --- a/test/index.test-d.ts +++ b/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' @@ -211,3 +212,45 @@ const postgrest = new PostgrestClient(REST_URL) expectType(y) expectType(z) } + +// 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>(true) + let err: PostgrestError | null + expectType>(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 + // @ts-expect-error error property does not exist when using throwOnError() + const { error } = result + let expected: + | { + username: string + messages: { + id: number + message: string | null + }[] + }[] + expectType>(true) + // just here so our variable isn't unused + error +} From f627a9987d6738168fe1aeeb7b90461f23e43d26 Mon Sep 17 00:00:00 2001 From: avallete Date: Tue, 7 Jan 2025 12:23:28 +0900 Subject: [PATCH 2/4] chore: re-use generic types --- src/PostgrestBuilder.ts | 32 ++++++-------------------------- src/PostgrestTransformBuilder.ts | 12 ++++++------ test/index.test-d.ts | 3 +-- 3 files changed, 13 insertions(+), 34 deletions(-) diff --git a/src/PostgrestBuilder.ts b/src/PostgrestBuilder.ts index 05a9f0d3..86a4cc48 100644 --- a/src/PostgrestBuilder.ts +++ b/src/PostgrestBuilder.ts @@ -1,21 +1,13 @@ // @ts-ignore import nodeFetch from '@supabase/node-fetch' -import type { Fetch } from './types' +import type { Fetch, PostgrestSingleResponse, PostgrestResponseSuccess } from './types' import PostgrestError from './PostgrestError' export default abstract class PostgrestBuilder implements PromiseLike< - ThrowOnError extends true - ? { data: Result; count: number | null; status: number; statusText: string } - : { - data: Result | null - error: PostgrestError | null - count: number | null - status: number - statusText: string - } + ThrowOnError extends true ? PostgrestResponseSuccess : PostgrestSingleResponse > { protected method: 'GET' | 'HEAD' | 'POST' | 'PATCH' | 'DELETE' @@ -69,27 +61,15 @@ export default abstract class PostgrestBuilder + : PostgrestSingleResponse, TResult2 = never >( onfulfilled?: | (( value: ThrowOnError extends true - ? { data: Result; count: number | null; status: number; statusText: string } - : { - data: Result | null - error: PostgrestError | null - count: number | null - status: number - statusText: string - } + ? PostgrestResponseSuccess + : PostgrestSingleResponse ) => TResult1 | PromiseLike) | undefined | null, diff --git a/src/PostgrestTransformBuilder.ts b/src/PostgrestTransformBuilder.ts index 4791702c..2be085c8 100644 --- a/src/PostgrestTransformBuilder.ts +++ b/src/PostgrestTransformBuilder.ts @@ -192,7 +192,7 @@ export default class PostgrestTransformBuilder< ResultOne = Result extends (infer ResultOne)[] ? ResultOne : never >(): PostgrestBuilder { this.headers['Accept'] = 'application/vnd.pgrst.object+json' - return this as PostgrestBuilder + return this as unknown as PostgrestBuilder } /** @@ -212,7 +212,7 @@ export default class PostgrestTransformBuilder< this.headers['Accept'] = 'application/vnd.pgrst.object+json' } this.isMaybeSingle = true - return this as PostgrestBuilder + return this as unknown as PostgrestBuilder } /** @@ -220,7 +220,7 @@ export default class PostgrestTransformBuilder< */ csv(): PostgrestBuilder { this.headers['Accept'] = 'text/csv' - return this as PostgrestBuilder + return this as unknown as PostgrestBuilder } /** @@ -228,7 +228,7 @@ export default class PostgrestTransformBuilder< */ geojson(): PostgrestBuilder> { this.headers['Accept'] = 'application/geo+json' - return this as PostgrestBuilder> + return this as unknown as PostgrestBuilder> } /** @@ -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[]> - else return this as PostgrestBuilder + if (format === 'json') return this as unknown as PostgrestBuilder[]> + else return this as unknown as PostgrestBuilder } /** diff --git a/test/index.test-d.ts b/test/index.test-d.ts index bf016d40..7f24a575 100644 --- a/test/index.test-d.ts +++ b/test/index.test-d.ts @@ -240,7 +240,6 @@ const postgrest = new PostgrestClient(REST_URL) .limit(1) .throwOnError() const { data } = result - // @ts-expect-error error property does not exist when using throwOnError() const { error } = result let expected: | { @@ -251,6 +250,6 @@ const postgrest = new PostgrestClient(REST_URL) }[] }[] expectType>(true) - // just here so our variable isn't unused + expectType>(true) error } From 42ca31a23b8585ad1e5eaa90979dbbc93f57e579 Mon Sep 17 00:00:00 2001 From: avallete Date: Tue, 7 Jan 2025 12:31:48 +0900 Subject: [PATCH 3/4] fix: return this to comply with PostgresFilterBuilder --- src/PostgrestBuilder.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/PostgrestBuilder.ts b/src/PostgrestBuilder.ts index 86a4cc48..2c3863e2 100644 --- a/src/PostgrestBuilder.ts +++ b/src/PostgrestBuilder.ts @@ -45,9 +45,9 @@ export default abstract class PostgrestBuilder { + throwOnError(): this & PostgrestBuilder { this.shouldThrowOnError = true - return this as PostgrestBuilder + return this as this & PostgrestBuilder } /** From 6a3ff51c3f2c8e13d5e3a22f1f9690432919da56 Mon Sep 17 00:00:00 2001 From: avallete Date: Tue, 7 Jan 2025 12:53:31 +0900 Subject: [PATCH 4/4] chore: fix test to check inheritance and not equality --- test/index.test-d.ts | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/test/index.test-d.ts b/test/index.test-d.ts index 7f24a575..745b8c40 100644 --- a/test/index.test-d.ts +++ b/test/index.test-d.ts @@ -209,8 +209,8 @@ const postgrest = new PostgrestClient(REST_URL) const x = postgrest.from('channels').select() const y = x.throwOnError() const z = x.setHeader('', '') - expectType(y) - expectType(z) + expectType(true) + expectType(true) } // Should have nullable data and error field @@ -253,3 +253,26 @@ const postgrest = new PostgrestClient(REST_URL) expectType>(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>(true) + expectType>(true) + error +}