feat: support graphql-ws client for GraphiQL IDE

graphql / express-graphql

Create a GraphQL HTTP server with Express.

MIT License

6.34k stars 538 forks source link

feat: support graphql-ws client for GraphiQL IDE #755

Closed junminstorage closed 3 years ago

junminstorage commented 3 years ago

To follow up #687 and continue to address #390, I drafted this PR to

[x] add support for https://github.com/enisdenjo/graphql-ws client for GraphiQL IDE.
[x] upgrade subscription-transport-ws to its latest version
[x] add example server startup script of using graphql-ws for subscription support
[x] add example server startup script of using subscriptions-transport-ws for subscription support
[x] use fetcher from https://github.com/graphql/graphiql

Developer provides websocketClient as part of graphiql option, its values are "v0" for subscriptions-transport-ws, "v1" for graphql-ws, I did this based on graphile/postgraphile#1338, open to better naming suggestion.

Specifically check out this tutorial script about how to start web socket server to support subscription using enisdenjo/graphql-ws : https://github.com/junminstorage/graphql-tutorial/blob/master/src/index_v1.js

You can also checkout the two newly added examples inside /examples directory in this PR for usage.

junminstorage commented 3 years ago

@acao this is the newly created PR. it looks all recently opened PRs failed on tests running on Node v12. Do you know why?

acao commented 3 years ago

@junminstorage not sure, appears to be the same issue on a few PRs and I can’t seem to find the actual error! Looking into it tonight, we will have this and all the subscriptions support docs released by friday at latest

junminstorage commented 3 years ago

@acao I think it is something similar to this issue https://github.com/nodejs/undici/issues/702

justinmchase commented 3 years ago

All of the failing tests for node 12 have the errors DeprecationWarning: The legacy HTTP parser is deprecated.

junminstorage commented 3 years ago

@justinmchase thank you for approving it. right, it is documented here: https://github.com/graphql/express-graphql/issues/756. @acao is it possible to continue to merge this PR and release it?

justinmchase commented 3 years ago

I agree it's probably something that can be fixed later. It appears that it would not create a regression.

acao commented 3 years ago

@junminstorage let me know if there’s anything I can help with here! We will need to get this test passing though. Seems to be an issue with a fixed http mocking utility not being compatible with the latest node 12. Do you think you’ll need any more patches to GraphiQL?

acao commented 3 years ago

@junminstorage ok we're good to merge here it looks like! thanks everyone for the awesome work, including @justinmchase for moving things along!

junminstorage commented 3 years ago

@acao I fixed the mocha issue as stated in #762 , this PR is ready to go.

acao commented 3 years ago

a note for everyone!

with this PR, you should now be able to use websockets with either the legacy or modern graphql-ws transport spec!

another PR must be merged for @stream and @defer support to become stable: https://github.com/graphql/graphql-js/pull/2839