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
nullable inputs/args are not covered in the docs very well #1236
Comments
It's just a trade-off over simplicity vs. correctness. In runtime, for input you might get However, declaring union type
That's why in docs it is made simpler:
Because all the |
@MichalLytek thanks for the response - totally agree its a trade off but becomes really important when doing things like updates where null can mean clearing a field vs undefined is no change (this is where I have been really badly bitten now!) I think we need a section in the docs that covers this clearly - because I haven't noticed this in the 2 years I've heavily used type-graphql (honestly very surprised I haven't noticed it earlier either tbh)... |
additional to previous comment; it makes sense to simplify this for Fields of an ObjectType, but for InputType and Args it is important to distinguish that null is a totally valid input when fields/args are marked |
That's possible, try to find if it's described somewhere and make a PR with the changes over that "nullable" topic 😉 |
Would be happy to make that contribution if that's ok. But first I need some sleep and then I've got a few args and input types that urgently need fixing up in a project to handle nulls! 😭 (updated link in issue with example of a input type from the validation section of docs) |
@MichalLytek 👋 I thought I'd add some additional bits I've subsequently found... The following is an example of why the optional chaining wasn't working as expected. In my previous example everything would be fine - but this is a better representation of what was happening in my case: // SomeResolver.ts
@Query(() => SomeResponse)
async allThings(
@Arg('filter', { nullable: true }) filter?: SomeFilter,
): Promise<Response<ISomething>> {
this.someService.foo(filter);
}
// SomeService.ts
foo(filter?: Filter = {}) {
console.log(filter?.search); // `{}` or `Filter` right?
} But because I previously didn't realize The types are all good, there's no issues on the surface until you call the GraphQL server with: {
allThings(filter: null) {
id
}
} and voila; I have also been working through my types and resolvers updating things that should allow null to allow null and restricting those that don't. I have been using // SomeResolver.ts
@Query(() => SomeResponse)
async allThings(
@Arg('filter', () => SomeFilter || null, { nullable: true }) filter?: SomeFilter | null,
): Promise<Response<ISomething>> {
this.someService.foo(filter)
} Without specifying the return type of
This is also the case for @InputType()
export class UpdatePayload {
// `id` is required when updating
@Field()
id!: string;
// `name` can either be undefined if unchanged or string when changing
@Field({ nullable: true })
@NotEquals(null)
name?: string;
// `description` can be one of undefined if unchanged, string when changing or null when clearing
@Field(() => String || null, { nullable: true })
description?: string | null;
} This also requires the This is a bit more than I was expecting to have to do for this and feels like I'm uncovering a bit more of a gap here? 🤔
Thanks |
This is a runtime expression, JS value that is evaluated, just like
This is a TypeScript limitation, I can't do anything about it. |
No worries, I figured it was something like that - thank you for taking the time to read & respond! 👍 |
Lol, spent whole morning due to just lacking of an example for explicit type
The pain point is, for |
Use @field() with @notequals(null) to value not undefined |
Firstly, this feels a bit stupid to be noticing after using type-graphql for 2 years but here it goes...
Describe the issue
The docs seemingly made it clear to me that having nullable was equivalent to an optional type... e.g.
and/or
and this is mostly true for fields but it comes to args/inputs its misleading ... as I've just found out while debugging an issue in my project.
The correct typescript types for this should be
and/or
I haven't seen this mentioned in the docs (and it feels like I'm stating the obvious) but when using nullable,
null
is a valid input for args/fields/etc and I've just realized a bunch of code with optional chaining is very broken now 😆Are you able to make a PR that fix this?
Would be good to get it confirmed that I'm not crazy in noticing this and it's not already mentioned somewhere I haven't seen... its a bit late where I am and I've also been chasing this issue all afternoon only to realize this now... 😭
Additional context
Specifically, I've been working on Query with the following signature...
and just realized this is a very valid GQL query for it:
and with this,
console.log(filter?.search)
will now error because"Cannot read properties of null (reading 'search')"
The text was updated successfully, but these errors were encountered: