supabase / postgrest-js

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

Return `undefined` instead of `null` for `data` and `error` result properties #367

Open mstade opened 1 year ago

mstade commented 1 year ago

Feature request

I considered opening a PR for this, but thought I should open it up for discussion first, because it would in fact be a breaking change.

Is your feature request related to a problem? Please describe.

Sort of. When calling supabase and there's no data in the response, probably because there was an error, data will be set to null. Likewise, error will be set to null if there's no error in the response. This makes it easy to destructure into a {data, error} tuple, but it unfortunately also makes it a little awkward to apply defaults, for example if you don't care for the error, or want to destructure the returned values. For example, this doesn't work:

const { data: [userprofile] = [], error } = await supabase
  .from('userprofile')
  .select('id, metadata, identities')
  .eq(`name`, name)

This will always cause an error when data is null, because null is not iterable. The = [] default does nothing, because it'd only apply when data is undefined.

This particular code can be fixed fairly easily, by adding the relatively new maybeSingle modifier:

const { data: userprofile, error } = await supabase
  .from('userprofile')
  .select('id, metadata, identities')
  .eq(`name`, name)
  .maybeSingle()

This may seem contrived, and this particular case is easily solved, but when data and error always return as null and never as undefined, it's pretty much impossible to destructure these objects further because doing so will always cause errors one way another. So you have to store away the data and error variables to destructure them later, like so:

const { data, error } = await supabase
  .from('userprofile')
  .select('id, metadata, identities')
  .eq(`name`, name)

const [userprofile] = data ?? []

Not that big of a deal, until you start having more than one query in the same scope:

const { data: data1 } = await supabase.from('a').select('stuff')
const { data: data2 } = await supabase.from('b').select('moarstuff')

const { stuff } = data1 ?? {}
const { moar } = data2 ?? {}

const togetherNow = { ...stuff, ...moar }

If data instead was returned as undefined, the above could be written more succinctly:

const { data: { stuff } = {} } = await supabase.from('a').select('stuff')
const { data: { moar } = {} } = await supabase.from('b').select('moarstuff')

const togetherNow = { ...stuff, ...moar }

Describe the solution you'd like

Return undefined instead of null for data and error, when it makes sense. E.g. data is never defined when there's an error, and error is never defined when there's data.

Describe alternatives you've considered

Alternatives are described above, essentially use the nullish operation on the returned result properties.

Additional context

Presumably this should be considered a breaking change. Most checks on these properties probably do simple falsy checks, or optional chaining, e.g.:

if (!data) {
  // Do stuff
}

const whatever = data?.stuff
const { things } = data ?? {}
const [first, ...rest] = data ?? []

If however, someone does a strict check for null, their code will break:

if (data === null) {
  // This won't work if data is undefined
}
soedirgo commented 1 year ago

Hey, sorry for the late response, and thanks for the detailed proposal!

So we actually made the change for supabase-js v2, but it was reverted to make it consistent with all the other client libraries.

This makes it easy to destructure into a {data, error} tuple, but it unfortunately also makes it a little awkward to apply defaults

TIL! Somehow I've survived years of JS without knowing you can set defaults like this 😅

As you mentioned, this would be a breaking change, so it's likely slated for supabase-js v3 - but this is one of the items under consideration, not just for postgrest-js but also the other libs.