RestLink causes network calls to be executed much later

[kyeb-stripe]

Hi! We've run into a performance issue with apollo-link-rest.

Recently, we started hoisting our queries to the top of our pages so that the network fetch gets kicked off before the initial render. If the fetch can be called before ~100ms of React rendering, the page will be finished loading ~100ms faster.

Unfortunately, apollo-link-rest seems to have an issue where this is impossible - even after hoisting queries to the top level, the fetch still doesn't get kicked off until after all of the initial rendering. It seems to be because the network call is somehow being made asynchronously, and the browser finishes executing the render before coming back and actually firing the network call.

Simple repro:

1. Open your browser's developer tools
2. Toggle the query
3. Observe that on hitting the debugger statement for the first time (immediately after the useQuery), the network request has not yet been kicked off

We've narrowed down the cause of the async-ness to #138. If you change the version of apollo-link-rest to v0.4.4 in the above CodeSandbox, then you can observe that on hitting the debugger statement for the first time, the network request has already been kicked off (yay)!

Internally, we've had to fork apollo-link-rest and revert #138 (we don't need support for mixed queries, and need the performance). Happy to discuss further if I can help debug at all!

[paulpdaniels]

You're right there is an async boundary that gets inserted though I'm surprised it has that big an effect. I suppose one fix could be to fully separate the logic and return the Observable directly when there are no non-rest fields. Ultimately I'm not sure if we could fix the performance of the flatMap since that comes from zen-observable here

[kyeb-stripe]

The effect on its own wouldn't be huge, but when you have several hundred milliseconds of React rendering also queued (performance issues that we're working on separately, ha) it can end up with a heavier impact.

I dug into zen-observable a bit to see if there was a simple solution, but I agree that the it would make more sense to fix within RestLink. Would it be pretty straightforward to add a fast path when there are no non-rest fields? Could you do something as simple as

if (!nonRest) {
  return new Observable(...);
} else {
  return obs.flatMap(({data, errors}) => new Observable(...))
}

If so, I'd be happy to put up a PR to try to fix, but you definitely have more context on how the mixed support actually works.

[fbartho]

@kyeb-stripe oh wow! I’m finding myself surprised too! Is this something you feel motivated to contribute back to this library?

Would you mind further elaborating on your use case too? The goal of this library is as a transitional tool between a REST world and true GraphQL, and there’s certain fundamental performance weaknesses that will never be resolved. For example, you can’t necessarily coalesce these API calls like you can with GraphQL. Selecting subfields still incurs the bandwidth/memory cost that you wouldn’t have with true GraphQL. Among other issues, it’s possible this will not be compatible with Suspense until ApolloClient core is, (if ever). Forgive me if I make incorrect assumptions, but surely Stripe has backend GraphQL on their roadmap?

[kyeb-stripe]

As far as I know, we're sticking to REST for the foreseeable future! For the Stripe Dashboard at least, we have a few tricks up our sleeve to reduce the memory/bandwidth cost 🙂 (feel free to reach out to me elsewhere if you want to learn more!)

I have a bit too much on my plate at the moment to fix this, but will let you know if I get some spare time to come up with a generalized fix. It's not within our use case, but ideally a fix would also remove the async-ness even with mixed queries, which the simple fix I suggested above would not do.