Add support for @defer directive by clarencenpy · Pull Request #1287 · apollographql/apollo-server

clarencenpy · 2018-07-02T17:24:02Z

This PR proposes the addition of @defer support to Apollo Server.
Full details of the proposed spec can be found here.

Summary:

This PR uses a modified version execute.js from graphql.js. Functions that are untouched have been removed, and exported from graphql directly.
execute() returns ExecutionResult | DeferredExecutionResult, the latter being if @defer has been applied to any fields.
DeferredExecutionResult wraps an ExecutionResult containing the initial response, and an AsyncIterable<ExecutionPatchResult>.

Testing:

I have extended the star wars test suite to add tests for @defer, while making sure that all the other tests are still passing. Tests are in __tests__/starWarsDefer-test.ts

stubailo · 2018-07-02T23:00:02Z

execute() returns ExecutionResult | DeferredExecutionResult, the latter being if @defer has been applied to any fields.

Hmm, I think this could be a bit confusing, since the calling code now needs to potentially deal with two different types. Could this be just one, or maybe a separate function that supports defer?

stubailo · 2018-07-02T23:00:37Z

Also, do we want to combine the interface with subscriptions in any way, since both are ways to get an initial result and then updates?

stubailo

Some initial feedback

stubailo · 2018-07-02T23:03:05Z

packages/apollo-server-defer/README.md

+```
+// Initial Response
+{
+  "asset": {


This would have a data field right?

packages/apollo-server-defer/README.md

+- Proposed format for `ExecutionPatchResult`
+  ```graphql
+  {
+    path: [string]!


stubailo · 2018-07-02T23:04:14Z

packages/apollo-server-defer/README.md

+
+#### Apollo Server:
+
+In order to support @defer, there are significant changes to be made to the execution phase if GraphQL.


stubailo · 2018-07-02T23:05:03Z

packages/apollo-server-defer/README.md

+
+#### Restrictions on @defer usage:
+
+- _Mutations:_ already execute serially, so there does not seem to be any use case for @defer. We should throw an error if @defer is applied to the root mutation type. However, @defer should apply normally for the selection set on a mutation.


However, @defer should apply normally for the selection set on a mutation.

I think we maybe decided that defer shouldn't be allowed anywhere in a mutation?

Also, I think that should also apply to subscriptions, at least at first to simplify things.

Ah yes! Because there is no analog to watchQuery on the client! Can do!

stubailo · 2018-07-02T23:07:25Z

packages/apollo-server-defer/src/execute.ts

@@ -0,0 +1,1630 @@
+/**


One thing that makes this file harder to review is that it's hard to know what comes from graphql.js and what is new. Can you point me to some of the most important functions to review?

Definitely. Let me strip out all the utility functions and leave only those that changed.

martijnwalraven · 2018-07-03T06:50:18Z

packages/apollo-server-defer/README.md

+
+#### A Better Way
+
+_@defer_ provides a way for us to mark fields in our schema to _not_ wait on, because they might be:


I think we should mention that @defer has been used inside Facebook and link to Lee's talk. Maybe we can say something like: the concept of @defer has generated a lot of interest ever since it was first talked about in 2016, but until now there was no way for people to actually use it.

I've added this under A Better Approach in the README.md. Thanks for pointing out!

martijnwalraven · 2018-07-03T07:04:49Z

packages/apollo-server-defer/src/execute.ts

+ */
+export interface DeferredExecutionResult {
+  initialResult: ExecutionResult;
+  deferredPatchesObservable: Observable<ExecutionPatchResult>;


I'm honestly still confused by the difference between Observable and async iterators, but since async iterators are already part of graphql-js for supporting subscriptions, and async iterators recently reached stage 4 and will be part of ES2018, should we consider using them instead? See http://2ality.com/2016/10/asynchronous-iteration.html.

Observable is still at stage 1 and doesn't seem to be going anywhere: https://github.com/tc39/proposal-observable
Maybe we can ask @benjamn what his thoughts on this are.

Also, I'm a little afraid of taking a dependency on RxJS because we want the bundle size on the server to stay as small as possible for edge execution. And V8 may get more optimized execution for async iterators (no idea if that makes sense or what it would look like).

It turns out async iterators are already available natively in Node 10 and recent versions of Chrome! They require some getting used to, but for-await-of and async generators are pretty powerful. See http://2ality.com/2018/04/async-iter-nodejs.html for examples.

Thanks for pointing this out Martijn! As per James' comments, I think this will require some more thinking about the pros and cons of both approaches. In the meantime, I will look at the subscriptions implementation to get a better understanding.

martijnwalraven · 2018-07-03T07:06:59Z

packages/apollo-server-defer/README.md

+}
+```
+
+Instead of holding the GraphQL response and page rendering until the entire query is resolved, @defer tells Apollo Server to return a partial query response if the deferred fields are not ready yet. In the example above, we see how it may be used to remove `progress` and `comments` from the intial query response. Apollo Server would then take care of resolving the rest of the deferred fields in the background, and stream them to the client when they are ready.


Do we also support nested @defer?

Yes, you should be able to nest @defer to arbitrary depth. Works on lists and object types. Unit tests are here

jbaxleyiii

@clarencenpy this is a great start! I left some comments on specifics but there are two main things I think we need to change:

Why is this another package? I would have expected it to be in apollo-server-core. Lets be really careful about adding a ton of packages here as it increases complexity to manage
I think the results need to include the field that is defered with a null value. Since we are limiting usage to nullable types, this won't impact type generation tools and will require the least amount of work for clients. It also makes debugging easier since you don't have to determine if there was a problem with the sent operation, but rather can see if the initial result has the data here. Additionally, it makes it easier to work with inside UI components

jbaxleyiii · 2018-07-03T10:35:36Z

packages/apollo-server-defer/src/execute.ts

+    if (observer) {
+      observer.next(patch);
+    } else {
+      return new Observable<ExecutionPatchResult>(observer => {


This is a nit, but lets do return Observable.of(path) instead

jbaxleyiii · 2018-07-03T10:42:13Z

packages/apollo-server-defer/src/execute.ts

@@ -0,0 +1,1630 @@
+/**
+ * Copyright (c) 2015-present, Facebook, Inc.


lets reference the original here but adjust the license with giving credit

jbaxleyiii · 2018-07-03T10:42:40Z

packages/apollo-server-defer/src/__tests__/starWarsSchema.ts

+  query: queryType,
+  types: [humanType, droidType],
+  directives: [GraphQLDeferDirective],
+  // TODO: Eventually bake in @defer as a standard directive and do validation


I don't think we should merge this is until that is done (baked in)

Yep! Instead of modifying the validation phase of graphql.js, I think a more minimal approach would be to modify the call to GraphQLSchema's constructor within AS2, to pass in the additional schema directives.

jbaxleyiii · 2018-07-03T10:43:16Z

packages/apollo-server-defer/src/__tests__/starWarsSchema.ts

@@ -0,0 +1,337 @@
+/**
+ * Copyright (c) 2015-present, Facebook, Inc.


remove and give credit in the LICENSE file

jbaxleyiii · 2018-07-03T10:43:31Z

packages/apollo-server-defer/src/__tests__/starWarsQuery-test.ts

@@ -0,0 +1,498 @@
+/**
+ * Copyright (c) 2015-present, Facebook, Inc.


Same comment

jbaxleyiii · 2018-07-03T10:49:52Z

packages/apollo-server-defer/README.md

+
+#### Who needs this the most?
+
+- Media companies, banks, other data-heavy enterprise applications.


I don't think this is needed. You did a great job outlining the benefits above, lets not count usage of this out for anyone

You're right there! @defer is for everyone 👍

jbaxleyiii · 2018-07-03T10:53:06Z

packages/apollo-server-defer/README.md

+
+In order to support @defer, there are significant changes to be made to the execution phase if GraphQL.
+
+- Roll our own derivative of graphql.js execution that supports deferred responses and streaming patches using observables.


I would characterize this as extending graphql-js execution to support deferred responses. I think its important for us to compare the value of async itterable (which already has support for subscriptions) and observables. I like observables but want to have strong reason for adding another data type within the system.

However, since we don't expose this data type, it may not even be worth mentioning and instead keep it as a changeable implementation detail. I do think that we may one day want to allow returning of an async iterable or observable for control of something like stream / live so lets give this some careful thought.

@evans I know you have done some thinking here, care to chime in?

jbaxleyiii · 2018-07-03T10:54:51Z

packages/apollo-server-defer/README.md

+#### Apollo Client:
+
+- No changes required
+- Initial implementation of @defer support should come as a Apollo Link. Reads from a socket connection or some other event stream, keeps the partial response in memory, merging patches as they come and pushing it through the link stack.


I don't think we should do it this way. @defer will need special traits for reading / bypassing the cache so I think we should build it into Apollo Client core. Its also important to make this a seamless experience for people and adding another link is not ideal

Would love to chat with you more about the changes required on the client side. My original thinking was to minimize the amount of changes needed on the client, but we can and should strive for a seamless experience!

jbaxleyiii · 2018-07-03T10:55:40Z

packages/apollo-server-defer/README.md

+
+#### Transport:
+
+- @defer requires a transport layer that is able to stream or push data to the client, like websockets or server side events. If the transport layer does not support this, Apollo Server should fallback to normal execution and ignore @defer.


and warn in the console or something if falling back

jbaxleyiii · 2018-07-03T10:57:35Z

packages/apollo-server-defer/README.md

+    ```
+
+  - The reason that this behavior is useful is because some fields can incur high bandwidth to transfer, slowing down initial load.
+  - We could allow the user to control this behavior, by taking an optional waitFor argument:


I think this is really interesting and we probably also want some kind of timeout so a socket doesn't get stuck for some reason. @peggyrayzis this could have interesting supsense tie ins

Agreed! It would be SOO cool to sync the same waitFor times throughout the presentation and data loading layer!

clarencenpy · 2018-07-03T17:36:28Z

Replying to @jbaxleyiii

Why is this another package? I would have expected it to be in apollo-server-core. Lets be really careful about adding a ton of packages here as it increases complexity to manage

I'd definitely hope to merge it with apollo-server-core!

I think the results need to include the field that is defered with a null value. Since we are limiting usage to nullable types, this won't impact type generation tools and will require the least amount of work for clients. It also makes debugging easier since you don't have to determine if there was a problem with the sent operation, but rather can see if the initial result has the data here. Additionally, it makes it easier to work with inside UI components

That makes sense. Made the changes!

Akryum · 2018-07-17T08:21:38Z

So excited for this! 😻 Awesome work!

- [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected]

nhusher · 2018-08-16T14:52:55Z

What's the status of this? I have a custom defer solution that I would like to ditch if possible, but it seems like this PR has stalled. Is there anything I can do to help?

lilianammmatos · 2018-10-05T16:46:22Z

@clarencenpy I'm so excited for this feature! 🙌 Any updates on this PR getting merged? It seems to have fizzled out a bit. Let me know if I can be of any assistance.

sibelius · 2018-10-18T21:11:36Z

having a custom execute function for graphql will make it hard to keep it sync with execute from graphql-js code

should we try a RFC on this one to get this custom execute on graphql-js?

or maybe break execute in some functions to be able to create new executions easily

janhartmann · 2018-12-05T09:54:27Z

Is there any updates on @defer?
I can not seem to find any working example with Apollo Server to work + React Apollo?

BruceL33t · 2019-09-06T07:58:35Z

Status?

chrbradley · 2019-12-10T17:21:01Z

@BruceL33t

apollographql/apollo-client#3686 (comment)

fabis94 · 2020-12-10T17:38:31Z

Any updates here? Looks like there hasn't been any activity for a year

johnculviner · 2021-07-09T16:02:57Z

Please please please consider keeping going on this....

This solves one of my biggest gripes with GQL - your response performance is limited to the performance characteristics of the slowest resolver dependency unless you issue multiple queries which really limits the ability of GQL to serve as a near perfect abstraction over legacy APIs etc.

I have 3 queries to paint a page now to keep things performing and it's a real mess... they are aptly named fastQuery mediumQuery and slowQuery 😦

glasser · 2021-07-12T17:10:35Z

The implementation of @defer in graphql-js is close to done (so we won't need this PR which forks graphql-js to implement an old version of the @defer spec), though there are still some tweaks being made. We are hoping to support it in Apollo Server this year!

paumartin · 2022-01-04T15:04:16Z

@glasser any updates on this? 🙏

glasser · 2022-01-05T22:07:56Z

We didn't hit this year as you noticed, but yes, we have plans. We're working on our 2022 roadmap at the moment and there will be details on defer/stream in there.

emilio-martinez · 2022-01-25T20:59:37Z

@glasser is there a tracking issue for Apollo Server support of this feature?

glasser · 2022-01-25T23:13:42Z

Posted the roadmap today: https://github.com/apollographql/apollo-server/blob/main/ROADMAP.md

Will be breaking this up into individual issues shortly. Note that we are not committing to definitely supporting defer in 4.0.0, but part of the goal of the refactor is to make defer easy to implement instead of the challenge it is today due to unnecessarily tight coupling between the core Apollo Server logic and the web framework integrations.

clarencenpy force-pushed the defer-support branch 3 times, most recently from 0d15d81 to 6c1a1f1 Compare July 2, 2018 20:22

clarencenpy requested review from evans, glasser, jbaxleyiii, martijnwalraven and stubailo and removed request for glasser July 2, 2018 20:44

evans mentioned this pull request Jul 2, 2018

question: Throttling resolvers execution #1147

Closed

stubailo suggested changes Jul 2, 2018

View reviewed changes

This comment has been minimized.

Sign in to view

clarencenpy force-pushed the defer-support branch from a44fc1f to 6dc307e Compare July 3, 2018 01:03

martijnwalraven reviewed Jul 3, 2018

View reviewed changes

jbaxleyiii suggested changes Jul 3, 2018

View reviewed changes

evans force-pushed the version-2 branch from a4c15d7 to d4a8e1a Compare July 13, 2018 17:51

evans changed the base branch from version-2 to master July 14, 2018 00:30

clarencenpy force-pushed the defer-support branch from e4513d7 to 6965c25 Compare July 16, 2018 22:57

clarencenpy changed the base branch from master to version-2 July 16, 2018 23:07

clarencenpy force-pushed the defer-support branch from d0cefaa to ed8e06d Compare July 17, 2018 01:43

clarencenpy changed the base branch from version-2 to master July 17, 2018 01:44

clarencenpy force-pushed the defer-support branch 2 times, most recently from 5c783ee to 064b86d Compare July 17, 2018 02:19

clarencenpy force-pushed the defer-support branch from 064b86d to 668634a Compare July 17, 2018 18:17

clarencenpy added 6 commits August 7, 2018 10:38

Merge branch 'master' into defer-support

8a16e44

Merge branch 'master' into defer-support

0fc3d2a

Add tests for runQuery.ts

dd1ff85

Subscriptions should use the old execute from graphqljs

1fa4105

requestDidEnd only called after the last patch resolves

927ca65

Publish

7e28c89

- [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected]

sibelius mentioned this pull request Oct 18, 2018

[Modern] Deferred queries facebook/relay#2194

Open

lilianammmatos mentioned this pull request Jan 14, 2019

re-implement support for @defer directive #2181

Closed

4 tasks

This was referenced Feb 20, 2019

Large responses block event loop, consider streaming response json #801

Closed

Apollo Server 3.0 Roadmap #2360

Closed

Base automatically changed from master to main June 24, 2020 18:16

glasser closed this Jul 12, 2021

snyk-bot mentioned this pull request Jul 31, 2022

[Snyk] Security upgrade node-fetch from 2.6.7 to 3.2.10 cleancoindev/apollo-server#8

Open

vutting4221 mentioned this pull request Aug 1, 2022

[Snyk] Security upgrade node-fetch from 2.6.7 to 3.2.10 vutting4221/apollo-server#25

Open

snyk-bot mentioned this pull request Aug 1, 2022

[Snyk] Security upgrade node-fetch from 2.6.7 to 3.2.10 k4ny/apollo-server#6

Open

github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023

trevor-scheer deleted the defer-support branch April 21, 2023 16:54


		#### Apollo Server:

		In order to support @defer, there are significant changes to be made to the execution phase if GraphQL.


		#### Restrictions on @defer usage:

		- _Mutations:_ already execute serially, so there does not seem to be any use case for @defer. We should throw an error if @defer is applied to the root mutation type. However, @defer should apply normally for the selection set on a mutation.


		#### A Better Way

		_@defer_ provides a way for us to mark fields in our schema to _not_ wait on, because they might be:

		@@ -0,0 +1,1630 @@
		/**
		* Copyright (c) 2015-present, Facebook, Inc.

		@@ -0,0 +1,337 @@
		/**
		* Copyright (c) 2015-present, Facebook, Inc.

		@@ -0,0 +1,498 @@
		/**
		* Copyright (c) 2015-present, Facebook, Inc.


		#### Who needs this the most?

		- Media companies, banks, other data-heavy enterprise applications.


		In order to support @defer, there are significant changes to be made to the execution phase if GraphQL.

		- Roll our own derivative of graphql.js execution that supports deferred responses and streaming patches using observables.


		#### Transport:

		- @defer requires a transport layer that is able to stream or push data to the client, like websockets or server side events. If the transport layer does not support this, Apollo Server should fallback to normal execution and ignore @defer.

Conversation

clarencenpy commented Jul 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stubailo commented Jul 2, 2018

Uh oh!

stubailo commented Jul 2, 2018

Uh oh!

stubailo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

martijnwalraven Jul 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jbaxleyiii left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

clarencenpy commented Jul 2, 2018 •

edited

Loading

martijnwalraven Jul 3, 2018 •

edited

Loading

clarencenpy Jul 3, 2018 •

edited

Loading

clarencenpy commented Jul 3, 2018 •

edited

Loading