Add context object to GraphQLDataSource.didReceiveResponse arguments
#3360
Conversation
| @@ -87,9 +91,10 @@ export class RemoteGraphQLDataSource implements GraphQLDataSource { | |||
| >, | |||
| ): ValueOrPromise<void>; | |||
|
|
|||
| public async didReceiveResponse<TResult = any>( | |||
| public async didReceiveResponse<TContext, TResult = any>( | |||
There was a problem hiding this comment.
Fly-by comment: Adding this as the first positional type argument would be a breaking change for any existing implementors (who aren't us, since we just rely on the default of any) — can it be added after TResult = any with its own default value?
(Realistically though, we should probably have this TContext as a type variable on the entire class rather than having it localize to this method.)
There was a problem hiding this comment.
You're absolutely right. I took the easy way in 0ecd617.
I'll take another stab at TContext on the class, but RemoteGraphQLDataSource.process() wasn't really having it.
There was a problem hiding this comment.
This is looking wonderful! I think it certainly will work in its current state and meet the necessary goal but there's just some polishing and bike-shedding of patterns (patterns which your PR did not introduce) to sort out before we finalize it and move it into the release.
packages/apollo-gateway/src/datasources/RemoteGraphQLDataSource.ts
Outdated
Show resolved
Hide resolved
packages/apollo-gateway/src/datasources/__tests__/RemoteGraphQLDataSource.test.ts
Outdated
Show resolved
Hide resolved
packages/apollo-gateway/src/datasources/__tests__/RemoteGraphQLDataSource.test.ts
Show resolved
Hide resolved
packages/apollo-gateway/src/datasources/__tests__/RemoteGraphQLDataSource.test.ts
Outdated
Show resolved
Hide resolved
This adds a third parameter to the `didReceiveResponse` method on a GraphQLDataSource. This gives the hook access to the context object, allowing it to read and modify the context as necessary. As demonstrated by the included test, this enables a GraphQLDataSource to inspect the response and add information to the context accordingly. This also provides parity with another hook, willSendRequest, which already has access to the context object.
Co-Authored-By: Jesse Rosenberger <[email protected]>
Co-Authored-By: Jesse Rosenberger <[email protected]>
Co-Authored-By: Jesse Rosenberger <[email protected]>
The sub-classing pattern is one we'd prefer to propagate, so we should use it in our tests as well. This also enables the use of `super`!
This reverts commit 5c6bf1f.
trevor-scheer
force-pushed
the
trevor/data-source-context
branch
from
7712eb8 to
c610ba6
Compare
|
Excuse my inability to understand this, but I'm having trouble following what 'context' is here. Flow chart implies that it's the context of the final response, and some of the documentation has hints of that as well, but I wanted to confirm this. What I'd really like to do is pass through some response headers from service X to the final response headers of the gateway. |
…nts (apollographql/apollo-server#3360) This adds a third parameter to the `didReceiveResponse` method on a GraphQLDataSource. This gives the hook access to the context object, allowing it to read and modify the context as necessary. As demonstrated by the included test, this enables a GraphQLDataSource to inspect the response and add information to the context accordingly. This also provides parity with another hook, willSendRequest, which already has access to the context object. Apollo-Orig-Commit-AS: apollographql/apollo-server@8881d45
This adds a third parameter
contextto thedidReceiveResponsemethod on aGraphQLDataSource. This gives the hook access to the context object, allowing it to read and modify as necessary.As demonstrated by the included test, this enables a
GraphQLDataSourceto inspect theresponseand add information tocontextaccordingly.This also provides parity with another hook,
willSendRequest, which already has access to thecontextobject.