Enable Decoding Responses Omitting Null Values (Set HandleMissingValues to .allowForOptionalFields in GraphQLSelectionSetMapper)

[gscalzo]

Enable Decoding Responses Omitting Null Values (Set HandleMissingValues to .allowForOptionalFields in GraphQLSelectionSetMapper)

[calvincestari]

Hi @gscalzo - I understand your request and I get that this behaviour is making the transition to Apollo iOS more difficult than it needs to be. As stated in the other issue, the fact remains that Apollo iOS is a spec-compliant GraphQL client.

They highlighted that Apollo Kotlin has functioned without this need for years

There will always be nuanced differences between clients and simply because they are from the same organization does not mean every facet of behaviour is the same. Formal specifications exist to minimize this.

@martinbonnin, @BoD - do you have a definitive answer on why the difference in behaviour is allowed?

and pointed out that the referenced specification is still in draft, making adherence somewhat ambiguous.

I linked to the draft spec because it is the most recent and up-to-date version of the specification. Yes it contains new behaviour that is not yet part of an official dated release but null response field values is not one of those. The very first released specification back in July 2015 stated "If result is null or a value similar to null such as undefined or NaN, return null."; this has been defined field response behaviour for 8 years now.

However, I noted in @AnthonyMDev's PR (#2897) the introduction of HandleMissingValues, particularly the allowForOptionalFields option, which would resolve our issue. The challenge is that GraphQLSelectionSetMapper is instantiated with .disallow as the default, and there's no current method to alter this from the client code.

Could this policy be accessible to the caller's code, possibly as a parameter in the ApolloStore class? This approach would maintain the existing and default behaviour while allowing users who need to handle omitted null values the flexibility to set this policy. What do you think about this?

Not allowing missing fields is a piece of the overall design that extends further than just GraphQLSelectionSetMapper. If you were to allow missing fields in the response you're then out of sync with the behaviour of the cache which is also does not expect missing fields; there are other parts of Apollo iOS built on this premise too. A change like this is more complicated to keep everything working together than simply exposing the ability to control HandleMissingValues.

This isn't behaviour that we'll change in Apollo iOS but you are always able to fork the repo, adapt it to your specific needs and maintain it yourself. I also do not believe this is a change we would accept back into the main repo either.

[martinbonnin]

@martinbonnin, @BoD - do you have a definitive answer on why the difference in behaviour is allowed?

AFAIK, this is out of practical reasons mainly. The JSON parsers use the following pseudo code:

var field: Int? = null

while (jsonReader.hasNext()) {
  when (jsonReader.nextName()) {
    "field" -> field = jsonReader.nextInt()
  }
}

return Data(field)

So it saves a bunch of generated code and hasn't been an issue so far but it's definitely not allowed by the spec.

Actually now that I think about it, it breaks a small optimization we have because the order of fields in the JSON is deterministic, nextName() skips a few strcmp because it knows in advance what to expect. If a field is missing, we have to fallback to scanning all possible names.
So not great and should probably fixed on the server indeed. I'm actually surprised DGS allows this. It's probably worth filing a bug there.