eager hasNext: false in @defer payloads

[martinbonnin]

Using router at 6b83a74 and this @defer query:

query deferVariation {
  allProducts {
    ...MyFragment @defer
    sku
    id
    __typename
  }
}

fragment MyFragment on Product { 
  __typename 
  variation {
    name 
    __typename 
  }
}

I'm getting these payloads:

--graphql
content-type: application/json

{"data":{"allProducts":[{"sku":"federation","id":"apollo-federation","__typename":"Product"},{"sku":"studio","id":"apollo-studio","__typename":"Product"}]},"hasNext":true}
--graphql
content-type: application/json

{"hasNext":true,"incremental":[{"data":{"variation":{"name":"platform","__typename":"ProductVariation"},"__typename":"Product"},"path":["allProducts",0]},{"data":{"variation":{"name":"platform-name","__typename":"ProductVariation"},"__typename":"Product"},"path":["allProducts",1]}]}
--graphql
content-type: application/json

{"hasNext":false}
--graphql--

I believe this is compliant but it's still introducing latency because the client doesn't close the flow until the hasNext: false is received. It would be nice to have the hasNext: false eagerly:

{"hasNext":false,"incremental":[{"data":{"variation":{"name":"platform","__typename":"ProductVariation"},"__typename":"Product"},"path":["allProducts",0]},{"data":{"variation":{"name":"platform-name","__typename":"ProductVariation"},"__typename":"Product"},"path":["allProducts",1]}]}

[abernix]

Thanks for opening this. There might be times when the router has to do this. I'm not sure if this is one of them.

[martinbonnin]

For some context, on the Kotlin Client side this is causing an extra emission so the user sees 3 updates at the moment (with the same values for 2 and 3). I'm trying to figure out if this is something that should be filtered client side or not (this might not be trivial to do as we have code that relies on hasNext to avoid race conditions when subscribing to the store after a query, see also apollographql/apollo-kotlin#4319 (comment)).

[jpvajda]

@martinbonnin 👋 thanks for pointing this out! We discussed this particular problem in our Web Client planning session today and @alessbell did some work on the client side to handle this case, so she was going to share some thoughts later.

[martinbonnin]

For some more Context, Apollo Kotlin merges all payloads so that what's emitted to the user is always the same (strongly typed) class.

For and example, with a query like below:

{
  fast
  ...deferedFragment @defer
}

fragment derferedFragment on Query {
  slow
}

the user is going to see something like this (details omitted):

payload 1:

Query.Data(
  fast = 42,
  deferedFragment = null
)

payload 2:

Query.Data(
  fast = 42,
  deferedFragment = DeferredFragment(
    slow = 0
  )
)

payload 3 (identical to payload 2):

Query.Data(
  fast = 42,
  deferedFragment = DeferredFragment(
    slow = 0
  )
)

The user can debounce this with .distinct() or we could do some filtering on our side but this ties into a silly .watch() race condition (see apollographql/apollo-kotlin#4319 (comment)) where having the information that an item is the last eagerly is simplifying things.

All in all, having the hasNext eager would be nice to have because:

1. it makes our Apollo Kotlin dev life easier
2. it might save a tiny little bit of power (the network connection is closed earlier)

We might have to handle it no matter what so I opened this issue to get some feedback about it but if the answer is that it's not possible, we'll find ways to support it.

[jpvajda]

@martinbonnin to confirm: if we couldn't resolve this issue, and released with this as is, would we need to bring into scope this issue for the Kotlin client to resolve before the release so a user could use .watch() ?

I agree, that hasNext eager would be ideal. I also wanted to confirm when you say "eager" is this the concept you have in mind?

While lazy loading delays the initialization of a resource, eager loading initializes or loads a resource as soon as the code is executed. Eager loading also involves pre-loading related entities referenced by a resource. For example, a PHP script with an include statement performs eager loading—as soon as it executes, eager loading pulls in and loads the included resources. Eager loading is beneficial when there is an opportunity or need to load resources in the background. For example, some websites display a “loading” screen and eagerly load all the resources required for the web application to run.

[martinbonnin]

if we couldn't resolve this issue, and released with this as is, would we need to bring into scope apollographql/apollo-kotlin#4319 (comment) for the Kotlin client to resolve before the release so a user could use .watch() ?

I think it's fine to release as-is and let users debounce for now. Functionality is still there. It's just sub-optimal. Users can still call .watch(), they just get an extra event in the initial Flow. Also I'm not sure how much of an use case it is to combine @defer + .watch(). My recommendation would be to ship as-is and see how the community uses the feature. This is why we're doing experimental releases after all.

I also wanted to confirm when you say "eager" is this the concept you have in mind?

Mostly yes. What I mean by "eager" is send the hasNext: false alongside the last incremental data

[jpvajda]

@martinbonnin sounds good, I agree it shouldn't block our release. To your point about:

I think it's fine to release as-is and let users debounce for now.

Can we ensure we update the documentation you've provided, it might be nice to be more explicit about this case.

See apollographql/apollo-kotlin#4373

[bnjjj]

FYI I have a wip branch with a fix that seems good enough. I think I will be able to finalize it this week.

[Geal]

a heads up on this: the fix in #1736 is not enough, because with parallel deferred responses (like query { a { b { ...@defer { c } } d { ...@defer { e } } } }), we might get a hasNext: false response followed by a hasNext: true one.

I am looking for a different solution in #1745 but it is not working right now, and it is still a bit of a hack. There is no way to correctly predict how many deferred responses there will be from inside query plan execution, except by synchronization, which could be disastrous for performance, so if we do not find a solution, the last empty response will come back as that's the only correct one for now.

@martinbonnin is there a way to change the kotlin client to be more flexible here? There is no guarantee that all defer implementations will do what it expects