Skip to content

__nullable__ vs Type.Optional #33

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

Closed
daedalus28 opened this issue Mar 7, 2025 · 9 comments · Fixed by #35
Closed

__nullable__ vs Type.Optional #33

daedalus28 opened this issue Mar 7, 2025 · 9 comments · Fixed by #35

Comments

@daedalus28
Copy link

Why does prismabox generate types with __nullable__ instead of just Type.Optional? I see there is generator for Type.Optional, but it isn't being used.

For a model with a field like this:

name String?

I am getting this:

name: __nullable__(Type.String({ additionalProperties: false })),

Where as I would expect

name: Type.Optional(Type.String()),

Also - this might be a separate issue, by why would prismabox add { additionalProperties: false } to scalar values like strings and numbers?

@m1212e
Copy link
Owner

m1212e commented Mar 9, 2025

Because prisma in some scenarios allows null OR undefined as types where optional only allows for undefined/is reflected as undefined in TS types

@m1212e
Copy link
Owner

m1212e commented Mar 9, 2025

Regarding the second aspect: This roots from calling the options generator function which injects this based on the global setting. Since this causes no harm its just there and I saved myself the time and complexity of implemention logic to check the necessity

@daedalus28
Copy link
Author

daedalus28 commented Mar 10, 2025

That's fair. For my use case, I'll just transform the output (or remap __nullable__).

For additionalProperties on scalars, it seems like you already have a method that knows when it's a primitive value. Am I right that it's just a small change here to transform options?

}) {
if (["Int", "BigInt"].includes(fieldType)) {

E.g. something like this?:

let optionsObject = JSON.parse(options)
delete optionsObject.additionalProperties
options = JSON.stringify(optionsObject)

I think it might be a real bug because it's included for json columns - so Type.Any() with additionalProperties:false doesn't make any sense.

@daedalus28
Copy link
Author

@m1212e would you be open to a PR with my suggested change?

@m1212e
Copy link
Owner

m1212e commented Mar 11, 2025

True, I think this would work, thats perfectly fine to do imo. I'd be happy to review a PR!

@daedalus28
Copy link
Author

Here you go:

#35

@daedalus28
Copy link
Author

daedalus28 commented Mar 11, 2025

It occurs to me that to properly close this issue, we should probably add some documentation with your explanation about __nullable__ vs Type.Optional.

@daedalus28
Copy link
Author

It occurs to me that to properly close this issue, we should probably add some documentation with your explanation about __nullable__ vs Type.Optional.

Updated.

@bmakan
Copy link

bmakan commented Mar 21, 2025

One spect that __nullable__ removes from Type.Optional is "can be omitted". This is quite annoying when using these types directly in Elysia's type guards.

E.g.:

const user = t.Object({
    name: t.String(),
    age: t.Optional(t.String()),
});

Requires:

{
    "name": "John",
    "age": null
}

instead of

{
    "name": "John"
}

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

Successfully merging a pull request may close this issue.

3 participants