Skip to content
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

Library API #7

Merged
merged 3 commits into from
May 29, 2019
Merged

Library API #7

merged 3 commits into from
May 29, 2019

Conversation

gobengo
Copy link
Contributor

@gobengo gobengo commented May 28, 2019

The motivation for this PR is to clean up what we've got such that the good stuff can be used as a library by other projects that want to add graphql-ws over WebSocket-Over-Http support to their apps that use apollo-server.

Library APIs

We're trying to expose an API that is as close as possible to some patterns documented by apollo-server itself. As such, I made these two new files based on those patterns.

  • subscription-server-api - Okay so I started with this based on our emails. This is the API documented for subscription-transport-ws. Once I got to implementing, though, I realized that this really isn't useful for most apps. This API only adds the websocket handler. It's out of scope to also wire up the rest of the GraphQL API that is done over HTTP. I really would be surprised if many app developers use this API. This is the API that's used inside of apollo-server-core's ApolloServerBase (which is annoying because it tightly couples all ApolloServers with the WebSocket-specific transport). So I moved on to the next API
  • apollo-server-express-api - This was inspired by these docs for how to do GraphQL Subscriptions with apollo-server when you might also want some other express middlewares in your app. I really have to imagine that this is the most common starting point for any real-world App. But adding support for this API involves two steps:
    1. Add this middleware to your express app so that it responds to all graphql-ws-over-WebSocket-Over-Http requests in a way that allows subscriptions to hang open
    2. Wrap whatever PubSubEngine you're using with EpcpPubSubMixin so that calls to PubSubEngine#publish also get published to the GRIP server via EPCP. Notably, creating this mixin requires a GraphQLSchema instance so that the graphql-ws message published to subscriptions can have the right fields added.
    • I guess it would be flashy if these were somehow collapsed into one line. But I really think that sugar is more lipstick than helpful. The whole point of GRIP + EPCP is deconstructing a Duplex Stream into two separate ones: one for publishing and the other that is read-only for subscribing. These two required actions mirror the underlying model. See next for an API we can expose to encapsulate this a bit more, but without being as flexible for people who want to preconfigure their own Express application.

Other API Ideas

  • Technically the first API documented for doing subscriptions with ApolloServer is this one, which I'd just call the apollo-server-api. You basically pass all your config to an ApolloServer constructor and then call ApolloServer#listen on the instance and things just magically work (unlike HttpServer#listen, ApolloServer#listen() returns a promise of both a graphql HTTP url and
    a graphql subscriptions URL that WebSocket conections should be made to).
    • @jkarneges I can spend some time trying to make this one work. It definitely looks the coolest in a README. But I do think the apollo-server-express-api I did above is what your customers probably use in the real world. Since I did that one already, and I'm already deviating from the subscription-server-api we emailed about, I wanted to pause here and check in before spending time on this.

Other Changes

There are lots of lines changed in here, mostly hygiene:

  • Removing subscriptions-transport-core as it wasn't needed after first trying to actually reuse code from subscriptions-transport-ws but realizing we couldn't because it was written against a full-duplex interface that wasn't conducive to the request-response nature of WebSocket-Over-Http
  • Moving a bunch of things out of FanoutGraphqlExpressServer that really had nothing to do with Fanout or this particular demo. I moved them into their own files in directories that indicate a logically separate module
    • graphql-epcp-pubsub
      • EpcpPubSubMixin can wrap any graphql PubSubEngine to forward any publishes to a URI via EPCP
    • graphql-ws - graphql-ws is a wire protocol with JSON messages that doesn't really need to be used over WebSocket itself, yet the canonical implementation of it is in subscription-transport-ws and tightly coupled to a WebSocket API. This module is for graphql-ws utilities that aren't coupled to any particular transport API
    • subscriptions-transport-apollo - ApolloServer depends on subscriptions-transport-ws, and hides away some pretty useful logic it uses for passing to the SubscriptionServer constructor from subscriptions-transport-ws. This exports a bunch of things that would be useful for making alternate SubscriptionServers that aren't from subscription-transport-ws.
      • ApolloSubscriptionServerOptions - this is useful if you want to be able to do what apollo-server-express's ApolloServer#installSubscriptionHandlers does but using an underlying SubscriptionServer that is not the one from subscription-transport-ws
    • subscriptions-transport-ws-over-http
      • GraphqlWebSocketOverHttpConnectionListener basically translates graphql-ws to WebSocket-Over-Http. This is the thing that currently encapsulates the logic to always respond to incoming graphQl subscriptions by creating a Grip-Channel equal to the subscription name. This will need to change for more complicated/parameterized subscriptions
      • GraphqlWsOverWebSocketOverHttpExpressMiddleware can be added as middleware to any Express app to automatically accept all incoming graphql-ws connections that come in over WebSocket-Over-Http

@jkarneges jkarneges merged commit 5f64aa1 into master May 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants