Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

throwOnError with await returns nullable type #801

Open
2 tasks done
dogukanakkaya opened this issue Jun 29, 2023 · 8 comments
Open
2 tasks done

throwOnError with await returns nullable type #801

dogukanakkaya opened this issue Jun 29, 2023 · 8 comments
Labels
bug Something isn't working

Comments

@dogukanakkaya
Copy link

dogukanakkaya commented Jun 29, 2023

Bug report

  • I confirm this is a bug with Supabase, not with my own application.
  • I confirm I have searched the Docs, GitHub Discussions, and Discord.

Describe the bug

.throwOnError should return non-nullable type because since in a case of error it will throw instead of returning an error object with null data object.

To Reproduce

See below code for example:

const { data, error } = await supabase.from('groups').insert({ name }).select('id').throwOnError().single()

In this example return type is nullabe { data: { id: string } } | null but it shouldn't because it'll throw on error.

In this case I have to do below to correct typing which doesn't make any sense to do extra if checks even though it's unnecessary:

const { data, error } = await supabase.from('groups').insert({ name }).select('id').throwOnError().single()

if (error) throw error

Expected behavior

If .throwOnError called return type must be { data: NonNullable<...>, error: null } or even without error object if possible.

Screenshots

Screenshot 2023-06-29 at 12 47 37

System information

  • OS: MacOS
  • Version of supabase-js: 2.26.0
  • Version of Node.js: 20.3.1
  • Version of Typescript: 5.1.3
@dogukanakkaya dogukanakkaya added the bug Something isn't working label Jun 29, 2023
@wallpants
Copy link

Thank you for opening this issue. I was about to open an issue for this exact thing.

@rfreydi
Copy link

rfreydi commented Oct 12, 2023

One workaround until it's resolved:

const get = async (id: string) => {
  const entity = await supabase.client.from('auth.users').select(`*`)
    .eq('id', id)
    .single()
    .throwOnError();

  return entity.data as NonNullable<typeof entity.data>;
}

@mmkal
Copy link

mmkal commented Nov 9, 2023

I opened a PR to support this: supabase/postgrest-js#494

I got impatient so I published it under @rebundled/postgrest-js. Here's how you can use it with @supabase/ssr:

npm install @rebundled/postgrest-js
import { CookieOptions, createServerClient } from '@supabase/ssr'
import { PostgrestClient } from '@rebundled/postgrest-js'
import { cookies } from 'next/headers'
import { Database } from '~/db'

export const createClient = () => {
  const client = createServerClient<Database>(
    process.env.NEXT_PUBLIC_SUPABASE_URL!,
    process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!,
    {
      cookies: {
        get(name: string) {
          return cookieStore.get(name)?.value
        },
      },
    },
  )

  // @ts-expect-error `.rest` is protected
  const { rest } = client
  const wrapper = new PostgrestClient<Database>(rest.url, {
    fetch: rest.fetch,
    headers: rest.headers,
    schema: rest.schemaName,
  }).throwOnError()
  return Object.assign(wrapper, {
    rest,
    realtime: client.realtime,
    auth: client.auth,
  })
}

You can then use like:

import {createClient} from '..'

export default async () => {
  const client = createClient()

  const { data: user } = await client.from('users').select('*').single()

  console.log(`Hello ${user.username}`) // no need to check truthiness/ throw manually
}

If you need access to the non-throwing client, you can use createClient().rest. Note that if the PR is merged, it'll probably make sense to make a change in this repo too, so explicitly creating a new PostgrestClient wouldn't be necessary. But this seemed like a non-invasive way to let people try this out now.

@AlbertMarashi
Copy link

+1. this is kinda annoying tbh

@ThatGuySam
Copy link

Would changing the functionality of .throwOnError() break existing apps that depend on null to represent that the data wasn't found, for example with .single().throwOnError()?

One alternative could just be more chainable throw methods, such as:

throwOnEmpty()
throwOnFalsy()
throwOn( function(){} )

@AlbertMarashi
Copy link

I don't see why it would be backwards compatible to return non-nullable when returning an array of items. maybeSingle should stay nullable, and single should be non-nullable

@yatoogamii
Copy link

Any update on this feature ?

@TomasHubelbauer
Copy link

In addition to supabase/postgrest-js#494 there is also supabase/postgrest-js#364. That PR is also in limbo, linking here just because it is relevant to the issue at hand. Out of curiosity, does anyone see the issue with returning directly the value of data with throwOnError() instead of { data }? I can come up with one problem which is queries where you want to get count not data, but in my experience, going straight to .data is so common, it would be nice to see some wrapper/helper specifically for that and it makes even more sense when couple with throwOnError where you know either the data is good or the control flow took the exception path so you can't make a mistake by not handling the error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants