[Feature Request] Ignore null arguments

[thanhbinh84]

Steps to reproduce:

1. Having a script to update profile such as:

mutation UpdateProfile ($birthday: String, $deviceToken: String)  {
    updateProfile(
        birthday: $birthday,
        deviceToken: $deviceToken
    )
}

2. Passing deviceToken value only

Expected
Generate graphql without birthday:

mutation {
    updateProfile(
        deviceToken: "abc"
    )
}

Actual
Generate graphql with null birthday:

mutation {
    updateProfile(
        deviceToken: "abc",
        birthday: null 
    )
}

Specs
Artemis version: ^6.0.4-beta.1

build.yaml:

targets:
  $default:
    sources:
      - lib/**
      - graphql/**
      - my.schema.graphql
    builders:
      artemis:
        options:
          fragments_glob: graphql/fragments/common.graphql
          schema_mapping:
            - schema: my.schema.graphql
              queries_glob: graphql/*.graphql
              output: lib/libs/graphql/graphql_api.dart
              naming_scheme: pathedWithFields

Artemis output:

# Please paste the output of
$ flutter pub run build_runner build --verbose
#or
$ pub run build_runner build --verbose

[INFO] Generating build script...
[INFO] Generating build script completed, took 415ms

[INFO] Creating build script snapshot......
[INFO] Creating build script snapshot... completed, took 12.1s

[WARNING] The package `adix` does not include some required sources in any of its targets (see their build.yaml file).
The missing sources are:
  - $package$
[INFO] Initializing inputs
[INFO] Building new asset graph...
[INFO] Building new asset graph completed, took 677ms

[INFO] Checking for unexpected pre-existing outputs....
[INFO] Deleting 3 declared outputs which already existed on disk.
[INFO] Checking for unexpected pre-existing outputs. completed, took 7ms

[INFO] Running build...
[INFO] Generating SDK summary...
[INFO] 4.3s elapsed, 1/17 actions completed.
[INFO] Generating SDK summary completed, took 3.7s

[INFO] 5.4s elapsed, 2/17 actions completed.
[INFO] 6.5s elapsed, 5/21 actions completed.
[INFO] 7.5s elapsed, 6/22 actions completed.
[INFO] 8.6s elapsed, 6/22 actions completed.
[INFO] 9.6s elapsed, 6/22 actions completed.
[INFO] 10.7s elapsed, 6/22 actions completed.
[INFO] 11.8s elapsed, 6/22 actions completed.
[INFO] 14.3s elapsed, 6/22 actions completed.
[INFO] 15.6s elapsed, 6/22 actions completed.
[INFO] 19.4s elapsed, 7/22 actions completed.
[INFO] 20.6s elapsed, 14/28 actions completed.
[INFO] 21.7s elapsed, 50/66 actions completed.
[INFO] 22.7s elapsed, 167/167 actions completed.
[INFO] Running build completed, took 22.8s

[INFO] Caching finalized dependency graph...
[INFO] Caching finalized dependency graph completed, took 81ms

[INFO] Succeeded after 22.8s with 4 outputs (172 actions)

GraphQL schema:

# If possible, please paste your GraphQL schema file,
# or a minimum reproducible schema of the bug.

GraphQL query:

# If possible, please paste your GraphQL query file,
# or a minimum reproducible query of the bug.

[vasilich6107]

Hi. Could you add a little bit reasoning behind this feature?

[thanhbinh84]

Hi. Could you add a little bit reasoning behind this feature?

There are two reasons:

1. In case profile has an attribute is in enum type, graphql will reject null value.
2. If we want to update only one field such as firstname only, it possibly empty other fields with null value

[vasilich6107]

Thanks.
I’ll check this with graphql faker.

[Grohden]

@thanhbinh84 @vasilich6107
about ignoring nulls, I've been using a flag for json serializable on build.yaml:

targets:
  $default:
    builders:
      # Some of our queries fail when we send null keys
      # (foo)
      json_serializable:
        options:
          include_if_null: false

Maybe this solves for you?

The downside is that it applies to your whole project, so if your backend relies on specifically receiving null vs undefined to create specific behavior, that will cause problems for you (oh well, only node will probably have undefined AND null to represent key absences...)

[comigor]

Hello there, how are you doing?
So, GraphQL spec accepts both null and the absence of the key as the same: https://spec.graphql.org/draft/#sel-GAFdRHABABsC0pB
And, as json_serializable have this option (as pointed by @Grohden), maybe we should avoid having this option on artemis?

[thanhbinh84]

Thanks for your info. @Grohden's solution works, we don't need to update our lib then.

Cheers.

[GP4cK]

@comigor @thanhbinh84 I don't think that solves the issue completely.
For example: let's say I have a user like this:

input User {
  id: String! 
  firstName: String
  lastName: String
  companyID: String
}

Now I want to update the firstName and remove them from a company, I would send a mutation like:

mutation updateUser {
  id: "myUserID"
  firstName: "Alice"
  # lastName is not included because I don't want to update it
  companyID: null
}

How can I do that?

[thanhbinh84]

@GP4cK I haven't tried but passing empty string "" might help, as json serializable will bypass null but keep empty string.

[GP4cK]

@GP4cK I haven't tried but passing empty string "" might help, as json serializable will bypass null but keep empty string.

Ok so then it will be the responsibility of the server to convert the empty string to null if it's a foreign key. That's a workaround.

[Grohden]

@GP4cK Just an idea: you could create a scalar (something like DbNull) use it with a union and deal with it differently on the server side
🤔
Not sure if artemis handles union types

Also, there's... a trick you could use

input User {
  id: String! 
  firstName: String
  lastName: String
  companyID: [String]
}

A list ([String]) would in some ways satisfy a Optional/Maybe idea:
if the list itself is null, then the value is not present (meaning undefined)
if the list is present but empty, then it means that the optional is empty (meaning null)
if the list is present and not empty, then it means it holds a value (meaning T)

Of course, this is a hack (which I may remember from list and maybe being monads (?)🤔) and probably will end up introducing more problems..

Anyway, I think that the only way to actually solve this is to have artemis processing a gql directive (maybe configurable on build.yaml) and then generating the json_serializable class with the include_if_null flag based on that directive

[comigor]

I understand your use cases and, although not spec-compliant, it seems better to have some configuration than sourcing to these kind of "hacks". I'll think about something.

[GP4cK]

Maybe we could use something like the Value<T> class in moor(?)

Extract from their doc:

If a field was set to null, we wouldn’t know whether we need to set that column back to null in the database or if we should just leave it unchanged. Fields in the companions have a special Value.absent() state which makes this explicit.

The source code is here

[GP4cK]

@comigor do you have any update on this? I'd be happy to help develop this if you tell him how you want it to be implemented.
Also, is there any plan to release a stable version 6?
Thanks.

[comigor]

@GP4cK I didn't have time to look at this yet. I understand your use case, but as it's not spec-compliant, it makes things harder.

It seems that to solve your problem, we'd have to remove all nulls but a few selected fields when converting to json, right? In this case, we'd have to mark all fields with includeIfNull: false, and allow to configure which fields to mark as true.

Still a hacky solution, but without need to change the backend would be to create a new option on artemis, named something like allowNullOnFields, implemented as a map/list of whitelisted fields to mark with includeWithNull = true, something like:

targets:
  "$default":
    builders:
      artemis:
        options:
          allow_null_on_fields:
          - User:
            - companyID

When this configuration is not null, like I said, all fields would be marked with JsonKey(includeWithNull: false), except those whitelisted.

[jjoelson]

So, GraphQL spec accepts both null and the absence of the key as the same: https://spec.graphql.org/draft/#sel-GAFdRHABABsC0pB

I understand your use case, but as it's not spec-compliant, it makes things harder.

Maybe I'm misunderstanding, but it seems to be saying that it's totally valid for the server to interpret these two differently and cites the exact use case discussed in this thread (emphasis mine):

GraphQL has two semantically different ways to represent the lack of a value

The first has explicitly provided null to the argument “arg”, while the second has implicitly not provided a value to the argument “arg”. These two forms may be interpreted differently. For example, a mutation representing deleting a field vs not altering a field, respectively. Neither form may be used for an input expecting a Non-Null type.

Is it really out of spec for the server to treat omitted arguments and null arguments differently?

[Luke265]

How about wrapping nullable field types with a simple class? Take apollo-android for an example:
https://github.com/apollographql/apollo-android/blob/5d1a72c0d303b172051d31bb992804d52d865e7b/apollo-api/src/commonMain/kotlin/com/apollographql/apollo/api/Input.kt#L44

If a field is nullable, it's generated type will would be Input<T>, and by default it's value would be Input.absent(). If you want to set a value to that field you must use either Input.optional or Input.fromNullable.

[cody1024d]

@comigor Have you given this any more thought? This is also a game-changer for me. We use "implicit" nulls (i.e. properties not included) to tell our back-end to NOT update that field, where "explicit" nulls clear the field (similar to @GP4cK above).

[vasilich6107]

@cody1024d i tried to implement this feature - see the #339
But I had no time to finish it)

[cody1024d]

@vasilich6107 I'm not sure ignoring all nulls would be the best policy. I think something like the suggestion by @Luke265 would be best. Somehow generating a small wrapper class around the optional values in the schema, which then play a part in the serialization. Going to ask the rest of my team, but I may spend some time working on this proposed solution as it's going to be a blocker for us either way

[vasilich6107]

@cody1024d if you check the PR there is an optional value so it will remove only non setted values

[henriqueArrazao]

Some news about the explicit null?
I think the best solution and easiest way to solve this issue would be create an "ExplicityNull()" object. Example:

      final notDonationsQuery = ProductForCatalogQuery(
        variables: ProductForCatalogArguments(
            where: ProductWhereInput(
              price: FloatNullableFilter(not: *******ExplicityNull()*******),
            ),
            take: 15,
            orderBy: [ProductOrderByWithRelationInput(id: SortOrder.asc)]),
      );

Another question, do you know if "ferry" package for flutter has this problem too? Very thanks.

[CezarCrintea]

I solved a similar issue - filtering nullable fields from some specific entities - by extending the RequestSerializer. I use as backend HotChocolate v12, and it wants no null fields in FilterInput & SortInput entities

[henriqueArrazao]

I think this should be changed from "enhancement" to "bug"

[syssam]

any update?

[vasilich6107]

@syssam no time to finish)
#339