Nexus: Is it possible to constrain optional input fields to only one of null | undefined?

Created on 14 May 2020  路  9Comments  路  Source: graphql-nexus/nexus

Currently all our optional/nullable input fields are outputting as, for example, field?: string | null.

null | undefined unions make for messy code and are usually considered bad practice in TS. I can understand that some people have use cases where undefined | null are distinguished from each other; however, we'd like to constrain these to just one or the other, probably null since that concept matches graphQL.

Is this possible with Nexus? Graphqlgen does allow this: by setting a default value of null, the input type will only be (for example) string | null. That doesn't seem to work with Nexus as far as I can tell.

Here's the code we're using to pull server-side types from the Nexus definitions:

export type NexusTypeOf<A> = A extends core.NexusInputObjectTypeDef<infer B>
  ? GetGen2<"inputTypes", B>
  : A extends core.NexusObjectTypeDef<infer C>
  ? GetGen2<"fieldTypes", C>
  : never;
efformodest needdiscussion scoptypegen typimprove

Most helpful comment

Is there really something in the gql spec about "if not passed, give undefined, if passed as null, give null"? I would doubt it, I believe that passed null or not passed anything should be treated as null.

As Sytten pointed out, that is indeed the current behavior, hence my previous comment too. Simply removing undefined from the types without doing any runtime transformation of the inputs would simply leave holes in the type-safety of your API.

Now I do think that we could offer this nonetheless @Weakky since we already wrap the resolvers anyway we could cast the null to undefined or vice-versa. This would obviously be an optional flag of nexus, but I think a lot of people would use it because I agree that the prop?: Type | null is pretty annoying sometimes.

Oh yeah, I fully agree it's annoying. We're having bad times with the Prisma Client too because the Prisma client does differentiate null and undefined. That means in lots of cases, you have to do the casting manually when passing GraphQL inputs down to the Prisma Client.

Doing additional runtime transformation to transform the inputs fields from undefined to null might be a good idea to make the dev experience smoother. It'd be great to be able to do that as a plugin, but the plugin architecture currently doesn't enable messing with the generated type definitions.

All 9 comments

The code that needs to be changed is in the type printer since only this one has access to the value of the default. This seems reasonable to me, it should not be a big change.

Something (maybe obvious) to be considered for this feature: Removing undefined or null from the generated type definitions will essentially no longer properly model what your inputs can be, potentially causing bugs.

The root problem here is that GraphQL doesn't differentiate _nullability_ from _optionality_. All nullable fields can technically return null | undefined and graphql-js will validate it as it doesn't violate the schema.

Let's say you remove undefined from all nullable fields. GraphQL will nonetheless give you undefined values for fields that weren't inputted at all. Is that really what you want? What happens then if your developers assume nullable fields can only be null | X and thus create strict conditions like if (x === null) {} else {} ?

@Weakky That doesn't make sense to me. If a field in GQL is nullable it just means you don't have to pass it. Similar to Postgres - if it's nullable then it doesn't have to have a value. Null=no value.

So not passing a value should result in null, not undefined. But if the underlying GQL implementation passes back both null and undefined for different circumstances, which seems bad, I could still see how that would present a compatibility challenge to make it different in Nexus. I don't know whether that's the case.

Is there really something in the gql spec about "if not passed, give undefined, if passed as null, give null"? I would doubt it, I believe that passed null or not passed anything should be treated as null.

@lukewlms I don't agree with you there.
The behaviour of graphql.js is that if the user explicitly sends a null it will be of type null otherwise it will be undefined. This is great because you have basically a REST PATCH instead of just a PUT. This is very important if you tend to do large mutations with a lot of inputs (say updateUserInfo with all the fields).

Now I do think that we could offer this nonetheless @Weakky since we already wrap the resolvers anyway we could cast the null to undefined or vice-versa. This would obviously be an optional flag of nexus, but I think a lot of people would use it because I agree that the prop?: Type | null is pretty annoying sometimes.

Is there really something in the gql spec about "if not passed, give undefined, if passed as null, give null"? I would doubt it, I believe that passed null or not passed anything should be treated as null.

As Sytten pointed out, that is indeed the current behavior, hence my previous comment too. Simply removing undefined from the types without doing any runtime transformation of the inputs would simply leave holes in the type-safety of your API.

Now I do think that we could offer this nonetheless @Weakky since we already wrap the resolvers anyway we could cast the null to undefined or vice-versa. This would obviously be an optional flag of nexus, but I think a lot of people would use it because I agree that the prop?: Type | null is pretty annoying sometimes.

Oh yeah, I fully agree it's annoying. We're having bad times with the Prisma Client too because the Prisma client does differentiate null and undefined. That means in lots of cases, you have to do the casting manually when passing GraphQL inputs down to the Prisma Client.

Doing additional runtime transformation to transform the inputs fields from undefined to null might be a good idea to make the dev experience smoother. It'd be great to be able to do that as a plugin, but the plugin architecture currently doesn't enable messing with the generated type definitions.

@Sytten @Weakky That's helpful, thank you for the additional information! Constraining to null would still be quite useful in our use case.

I'd add that the null | undefined is especially annoying when combined with Nexus Plugin Prisma.
If I have a CRUD operation t.crud.myModel, I want to directly pass through args like args.where to prisma.myModel.findUnique({ where: args.where }) in my resolver, but it is not possible in strict TS mode because of null/undefined mixes.

Not sure if it is a feature or a bug :P
Allowing users to control the where input seems like a recipe for disaster to me from an access security perspective...

That's why we have the FieldAuthorize plugin, in my opinion.
Otherwise, there's just no way to let clients pass date/string filters without a TON of if-elses.
Also - this is the current behavior with the default resolver. If you override the resolver, then you get stuck with these TS problems :)

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ryands17 picture ryands17  路  4Comments

DregondRahl picture DregondRahl  路  3Comments

tonyfromundefined picture tonyfromundefined  路  3Comments

jasonkuhrt picture jasonkuhrt  路  3Comments

capaj picture capaj  路  5Comments