required fields with default value being generated to a nullable field

[subzero911]

Expected conversion:
required Enum_ColorMode? colorMode,

Actual conversion:
Enum_ColorMode? colorMode,

! in graphql means "required", not "non-nullable". It can be both required and nullable.

I expect that if I have a required field with default value, than I MUST put some value on Dart side, but I CAN put just "null", and a server will substitute it with default value in this case.

related to #293

[github-actions]

👋 @subzero911
Thank you for raising an issue. I will investigate the issue and get back to you as soon as possible.
Please make sure you have provided enough context.

This library is created and maintained by me, @budde377. Please consider supporting my work and ensure our survival by donating here.

[budde377]

Hi @subzero911

Please provide some more context to allow me to efficiently help you.

This could be an example GraphQL operation and schema along with generated code and what you expect to be generated.

[subzero911]

My scheme:

generated code:

expected to be generated...

So the compiler will force me to fill the value:

[budde377]

I don't see how this is not solved by #293?

[budde377]

I.e I don't agree that the expected code is in fact expected. If you omit the value or supply null it should not be sent to the server.

[subzero911]

So it basically means that whether I have a ! mark or not, either way it will generate a nullable non-required field.

[budde377]

Yes, but the implementation differs. I.e null values will be not be sent to the server. I believe you confirmed with your backend dev eight months ago that this works by letting the sever fill out the default value?

[subzero911]

You're probably right.
GraphQL specs assume that only optional values can have a default value:

So it's rather an incorrect scheme than a bad code generation.

[subzero911]

Hi again, I checked this twice
There's a nuance between arguments and types:

• in field arguments ! means required:
  

• in fields ! means non-nullable:
  https://graphql.org/graphql-js/basic-types/

So it's legit to write colorMode: ColorMode! = 'light'
In this case, code generation tool should generate:
Enum_ColorMode colorMode = 'light',
instead of Enum_ColorMode? colorMode

I believe you confirmed with your backend dev eight months ago that this works by letting the sever fill out the default value?

It's unsafe to rely on server implementation.
It could lead to a scheme validation failure, if I sent null, but the server was waiting for a value

Example for a Spring server:

input TestInput {
    nonNullable: String!
    nullable: String
    nonNullableDefault: String! = "Hello"
    nullableDefault: String = "World"
}

[budde377]

Thank you for your persistence on this issue and for keeping up a good debate.

According to your first image, the behaviour for the default value when you provide it to a required field is not defined. As such, you can argue that the engineer writing the schema should not add default values to required fields. Of course, this is not a perfect world, and I recognise that considering that schema for invalid when generating the client is not an option.

I am hesitant to generate the default value because it currently isn't a concern for the generator, and making it so carries a few problems:

• Backwards compatibility: Right now the schema can update the default values as it see fit in a backwards compatible way. The server can change the default value, and the client will not need to be rebuilt because they are not generating and sending any default value. This is a very powerful feature.
• Inconsistent default value behaviour: Say that we only generate the default value for the required fields. This might limit the impact of the point above, but it will require an inconsistent handling of default values. It is not clear to the implementor when the client will send a default value and when it will not.
• Complex default value serialisation: Generating the Enum value is easy, but consider complex input types or custom scalars. If we start a best-effort attempt at parsing the default value from the schema, converting it into a Dart type, and setting it, we are opening a can of worms I am not sure I want to handle.

With these reasons in mind, I think there are two appropriate solutions:

• Do not make the field required, and do not send it to the server (current implementation).
• Do make the field required and force the client-developer to provide a value (old implementation).

As you are pointing out, different servers might have different interpretations of the spec, so I think it would be reasonable if the solution was configurable. We can add a flag to the config allowing the user to specify their desired behaviour.