graphql-hive / graphql-hive/envelop

[useResponseCache] cached Async-Iterable result breaks clients

Open
#2,236 2 comments 1 reaction 1 assignee Claimed by @EmrysMyrddin View on GitHub
kind/bug stage/3-local-solution
Dominant language
No language data
Stars
827
Forks
132
PR merge metrics
No merged PRs in 30d

Description

### Issue workflow progress

_Progress of the issue based on the
[Contributor Workflow](https://github.com/the-guild-org/Stack/blob/master/CONTRIBUTING.md#a-typical-contributor-workflow)_

- [ ] 1. The issue provides a
[minimal reproduction](https://en.wikipedia.org/wiki/Minimal_reproducible_example) available on
[Stackblitz](https://stackblitz.com/fork/node).
- _Please install the latest `@envelop/*` packages that you are using._
- _Please make sure the reproduction is as small as possible._
- [ ] 2. A failing test has been provided
- [x] 3. A local solution has been provided
- [ ] 4. A pull request is pending review

---

***Intro***
- Reproduction cannot be produced for this issue. Or it would require building an entire application server to client.
- The issue surfaced using Relay as GraphQL client

**Description**
Caching Async Iterable is extremely useful, but GraphQL clients expect that queries using `@defer` and `@stream` are executed in streaming mode.
Instead, the implementation of this plugin delivers the whole cached result within a single response.

This entirely breaks Relay because it doesn't look within the initial response for any fragment that is marked as `@defer` but instead continue waiting for more parts to be delivered with the deferred fragments.

Additional information on Relay behaviour are available in [this GitHub issue](https://github.com/facebook/relay/issues/3904).

**Workaround**
A workaround is to add an extension to the response to let clients know that the operation is complete.
In case of Relay this must be `extensions: { is_final: true }`. Although the casing looks weird, so it might be possible that other clients expect `isFinal` instead.

This workaround can be implemented just by adding the following line of code before [line 615](https://github.com/n1ru4l/envelop/blob/main/packages/plugins/response-cache/src/plugin.ts#L615):
```js
result.extensions = { ...extensions, is_final: true };
```

With this extension Relay does process all the fragments available in the response even if this is not delivered in streaming mode.
I hope you will consider accepting this fix quickly since this is my last blocker for migrating from Helix to Yoga.

**Proper solution**
Even with the workaround above, Relay still issues the following warning
```
Warning: RelayModernEnvironment: Operation `AccountEventsNewPage_Query` contains @defer/@stream directives but was executed in non-streaming mode. See https://fburl.com/relay-incremental-delivery-non-streaming-warning.
```

This tells me that GraphQL clients expect a stream mode.
With this in mind, the only proper way to solve this problem is to cache the response in parts. Then deliver the first part straight away and yield results for the following parts sequentially.

This will take longer to implement, so the workaround is still valid to avoid breaking clients.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.