Type-graphql: @Field({ defaultValue: 0 }) marks field as nullable

[andreialecu]

Describe the bug

So I'm not sure if this is a bug or I'm simply doing something wrong, but it seems confusing.

I found defaultValue through intellisense on @Field and I'm using it on @ObjectType().

Setting a defaultValue on a field makes the field nullable.

I couldn't find any documentation regarding defaultValue other than for @Args().

To Reproduce

@ObjectType()
export class Test1 {
  @Field({ defaultValue: 0 })
  public testField: number;
}

Generates the following schema:

type Test1 {
  testField: Float
}

Additionally, trying @Field({ defaultValue: 0, nullable: false }), results in the following error:

(node:27764) UnhandledPromiseRejectionWarning: Error: Wrong nullable option set for testField. You cannot combine default value '0' with nullable 'false'.
    at Object.wrapWithTypeOptions (\node_modules\type-graphql\dist\helpers\types.js:37:15)

Expected behavior

The schema should have the field marked as required:

type Test1 {
  testField: Float!
}

Enviorment (please complete the following information):

• OS: Windows
• Node v12.8.0
• Package version 0.17.5
• TypeScript version 3.7.3

[MichalLytek]

I found defaultValue through intellisense on @Field and I'm using it on @ObjectType().

defaultValue is only for input types, as you can read in the GraphQL Spec. There's no testField: Float = 0 syntax for object type fields, only for input types. You should use public testField: number = 0 syntax for runtime default value.

There's no technical possibility to detect if @Field is placed on @ObjectType() or @InputType(), that's why you have it available through intellisense 😞

[andreialecu]

Cool. A warning about to this in the documentation would help.

[andreialecu]

This creates a problem when sharing the same class for both @InputType and @ObjectType. I'd like properties to have default values for the inputs, but be marked as required in the schema for when they are returned from resolvers.

How would that work without duplicating classes right now? The documentation specifies @InputType() and @ObjectType() can go both on the same class for purpose of code reuse.

If defaultValue isn't supposed to be used for obect type fields, isn't it a bug that they are marked nullable as a side effect of having it set?

[andreialecu]

It seems that setting a default value simply through public testField: number = 0 does set the input default value correctly in the schema, and also leaves the object field required.

@ObjectType()
@InputType('TestInput')
export class Test {
  @Field()
  public testField: number = 0;
}

type Test {
  testField: Float!
}

input TestInput {
  testField: Float = 0
}

In this case, what's the purpose of defaultValue?

I still believe it is a bug that assining a value to it on an object field makes the field nullable.

[MichalLytek]

In this case, what's the purpose of defaultValue?

For many cases, I can't detect an initializer value, so you have to use a decorator option, like @Arg("servings" { defaultValue: 2 }) servings: number. So this is for API consistency purposes.

This creates a problem when sharing the same class for both @InputType and @ObjectType.

Sharing body between input and output type is always a problem as you the behave differently.

I still believe it is a bug that assigning a value to it on an object field makes the field nullable.

I think that this line is resposible for this issue:

if (!typeOptions.nullable && typeOptions.defaultValue === undefined) {
  gqlType = new GraphQLNonNull(gqlType);
}

The current pipeline doesn't allow to easily distinguish between input and output types on generating nullable type, so for now use the property initializer approach as a workaround and I will fix it in vNext 😉

[andreialecu]

I assumed extracting the initializer value wouldn't work for @Arg().

But since this is a different decorator, maybe defaultValue could be removed altogether from @Field() in vNext, and remove a lot of the confusion regarding it.

[MichalLytek]

Removing this won't allow you to "reuse" the same class as output type and in other parts of the app, as you may don't want to have ? in the TS type signature in other parts of the app and have a default value only for GraphQL input.

[andreialecu]

I just noticed a different issue now with this.

  @Field()
  public testField: number = 4;

When returning an object from a resolver using the above signature, if the field is undefined, it won't be initialized to the value of 4 and it will pass through undefined, resulting in Cannot return null for non-nullable field XXX.testField.

Using @Field({ defaultValue: 4 }) instead correctly returns the value of 4 even if the returned object doesn't have that field defined. However, it results in the nullable issue with the field in the schema.

So there's definitely some inconsistency here, if field initializers are supposed to be supported.

Is this undocumented behavior that I shouldn't rely on? defaultValue fills in unitialized values for object type fields, but the field initializer does not.

[MichalLytek]

Default values for object types are not supported and not tested, so I can't help you with that right now as it's a kinda undocumented feature. I have no idea how defaultValue can work for returned object type as I can't event put that property in ObjectType config of graphql-js.

If you want, you can create a failing test cases for that, then apply a fix and then create a PR for that.

[kdong007]

@andreialecu FieldResolver is what you want to go for

[acro5piano]

@andreialecu @kdong007

I ended up with creating a middleware to inject the default value.

import { UseMiddleware, MiddlewareFn } from 'type-graphql'

function DefaultValue<T>(defaultValue: T): MiddlewareFn {
  return async (_, next) => {
    const original = await next()
    if (original === undefined || original === null) {
      return defaultValue
    }
    return original
  }
}

@ObjectType()
export class Test1 {
  @UseMiddleware(DefaultValue(0))
  public testField: number;
}

I also tried Custom decorators ( createMethodDecorator ) but Unable to resolve signature of property decorator when called as an expression was raised and could not fix it.

[backbone87]

On the behavior regarding argument lists and input types: https://github.com/graphql/graphql-spec/issues/570

Lets a asume a string argument or input field:

• nullable: true (type-graphql behaves correctly)
  
  
  
  • omitted: resolver gets undefined (this is a "legacy" behavior of graphql-js)
  
  
  • passed null: resolver gets null
  
  
  • passed a string: resolver gets the string
  
  
• nullable: false (type-graphql behaves correctly) (this is the only case where a value must be passed when querying)
  
  
  
  • omitted: error
  
  
  • passed null: error
  
  
  • passed a string: resolver gets the string
  
  
• nullable: true, defaultValue: null (type-graphql behaves correctly)
  
  
  
  • omitted: resolver gets null
  
  
  • passed null: resolver gets null
  
  
  • passed a string: resolver gets the string
  
  
• nullable: true, defaultValue: 'foo' (type-graphql behaves correctly)
  
  
  
  • omitted: resolver gets string "foo"
  
  
  • passed null: resolver gets null
  
  
  • passed a string: resolver gets the string
  
  
• nullable: false, defaultValue: null: invalid definition (type-graphql behaves correctly)
• nullable: false, defaultValue: 'foo' (type-graphql behaves incorrectly: if nullable: false is set explicitly, then type-graphql incorrectly rejects the definition as invalid. if nullable: false is set implicitly through global setting, then type-graphql silently converts the field to a nullable: true field. both cases are incorrect behavior.)
  
  
  
  • omitted: resolver gets string "foo"
  
  
  • passed null: error
  
  
  • passed a string: resolver gets the string
  
  

since this issue also talks about output types: should i create a new issue to track the nullable behavior for input types/arguments?

[MichalLytek]

Closing as should be fixed by #806 🔒
In new release nullable won't be coupled with defaultValue.