Handling stripped nulls from response

[leoasis]

At Twitter, our GraphQL API returns responses with null values stripped, in order to save bandwidth.

From our experience, this leads to quite some noticeable savings, and in some of our biggest responses (13-25k) we see a 5% reduction of the response after compression. Regarding uncompressed size, this goes down as much as ~25%.

While not technically spec compliant, this seems to be a nice performance optimization some servers could do, and the full response with null values should be able to be reconstructed client side, since Relay has the AST for the query that was used to make the request with.

Relay used to kind of support this at the normalizer level, but it was removed in this commit. Then it continued to work but causing a bunch of warnings of the type: RelayResponseNormalizer(): Payload did not contain a value for field <SOME_FIELD>. Check that you are parsing with the same query that was used to fetch the payload., and then in version 8.0.0 it stopped working completely with this commit, where a fragment with missing data would suspend.

I wonder if this is something that can be considered to be brought into Relay, if there's enough interest.

If this is not something that may be worth adding as a feature, at the very least it would be good to have some kind of way to know at the network layer the structure of the operation that was requested, as a way to reconstruct the missing fields from the response before Relay receives it.

[josephsavona]

Thanks for posting and including detailed estimate of the size savings - those figures are definitely non-trivial so we can see how it could be beneficial to enable for your app. There wasn't a strong motivation for us to remove the feature - we primarily did so because it wasn't spec-compliant and we weren't using it. We'll discuss a bit internally and I'll follow-up here!

In the meantime - was anyone else using this feature? Were folks even aware that the option existed?

[sibelius]

As a workaround for now you could disable the feature flag to avoid suspending your fragments

import RelayFeatureFlags from 'relay-runtime/lib/util/RelayFeatureFlags'
RelayFeatureFlags.ENABLE_RELAY_CONTAINERS_SUSPENSE = false

related to #3006

[josephsavona]

We discussed internally and don't see any blockers to adding this back. The main points of discussion:

• Q. What about optimistic updates? A. This feature should be disabled for optimistic updates and developers should have to provide keys for all fields (though they could still choose to provide an explicit undefined or null).
• Q. Does this have any complications with suspense, especially @defer? A. Tl;dr No. If we did traverse into deferred fragments before we were sure the data was available this could be problematic when used in combination w suspense - we might think that a fragment was not pending because had populated null for its fields. But we intentionally avoid traversing into deferred fragments until we know the data (should) be available, so this isn't a problem.

So: we're open to PRs to re-enable this feature and can also add it to our internal backlog. If anyone is interested in working on this (@leoasis yourself included!) please let us know so that we don't also work on the same thing ourselves!

[leoasis]

This is great to know! thanks for following up internally and reaching to a decision! I'll see if I can spend some time looking at this.

If I can find time to work on this, I'll let you know!

[leoasis]

I think I can find some time to tackle this now! Will work on this in the next days