-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Gateway support via new @apollo/server-gateway-interface package #6771
Conversation
🦋 Changeset detectedLatest commit: 94f8439 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for apollo-server-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 94f8439:
|
Until now, the refactored AS4 did not support Apollo Gateway (or any implementation of the AS3 `gateway` option). That's because `GraphQLRequestContext` is part of the API between Apollo Gateway and Apollo Server, and that type has changed in some minor but incompatible ways in AS4. (Additionally, clashes between `declare module` declarations in AS3 and AS4 caused issue, but we removed those declarations from this branch in PRs #6764 and #6759.) This commit restores gateway support. It does this by having AS4 produce an AS3-style request context object. It uses a new `@apollo/server-gateway-interface` package to define the appropriate types for connecting to Gateway. (Note: this package will be code reviewed in this PR in this repo, but once it's approved, it will move to the apollo-utils repo and be released as non-alpha there. Once we've released AS4.0.0 we can move that package back here, but trying to do some prereleases and some non-prereleases on the same branch seems challenging.) This PR removes the top-level `executor` function, which is redundant with the `gateway` option. (Internally, the relevant field is now named `gatewayExecutor`.) Some types had been parametrized by `TContext`, because in AS3, `GraphQLExecutor` (now `GatewayExecutor`) appeared to take a `<TContext>`. However, even though the type itself took a generic argument, its main use in the return from `gateway.load` implicitly hardcoded the default `TContext`. So we are doubling down on that and only allowing `GraphQLExecutor` to use AS3's default `TContext`, the quite flexible `Record<string, any>`. Most of the way toward #6719.
80e8f81
to
8a6ee4a
Compare
"name": "@apollo/server-gateway-interface", | ||
"version": "0.0.0", | ||
"description": "Interface used to connect Apollo Gateway to Apollo Server", | ||
"type": "module", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can drop this given no runtime?
"type": "module", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure that TSC doesn't look at this field? I guess that's just a nodenext thing? Do you think it hurts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair question. I don't think it hurts, and I can't say for sure about nodenext
so I don't mind if you leave it.
status?: number; | ||
} | ||
|
||
export interface GatewayHTTPHeaders { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this just be a re-export of FetcherHeaders
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooh, nice, I think you're right. FetcherHeaders also has extends Iterable<[string, string]>
but that shouldn't hurt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Request and Response are intentionally smaller though, but you didn't suggest that :) )
Co-authored-by: Trevor Scheer <trevor.scheer@gmail.com>
Co-authored-by: Trevor Scheer <trevor.scheer@gmail.com>
This package was reviewed in apollographql/apollo-server#6771
This package was reviewed in apollographql/apollo-server#6771
This package was reviewed in apollographql/apollo-server#6771
Trevor said it would be fine to merge (and he's about to be out for a week) |
This package was reviewed in #6771
This package was reviewed in #6771
This package was reviewed in #6771
Until now, the refactored AS4 did not support Apollo Gateway (or any
implementation of the AS3
gateway
option). That's becauseGraphQLRequestContext
is part of the API between Apollo Gateway andApollo Server, and that type has changed in some minor but incompatible
ways in AS4.
(Additionally, clashes between
declare module
declarations in AS3 andAS4 caused issue, but we removed those declarations from this branch in
PRs #6764 and #6759.)
This commit restores gateway support. It does this by having AS4 produce
an AS3-style request context object. It uses a new
@apollo/server-gateway-interface
package to define the appropriatetypes for connecting to Gateway.
(Note: this package will be code reviewed in this PR in this repo, but
once it's approved, it will move to the apollo-utils repo and be
released as non-alpha there. Once we've released AS4.0.0 we can move
that package back here, but trying to do some prereleases and some
non-prereleases on the same branch seems challenging.)
This PR removes the top-level
executor
function, which is redundantwith the
gateway
option. (Internally, the relevant field is now namedgatewayExecutor
.)Some types had been parametrized by
TContext
, because in AS3,GraphQLExecutor
(nowGatewayExecutor
) appeared to take a<TContext>
. However, even though the type itself took a genericargument, its main use in the return from
gateway.load
implicitlyhardcoded the default
TContext
. So we are doubling down on that andonly allowing
GraphQLExecutor
to use AS3's defaultTContext
, thequite flexible
Record<string, any>
.Most of the way toward #6719.