-
Notifications
You must be signed in to change notification settings - Fork 259
Add subscription support to @apollo/subgraph
#2388
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
Add subscription support to @apollo/subgraph
#2388
Conversation
👷 Deploy request for apollo-federation-docs pending review.Visit the deploys page to approve it
|
🦋 Changeset detectedLatest commit: 22e8305 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 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 |
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. |
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.
The code changes seem reasonable to me. Would you mind adding a test case and a changeset (via npx changeset
)?
Thanks for reviewing @trevor-scheer! @martijnwalraven it sounds like this is really close - see Trevor's review comments. Thanks! |
👋 @martijnwalraven @trevor-scheer - is there anything left to do with this, or do you think we can go ahead and merge it? |
I pushed up a test and changeset so this is ready to go. Validated that the test fails before @martijnwalraven's changes (and it clearly passes with them). |
The changes needed to support subscriptions in
@apollo/subgraph
are fairly minimal: we need to augment the resolver typings, and propagate thesubscribe
property toGraphQLFieldConfig
.One thing to call out is that this PR follows the definitions in
graphql-js
. That means bothresolve
andsubscribe
are optional, even though it could make sense to require at most once. The property types are also rather loosely defined: there is no attempt to validate thatsubscribe
returns anAsyncIterator
for example.