add test to ensure defer/stream payloads are always sent in correct order

[yaacovCR]

currently failing @robrichard @brainkim

[yaacovCR]

Now passing, with some changes in payload order, and in execution algorithm, namely completion does not start for any streamed item until completion for the prior item finishes.

This is suboptimal. What we want is for completion to start for all possible items as soon as possible, but for payload order to still be preserved. Or -- perhaps, we want to relax the requirement in the spec for all items to be sent in sequential order?

[yaacovCR]

Note that the failing check has to do with uploading code-coverage and appears--at least to me--to be orthogonal to the changes here.

[yaacovCR]

see discussion at #2848 (which perhaps should be moved here?)

I have reverted the "fix" for now as I am not sure it is the desirable behavior. Perhaps we should "fix" the tests to reflect the current implementation.

[yaacovCR]

I added additional tests for stream payloads that are asyncIterables, defer payloads, and defer/stream payload interactions, as I understood from recent WG notes that we want to enforce order for both defer/stream payloads by default, and occurred to me that this "bug" may be more far-reaching, include defer fragments, etc.

Personally, I am agnostic to what the desired behavior should be, at this point tending more toward @robrichard that we should just send what we have, when we have it.

Although I am sensitive to @benjie 's point about malicious servers, I think that we can push onto clients to handle that situation less naively.

[benjie]

Although I am sensitive to @benjie 's point about malicious servers, I think that we can push onto clients to handle that situation less naively.

I would like to caution against pushing requirements onto the client without sufficiently evaluating the alternatives, primarily because there's a lot more GraphQL clients than there are GraphQL servers (e.g. there's a bunch that people roll their own using e.g. window.fetch(...)). Demanding that every one of these GraphQL clients factors in subtle intricacies in the defer/stream protocol is a big ask. When it goes wrong, some people aren't going to think "Oh I implemented that wrong" but rather "Why is GraphQL giving me payloads out of order? That seems like a bug..."

[yaacovCR]

I agree there is validity to architecting toward the simplest possible behavior in the hopes of not being blamed then things go “wrong.”

But I also think we should be thinking primarily about use cases. Might it make sense to display data from a deferred User Profile fragment when it’s ready even when the complete User fragment higher up in the tree has not yet come in? I think that would really be the clients choice. Would it make sense to give client data from a later list item as soon as available, “leaking” as well the knowledge that they are X intervening items? I think it would.

I am envisioning basically the overarching goal of incremental delivery being to provide overall speediest delivery.

GraphQL lists always have a defacto order when resolved and so they can be thought of as a map with numeric keys.

Overall agree with argument approach to dictate this, but I feel like the default behavior should be to optimize for fastest delivery, not for easiest client implementation.

But I wouldn’t put my own preference on a pedestal. I will be working on an stitching implementation that will eventually probably have to handle both, whether by default or by argument, so I probably have to just start out not making assumptions about the order…

[yaacovCR]

Closed as fixed here and in graphql-executor