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

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

Open
prescience-data opened this issue Aug 24, 2021 · 9 comments
Open

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

prescience-data opened this issue Aug 24, 2021 · 9 comments
Assignees
Labels
enhancement New feature or request

Comments

@prescience-data
Copy link

prescience-data commented Aug 24, 2021

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: #244 (comment))

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.

@prescience-data prescience-data added the enhancement New feature or request label Aug 24, 2021
@soedirgo
Copy link
Member

+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
Copy link
Author

prescience-data commented Aug 25, 2021

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
Copy link

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
Copy link
Member

soedirgo commented Jul 25, 2022

@danielpox thanks for the input! Yes, in supabase/postgrest-js#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
Copy link

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
Copy link

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
Copy link

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
Copy link

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.

@veloware
Copy link

veloware commented Sep 11, 2024

Also running into this, and its an absolute pain. Im building forms that have optional fields, and in typescript, that pretty simple with -

type FormData = {
   foo?: string;
}

but constantly running into supabase problems with it complaining that I cannot assign string | undefined to string | null

I think this would be relatively easy to coerce using something like -

undefined ?? null    // null

but would be awesome if this could be a config in the lib as mentioned above :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants