fix(gateway): pass null required fields correctly to resolvers

[jakeblaxon]

This fixes a bug in the Apollo Gateway, where certain subfields that were null in the parent object that is passed to resolvers were getting expanded into objects that contained all null subfields.

Scenario

When service B requires a field from service A, if that field is null in service A, it would be passed to B's resolver expanded as an object with all null subfields.

For example, if B required the field movie from service A in its resolver, and the value in service A looks like the following:

"actor": {
    "movie": null
}

the actor object, being the parent, would be passed to B's resolver as the following:

"actor": {
    "movie": {
        "title": null,
        "year": null,
        ...
    }
}

This can cause unexpected errors in the resolver logic, since the resolver may assume that the title and year fields will never be null (as per the graphQL schema).

This fix prevents this expansion, so that the actor object passed toB's resolver remains the same, i.e. the actor.movie field is null.

[jakeblaxon]

@trevor-scheer this was another bug I had noticed when working with nested @requires fields. I believe I pinpointed the source of the error, but I'd like your review to make sure that this fix is safe. I'd prefer to get this fix in with the same release as #4064, if possible. Please let me know if you need anything from me. Thanks!

[jakeblaxon]

@trevor-scheer can you take a look at this when you get a chance? This fixes an issue where the resolver parent type can sometimes break the contract as specified by the schema (e.g. certain non-nullable fields appear as null in the parent type passed to the resolver). This can cause some nasty bugs in production since it breaks type safety.

[trevor-scheer]

Hey @jakeblaxon, thank you for patience (and effort!) on this. This looks good to me. If you don't mind pushing up a few comments that explain the conditions that we're handling differently that would be much appreciated. Explaining the edges will be helpful for us in the future 🙂

[jakeblaxon]

@trevor-scheer sounds good! I actually just found what I think is a more simple fix for this. I left a comment above to explain the check as well. Can you take another look and let me know what you think?

[trevor-scheer]

@jakeblaxon in digging in a bit more deeply to understand this change, I found that your test passes even if I revert the change. Are you aware of that? This is not to say the change is necessarily wrong, but if it's correct then I think the test may be missing the point. Maybe we need to reconsider the testing path? Let me know your thoughts!

[jakeblaxon]

@trevor-scheer That's interesting. The test fails for me, even with the latest pull from main. Did you make sure to run npm run compile beforehand? I know that's tripped me up before.

[trevor-scheer]

Thanks @jakeblaxon, my mistake on that and all is well! This is good to go. Thanks again 🥳