supabase / supabase-js

An isomorphic Javascript client for Supabase. Query your Supabase database, subscribe to realtime events, upload and download files, browse typescript examples, invoke postgres functions via rpc, invoke supabase edge functions, query pgvector.
https://supabase.com
MIT License
2.86k stars 220 forks source link

[Feature Request] - Option for nulls as undefined #244

Open prescience-data opened 2 years ago

prescience-data commented 2 years ago

Feature request - Option for nulls as undefined

Currently, all empty values returned by the API are set as null.

The difference between null and undefined for many (obviously not all) TypeScript devs is meaningless, leading many to drop the null type completely. (In current projects I go as far as enforcing the Microsoft no-new-null eslint rule for example.)

This isn't meant to trigger any debate over if that is "right or wrong", just that undeniably (and for whatever reasons) some developers have this particular preference for type simplification.

Describe the solution you'd like

a) A configuration option in the SupbaseClient that coerces all nulls to undefined. b) A server-side option to send null values as undefined pre-flight. (edit: https://github.com/supabase/supabase-js/issues/244#issuecomment-905116552)

Describe alternatives you've considered

Presently I need to run each record through a recursive function to convert each null value and adjust the type shape accordingly.

Additional context

A specific case where this is especially problematic:

import { z } from "zod"

export const FooSchema = z.object({
  id: z.number().optional(),
  title: z.string(),
  color: z.string().optional().default("red")
})

const { data, error } = await supabase.from(`foos`).select(`id, title, color`).single()
if (data) { 
  const foo = FooSchema.parse(foo)
}
ZodError.ts:134 Uncaught (in promise) ZodError: [
  {
    "code": "invalid_type",
    "expected": "string",
    "received": "null",
    "path": [
      "color"
    ],
    "message": "Expected string, received null"
  }
]

Normally this would convert an empty value for "color" to "red", however due to the fact that null !== undefined it fails.

soedirgo commented 2 years ago

+1, I realized after the fact that e.g. the TypeScript coding guidelines (caveat: read the title) uses only undefined as the bottom type and forbids null. There isn't much point in having 2 bottom types and I think we also use undefined in a few places.

Rather than making it a configuration though, I'd prefer to apply this for supabase-js v2 instead. What do you think?

prescience-data commented 2 years ago

That sounds awesome. 😍 After thinking and reading about this a bit more, I retract my "option b (server-side)" as I could see this mode may easily break other non-TypeScript languages. To "steel-man" the other side, there is an argument to be made when dealing with database data that a Prisma team member has made here, however, I think so long as the transformation only applies to the JS library I personally think the benefits (sindresorhus, Crockford) of simplification outweigh the cons.

(ps: I always love reading that warning on the TS guidelines page, they must be so sick of this debate by now 🤣)

danielpox commented 1 year ago

One thing to note regarding JSON is that undefined does not exist, whereas null is a value.

For instance, JSON.parse('undefined') throws an error, but JSON.parse(null) returns null.

When stringifying a JavaScript object into JSON, all key/value-pairs with the value of undefined will be skipped, whereas pairs with the value of null will be included.

Here's an example: JSON.stringify({ empty: null, hide: undefined }) returns '{"empty":null}'. This is very handy when you want to send back as little data as possible to the client, by removing all fields "without value".

null is considered a "value", as it represents "nothing", "empty" or "void", but undefined means it doesn't exist at all.

So, when retrieving data via Supabase (or Postgrest), we may get a whole lot of fields being null, when we may want them to be excluded altogether, depending on the use case.

We could of course use some recursive function to do this manually, but it would be nice if it would be baked in to the library via an options parameter.

So, it's not just a question about TypeScript or schema validations, because it returns a completely different dataset in JSON depending on whether it's null or undefined!

soedirgo commented 1 year ago

@danielpox thanks for the input! Yes, in https://github.com/supabase/postgrest-js/pull/278 we decided to adopt the Prisma team's stance on this. We do use undefined instead of null for function args etc., but not for Postgres values.

NiklasPor commented 5 months ago

Hi all, while I also would love this feature from TypeScript / client side, it would also be awesome if the payload sent by the backend could be stripped of null values. Rows / queries which return a lot of nulls often include way more key/value pairs (where the value is null) then they need to.

antoniormrzz commented 3 months ago

With each passing day, I'm coming back to this issue, wondering if this is the reason that zod in trpc output is failing. Am I correct to assume that supabase now returns undefined for null? and that the typegen in cli doesn't account for this by generating return types as well? While the response may be typed, I am using supabase-to-zod and then using the zod result in trpc output validation.

NiklasPor commented 3 months ago

With each passing day, I'm coming back to this issue, wondering if this is the reason that zod in trpc output is failing. Am I correct to assume that supabase now returns undefined for null? and that the typegen in cli doesn't account for this by generating return types as well? While the response may be typed, I am using supabase-to-zod and then using the zod result in trpc output validation.

The author describes the implementation as "hacky" and the last release was a year ago. I'd be careful in building on it in general.

But I think the idea of it is superb.

antoniormrzz commented 3 months ago

The author describes the implementation as "hacky" and the last release was a year ago. I'd be careful in building on it in general.

But I think the idea of it is superb.

The idea of anything zod is repulsive to me. Unfortunately I was not behind that decision. zod was put in to use tRPC, and then supabase-to-zod to generate zod validators. I am a NestJS type of gentleman myself. Class-validator rules.