graphql
graphql copied to clipboard
nestjs
`defaultValue` makes property optional and it can be overridden by explicit `null`
I'm submitting a...
[ ] Regression
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.
Current behavior
I am using the code first approach.
TLDR: in the @Field() decorator, the defaultValue option always makes the property nullable (regardless of the nullable option), even though null may not be an acceptable value in an input type.
The @Field() decorator can take an options object to configure the GraphQL property to be nullable or not and to have a default value or not.
Illustration:
@InputType()
export class CreateUserInput {
@Field((type) => [String], { nullable: false, defaultValue: [] }) // nullable: false makes no difference at all
details: string[];
}
The previous code will generate the following schema.
input CreateUserInput {
details: [String!] = []
}
This means that the values in the details array must be non-null strings, that details has a default value of an empty array BUT details itself can be set to null.
This means that ALL the following mutations are valid but the last one should not. details should either be an array or not be present at all.
mutation CreateUser {
explicit: createUser(input: { details: ["Nest", "is", "so", "cool"] })
omitted: createUser(input: {})
explicitly_null: createUser(input: { details: null }) # That's the behaviour I want to exclude
}
Expected behavior
I should be able to define input fields with a default value AND make them required to prevent them from accepting the value null as user input.
The typescript code above should produce the following schema (notice the TWO exclamation marks). Thus, the property does not accept null as an explicit value and uses the default value.
input CreateUserInput {
details: [String!]! = []
}
Minimal reproduction of the problem with instructions
https://github.com/Haltarys/Nest-GraphQL-Bug
What is the motivation / use case for changing the behavior?
I expect the details parameter to always be an array, regardless of the user input.
Environment
Nest version: 7.6.13
For Tooling issues:
- Node version: 15
- Platform: Ubuntu 20.04
Others:
Note: I used an array in an input type to illustrate the problem but it works with any type as long as it is an argument of a query/mutation.
type Query {
getProducts(sortAsc: Boolean = true): [Product!]! # sortAsc can be null
}
# Any of these values can be overridden with null
input UserInput {
age: Int = 42
isAdmin: Boolean = false
name: String = "Bruce Wayne"
}
I should be able to define input fields with a default value AND make them required to prevent them from accepting the value null as user input.
This isn't possible. If you define a default value, then the logic is, you are definitely expecting a null value to come through, thus assigning the default value. Or, put another way, if a field is non-nullable, a default value is useless. You are requiring a value to be given, so the client must pass a value other than null.
The issue seems to be that defaultValue for the array type is always ignored. No matter what is put in defaultValue, if null is passed by the client, null goes through and the defaultValue isn't set.
Scott
@smolinari What I mean by "make them required" is that they cannot accept null. But of course, if they have a default value, then the field is not required per se. But that doesn't mean that it should be nullable.
When working with just express and apollo-server, this schema is perfectly valid.
input A {
age: Int! = 42 # What I want to do
}
input B {
age: Int = 42 # The current behavior
}
In A, age is an Int with a default value of 42. But if I decide to override it, I cannot override it with null.
But in B, I can set age to null which isn't a valid value for my usecase.
That's what I am trying to show here:
mutation CreateUser {
explicit: createUser(input: { details: ["Nest", "is", "so", "cool"] }) # Overriding with correct values
omitted: createUser(input: {}) # Omitting, thus the default value [] is used
explicitly_null: createUser(input: { details: null }) # Overriding with null, which is what I want to prevent
}
@Haltarys - I see what you are getting at. And my comment above is also incorrect. "null" is a value in the end.
My understanding is, as soon as a field is non-nullable, default values are useless.
input A {
age: Int! = 42 # 42 will never be set
In the same vein:
input CreateUserInput {
details: [String!]! = [] # the empty array '[]' will never be set, as details must have an input value (which also can't be null).
}
So, from that logic, you can't have a default value AND not override with null. Or put another way, the non-null field means it is mandatory, thus, default values with non-null fields are useless.
So, reiterating my understanding:
- Nullable fields can be empty, contain null and default values will work, if null isn't sent (so overridable with null).
- Non-nullable fields are mandatory (i.e. can't be empty) and can't have null as a value. Default values have no effect.
That's it. There is no way to have a non-nullable field and a default value and it makes no sense to want this, because of rule 2 above.
Scott
@smolinari That is the problem. age: Int! = 42 IS set.
Non-nullable fields are mandatory (i.e. can't be empty) and can't have null as a value. Default values have no effect.
They do have an effect. Non-nullable fields are not mandatory if they have a default value. They're just non-nullable.
To prove it, I have made a repo with just apollo-server and graphql. Here is the link. Make sure you are on the branch apollo-graphql. There is an example file named example.gql with the queries. You'll see that the one named explicit_null_A is not valid.
I see what you mean, but wanting a non-nullable field with a default still makes no sense to me. The spec even says, non-null fields are required and shouldn't accept ommissions. I'm wondering if this is a bug in Apollo Server???
All that aside, what is the use case where you need a default value on a required field or rather a default value that can't be null overridden?
Scott
The way I see it, being nullable and having a default value are two unrelated metadata (I'm not sure it's the right word but you get it) about a field.
They both allow said field to be omitted: in the first case, the value will be null because nothing was provided, and in the second, it will fall back to the default value.
a default value that can't be null overridden?
That's exactly what I'm trying to do. And the syntax age: Int! = 42 allows exactly that: age can be omitted, and overridden, but not with null. The fact that a field is required or not is tied to my application's logic, not GraphQL. I apologize if my explanation was unclear.
Also, the spec says that all types are nullable by default but @nestjs/graphql makes them non-nullable by default. So it wouldn't be the first time we don't stick perfectly to the spec. (And I think it's better with Nest's way). But I'm rambling.
The spec even says, non-null fields are required and shouldn't accept ommissions. I'm wondering if this is a bug in Apollo Server???
OK. Just to prove the point, I made another GraphQL server using express-graphql. This is based on the official GraphQL tutorial. If that's not spec-compliant, I don't know what is. :wink:
The result is the same. Find it here. Make sure you are on branch express-graphql.
We see the same issue. In Apollo Server, you can have a non-null field with a default value.
The ! only means that the field cannot be null. If a default value is set, the ! does not imply that you must provide an argument in Apollo Server.
The resulting behavior in NestJS is incorrect:
@Args('id', { defaultValue: '42', nullable: false }) // note: nullable: false ignored by NestJS
If the consumer passes in null for this argument, then the resolver gets null as its value. Not the defaultValue! The field is nullable because NestJS ignored nullable: false. null is a value, so therefore the defaultValue doesn't get applied.
NestJS should be generating this GQL, which would work correctly:
(id: String! = "42")
This is a blocker to us on a large internal project.
I think even the GraphQL spec has been changed (or I seriously overlooked it).
In addition to not accepting the value null, it also does not accept omission. For the sake of simplicity nullable types are always optional and non‐null types are always required.
http://spec.graphql.org/June2018/#sec-Type-System.Non-Null
Which clearly defines "null" as not acceptable to a non-null field in an input object or arg.
Scott
As I mentioned in my previous comment, using an officially endorsed package by GraphQL (therefore spec compliant), the expected behaviour can be achieved. So there is definitely a contradiction here, but it'd be nice to be able to use that trick. That would minimise manual input checking.
I think the general issue here is assuming non-null logic and default logic in GraphQL is sufficient for form/ data validation. At least that is the understanding I'm getting from the conversation and looking back at it after all this time and with the knowledge I've gained too. To assume GraphQL non-null fields and default values should be validating in general is incorrect. The GraphQL spec isn't about data validation, but rather field correctness from an API standpoint. So, trying to bend the logic to fit form validation will fail miserably aka it shouldn't be done. So, this isn't a bug, it's a feature IMHO.
Scott
I just upgraded a project from @nestjs/graphql v8 to v12 (with Apollo driver) and noticed the behaviour of defaultValue changed.
The declaration :
@ArgsType()
class MyArgs {
@Field(() => String, { defaultValue: 'foobar' })
foo: string;
}
...in v8 would produce the SDL:
foo: String = "foobar"
In v12 it's now changed to:
foo: String! = "foobar"
This implies the argument is non-nullable, but has a default value if it's omitted. Which is what was originally expected in my example anyway. But if I explicitly add nullable: true to the declaration, I can obtain the previous behaviour if I prefer.
So, I would suggest that this issue has already been resolved?
Regarding the correctness of having a non-nullable field with a default value, I agree with the OP that nullability and optionality are two different things. This is similar to how JavaScript has two separate concepts of null and undefined.
It should be possible to declare an input field or argument that doesn't accept null as a value but which can be omitted and then uses a default value. Omitting an argument is not the same as setting it to null.
The relevant parts of the GraphQL spec say:
§5.4.2.1: Arguments can be required. An argument is required if the argument type is non‐null and does not have a default value. Otherwise, the argument is optional.
§5.6.4: Input object fields may be required. Much like a field may have required arguments, an input object may have required fields. An input field is required if it has a non‐null type and does not have a default value. Otherwise, the input object field is optional.
So, I would suggest that this issue has already been resolved?
I agree.
However, your follow-up logic is missing an important point. Not sending a value (omission) will still fail a non-nullable field. So, what the OP was expecting isn't possible. You can't omit an argument, as it will become null or be considered null and will fail the API validation.
From the spec:
Input Coercion
If an argument or input‐object field of a Non‐Null type is not provided, is provided with the literal value null, or is provided with a variable that was either not provided a value at runtime, or was provided the value null, then a query error must be raised.
IMHO, what breaks minds with this topic is the ability to have a default value on top of the concept of nullable arguments and objects in a GraphQL API. People switch mindframes at that point (notice the OP speaking of "Input from the user"), because they think of setting the default value in terms of their business logic and not really API functionality. If you stick to just the API functionality, nullability, even with the default, is simple.
So to try and help to explain this "simpleness", which I am convinced of, let's first imagine there is no default setting ability at all. Either an argument or object is required to be sent, or it is not, i.e. you can't send a null or you can or you can't omit data or you can. There can be null/nothing nullable: true or there must be data nullable: false. The nullability alone is really easy, right?
Now let's put the ability to add a default value back onto to the nullability feature.
The "API" logic for incoming arguments/ objects is the following:
If nullable: false i.e. non-nullable, a value must be sent to the API and sending null or omitting the data will fail the API validation. The default value is irrelevant.
If a null is allowed to be sent i.e. nullable: true and a null is sent or the field is omitted, and there is a default, then the default is used to fill in a value, so you'll never get a null into that field from the API.
That's it.
If you ask me, I'd suggest to not even do default value logic within the API (i.e. don't use default: in the @Field decorator). Keep it in your user input (business) validation logic and think of API fields as either being required to be sent with data or can be null/ omitted (which with the GraphQL spec, should be the default, but is another huge topic). If you can do this, you'll have a much easier time, because user input validation is in the right place and not mixed up in the API validation logic.
So, I believe (haven't tested it) the solution for the OP is, have a validation pipe set up with transform: true set on the respective resolvers, and do this:
@InputType()
export class CreateUserInput {
@ValidateIf((object, value) => value !== null)
@AnyOtherValidation() // put anything else here for further user input validation
@Field((type) => [String], { nullable: true })
details: string[] = [];
}
The @ValidateIf decorator from class-validator will allow the null/ omitted data to be passed through without any other validation being kicked off and TypeScript will assign the default. If there is a value, it gets handled via the validation logic.
Scott
@smolinari Sorry, but that's just not right. You're conflating two different concepts. Setting an argument to null and omitting an argument entirely are not the same thing.
Not sending a value (omission) will still fail a non-nullable field. So, what the OP was expecting isn't possible. You can't omit an argument, as it will become null or be considered null and will fail the API validation.
Omitting an argument does not mean it is set to null. In the absence of a default value, it means it is not set at all, which in JavaScript may be represented as undefined. When there is a default value, that is what will be used.
Your quote from the GraphQL spec about Input Coercion is describing the low level behaviour of types, without reference to default values. Default values as they apply to arguments and input fields are described later in the spec. I think we can infer that the default value of an omitted argument is applied before input type coercion comes into play.
IMHO, what breaks minds with this topic is the ability to have a default value on top of the concept of nullable arguments and objects in a GraphQL API. People switch mindframes at that point (notice the OP speaking of "Input from the user"), because they think of the default in terms of their business logic and not really API functionality. If you stick to just the API functionality, nullability, even with the default, is simple.
The issue is not about business logic vs API functionality vs user input validation. It's simply about what types a field will accept – specifically, whether a field with a default value can or must also accept null as a value. It is perfectly legitimate within the GraphQL API specification to define a field which has a default value and does not accept null.
As a practical example, let's say I have a food API and I want to create a vegetables query which will return a list of objects of type Vegetable. But some fruits (e.g. tomatoes, cucumber) are sometimes considered to be vegetables and sometimes not. I expect most consumers will want these "fruit vegetables" included, but I want to provide the option of removing them from the result. It's a basic filtering exercise.
I come up with the following:
type Query {
vegetables(includeFruit: Boolean = true): [Vegetable!]!
}
Now as a client I can execute:
query {
vegetables(includeFruit: false) {
id
name
}
}
and my resolver will receive includeFruit with the value false, meaning I should filter out the fruits. Likewise a client can execute vegetables(includeFruit: true) if they want the fruits included, and my resolver will receive includeFruit with the value true.
If a client is happy with the default behaviour, they don't need to pass this argument at all:
query {
vegetables {
id
name
}
}
Here in my resolver includeFruit will be given the default value of true, so I still know not to filter out the fruits.
So far so good. However, under this definition of the field there is another possibility. A client can also do:
query {
vegetables(includeFruit: null) {
id
name
}
}
and my resolver will receive includeFruit with the value null.
Wait! I wasn't expecting that! Now I have to make sure my resolver can handle this non-boolean value which I didn't want to receive. In the case of a boolean, I'm probably doing a truthiness comparison so null will act like false and it won't matter, but if the field were a string or an array it could lead to a runtime error if my resolver were to perform operations on the value without checking for null.
If I make includeFruit non-nullable by changing the definition of the argument to:
type Query {
vegetables(includeFruit: Boolean! = true): [Vegetable!]!
}
then I eliminate the possibility of sending null. Clients can still send true or false explicitly, or omit the argument entirely to obtain the default value. Problem solved.
Setting an argument to null and omitting an argument entirely are not the same thing.
But, they are considered the same in GraphQL. I realize "null" isn't the same as not sending any data for the field. The field is "there", when the data sent is a "null". It's just that GraphQL (the API) considers them the same, as something that meets the nullable requirement.
Your quote from the GraphQL spec about Input Coercion is describing the low level behaviour of types, without reference to default values. Default values as they apply to arguments and input fields are described later in the spec. I think we can infer that the default value of an omitted argument is applied before input type coercion comes into play.
Correct. Though, this doesn't change what I said, does it? If a missing argument/ object or a null is sent for the field, the default is used (if there is one). If a value is set and required (non-nullable), the default is useless.
and my resolver will receive includeFruit with the value false, meaning I should filter out the fruits. Likewise a client can execute vegetables(includeFruit: true) if they want the fruits included, and my resolver will receive includeFruit with the value true
This is 100% business logic and not API validation logic, so an example that misses the point (or makes it, depending on your point of view. .Haha! :grin: ). You are still in a "I want the API itself to validate business logic" mindframe. That's not what it is made to do and trying to fit this thinking onto it will always fail.
Wait! I wasn't expecting that!
Then make the field non-nullable.
but if the field were a string or an array it could lead to a runtime error if my resolver were to perform operations on the value without checking for null.
Did you read my final solution? I edited my post later, so you might have missed it. Basically it is putting the default in the runtime. Problem solved.
or omit the argument entirely to obtain the default value
Not possible. If the value is omitted in a non-nullable field, GraphQL will fail. A value must be sent. A very simple rule that people constantly get wrong and one of the reasons why a lot of people hate GraphQL. They want it to do stuff it really shouldn't. I've been with this for about 6 years. I've been in discussions a couple of years after GraphQL's in the GraphQL repo and one of the biggest concerns of the makers of GraphQL (Lee Byron mainly) was that too much was expected of GraphQL in a lot of the improvement suggestions. Way too often you'd read something along the lines of "that belongs in your business logic layer and not in GraphQL" from Lee. It's why, for instance, authorization logic was never made a part of the spec. It's business logic. People have bent things around GraphQL to still do it. It's a serious waste of time.
Again, having or deciding there should be a default value is a business decision and has little to do with API correctness. The fact you can set a default on a nullable field just means, you'll never ever get a null from the API, which does simplify somethings, but can complicate other situations. In the case of booleans, it's a bit of a conundrum, as you noted. The null would be seen as falsey and graphql, with a default setting the value to true, would break the missing field default logic. And to that "thinking" I say again, it's business logic, not API correctness.
To me, the default feature should be removed from GraphQL, because of the argument you've just made.
If you accept it wasn't built for that kind of validation or default setting (to make the business logic work), and instead, you take care of the validation 100% in your business logic layer, you'll never face that conundrum. :grin:
Scott
But, they are considered the same in GraphQL.
I'm not sure why you say this? I've already pointed to the sections of the specification which explain otherwise, please refer back to earlier comments. 🙂
If a missing argument/ object or a null is sent for the field, the default is used (if there is one). ... The fact you can set a default on a nullable field just means, you'll never ever get a null from the API
Both these statements are incorrect, I'm afraid you've misunderstood how this works. Sending a null value does not cause the default value to be used. Again I remind you that null and omission are different cases! If a field is nullable then you certainly can get a null value passed in, whether there is a default value or not.
This is 100% business logic and not API validation logic, so an example that misses the point (or makes it, depending on your point of view. .Haha! 😁 ). You are still in a "I want the API itself to validate business logic" mindframe. That's not what it is made to do and trying to fit this thinking onto it will always fail.
Absolutely wrong, sorry. We are talking about the static definition of types, which is very much in the scope of the GraphQL API layer and not business logic.
Runtime input validation is very often necessary for other reasons, but one of the great benefits of GraphQL is it's not required for basic type safety. Nullability is an aspect of typing, not validation. You define the types of fields in your schema, and then this metadata is available not just within the server but also to all clients who interact with your API. My client should know whether includeFruit accepts a null value before ever executing a request.
Not possible. If the value is omitted in a non-nullable field, GraphQL will fail. A value must be sent.
It most certainly is possible, when we are talking about arguments or input fields which have a default value. The spec allows for it, and I'm doing it right now in my project. It works exactly as it should.
Again, having or deciding there should be a default value is a business decision and has little to do with API correctness.
That's debatable. But the fact is, default values are a part of the GraphQL specification so the designers must have thought they belonged there. Just as with types, incorporating default values into the schema has the benefit of allowing introspection so clients can see what value is implied by omitting a field.
I'm not sure why you say this? I've already pointed to the sections of the specification which explain otherwise, please refer back to earlier comments. slightly_smiling_face
§5.4.2.1: Arguments can be required. An argument is required if the argument type is non‐null and does not have a default value. Otherwise, the argument is optional.
§5.6.4: Input object fields may be required. Much like a field may have required arguments, an input object may have required fields. An input field is required if it has a non‐null type and does not have a default value. Otherwise, the input object field is optional.
Where do these rules say that an omitted value is not considered as "null" to GraphQL? Or that GraphQL's validation will fail if a null or empty field is sent in a non-nullable field? You've lost me. :thinking:
but one of the great benefits of GraphQL is it's not required for basic type safety. Nullability is an aspect of typing, not validation.
I agree and have said nothing otherwise. Avoiding adding default values in the schema, is what I'm suggesting. A default value for a field has nothing to do with Type safety.
It most certainly is possible, when we are talking about arguments or input fields which have a default value. The spec allows for it, and I'm doing it right now in my project. It works exactly as it should.
So, you have a non-null field and you send a null or no data (field omission) and you get a default value and no error? That would go against what is stated in the coercion section. But, I'm open to learn better (more on that below).
We are talking about the static definition of types, which is very much in the scope of the GraphQL API layer and not business logic.....
API correctness and Type correctness is the same to me. I guess I could have been using that terminology instead.
Let's concentrate on what you are saying works, where I said it shouldn't. Can you show your code please?
Scott
Where do these rules say that an omitted value is not considered as "null" to GraphQL?
They don't - but nor have I seen anything that agrees with you that this is the case. Which makes sense, because that would be an implementation detail outside the scope of the specification. In JavaScript an omitted field will generally become undefined whereas in other languages null or some other equivalent construct may be used.
What those rules do describe is the interaction between default values and optionality. They explain that a non-nullable field with a default value is optional, i.e. can be omitted.
So, you have a non-null field and you send a null or no data (field omission) and you get a default value and no error? That would go against what is stated in the coercion section. But, I'm open to learn better (more on that below).
Yes, exactly. In @nestjs/graphql v12, I can define an argument foo as non-nullable with a default value like:
@ArgsType()
class ExampleArgs {
@Field(() => String, { defaultValue: 'bar', nullable: false })
foo: string;
}
@Resolver()
class ExampleResolver {
@Query(() => String, { nullable: false })
example(@Args() { foo }: ExampleArgs): string {
return foo;
}
}
and get the desired SDL:
type Query {
example(foo: String! = "bar"): String!
}
When I operate against this from a client, I can send an arbitrary string value for foo, or I can omit the argument and receive the default value "bar". If I send null as the value for foo, I get an error, just as the schema says I should.
There is no default in the TS code, so there is no way you are getting = "bar" in the DSL.
Nonetheless, I've gone and tested this myself.
With:
@Query(() => String)
async helloWorld (@Args('input', { nullable: false, defaultValue: 'HelloWorld Default!' }) input: string): Promise<string> {
return `${input}` || 'Input was empty!!!'
}
I get the DSL
type Query {
helloWorld(input: String! = "HelloWorld Default!"): String!
}
And if I try to send the query without input (omitting it), I get the error:
{
"errors": [
{
"message": "Syntax Error: Expected Name, found \")\".",
"locations": [
{
"line": 2,
"column": 15
}
]
}
]
}
If I try to send null, I get:
{
"errors": [
{
"message": "Expected value of type \"String!\", found null.",
"locations": [
{
"line": 2,
"column": 22
}
]
}
]
}
If I send any other value, I get that value passed to the resolver. In other words, default is ignored,when the field is non-nullable.
Now, if I change it to be nullable:
@Query(() => String)
async helloWorld (@Args('input', { nullable: true, defaultValue: 'HelloWorld Default!' }) input: string): Promise<string> {
return `${input}` || 'Input was empty!!!'
}
I still get the same error with input missing. And I get "null", when I send null.
So, default values don't even work with args.
Scott
There is no default in the TS code, so there is no way you are getting
= "bar"in the DSL.
It doesn't matter what default is set in the code at runtime.
And if I try to send the query without
input(omitting it), I get the error:{ "errors": [ { "message": "Syntax Error: Expected Name, found \")\".", "locations": [ { "line": 2, "column": 15 } ] } ] }
This indicates your client query wasn't well formed. GraphQL doesn't allow empty argument lists like () so if you're omitting all of the arguments to the field, make sure you remove the brackets as well.
Otherwise it looks like your example is working?
GraphQL doesn't allow empty argument lists like () so if you're omitting all of the arguments to the field, make sure you remove the brackets as well.
Oh. :facepalm:
So yes. The default is taken, if no field is given. I stand corrected.
Ok to reiterate.
-
With a non-nullable field, with a default, the field is optional (despite it showing as required). You'll get the default value, if the field is omitted. If you send null, you get the "Expected value of type String!, found null." error.
-
With a non-nullable field, without a default, the field is required. It will fail with a "field is required" error, if the field is omitted. If you send null, you get the same error as with 1.
-
With a nullable field, with a default, the field is optional. You'll get the default, if the field is omitted. You'll get null, if null is sent.
-
With a nullable field, without a default, the field is optional. If you send a null, you'll get a null. If you omit the field, you'll get
undefined.
I still say adding a default value is business logic. :grin:
Scott
And this issue can still be closed.