feat: @defer, @stream, websockets support in a createGraphiQLFetcher utility #1770

acao · 2021-01-19T00:38:35Z

This introduces a backwards compatible client wrapper that provides graphql-ws subscriptions specification, as well as support for @defer and @stream IncrementalDelivery spec.

It wraps clients such as fetch-multipart-graphql, graphql-ws and even legacy support for subscriptions-transport-ws

new npm packages:

@graphiql/toolkit will become a set of types and utilities for building tools like GraphiQL. it will here prevent a minor cyclic dependency issue between the fetcher module and graphiql itself, which impacts typescript project references. breaking out graphiql into more component packages as such will enable other architectural changes for 2.0
@graphiql/build-fetcher is the standalone utility for generating a Fetcher instance that for this iteration will support http multipart IncrementalDelivery as well as graphql-ws transport

important changes:

graphiql Fetcher prop now supports an array of incremental responses.

Todo

new packages and utility and some integration tests
demo implementation
working server implementation of both graphql-ws & multipart with express-graphql defer stream release tag
more unit & integration test coverage
more documentation
end to end tests

how to demo locally

from the repo root:

yarn --force && yarn build
yarn start-graphiql

then this query should be enough to get started:

{
  streamable @stream(initialCount: 2) {
    text
  }
}

demo in netlify?

https://deploy-preview-1770--graphiql-test.netlify.app/?query=%7B%0A%20%20isTest%0A%20%20stream%3A%20streamable(delay%3A%20500)%20%40stream(initialCount%3A%202)%20%7B%0A%20%20%20%20text%0A%20%20%7D%0A%20%20anotherStream%3A%20streamable(delay%3A%20200)%20%40stream(initialCount%3A%200)%20%7B%0A%20%20%20%20text%0A%20%20%7D%0A%7D%0A

feedback welcome!

I'm interested in feedback on all working features, including the new utility, and the GraphiQL IncrementalDelivery implementation. (anything except documentation, tests, still some polish needed there, haha!)

Credit

@defer & @stream support has been a HUGE effort across the GraphQL community.
Spec development and libraries by @robrichard @lilianammmatos and @enisdenjo
Prior Art from @Urigo and @n1ru4l

acao · 2021-01-20T17:18:39Z

cc @n1ru4l @enisdenjo @robrichard @lilianammmatos @Urigo: you all might find this PR interesting as I'm using your all's code.

I figured you might know of some important common configuration settings I might be overlooking for the first iteration? particularly here and here is where the logic in question resides. I think it's best to build a proper integration suite for this.

codecov · 2021-01-20T19:50:37Z

Codecov Report

Merging #1770 (03d28b1) into main (881d8b5) will decrease coverage by 0.19%.
The diff coverage is 54.80%.

@@            Coverage Diff             @@
##             main    #1770      +/-   ##
==========================================
- Coverage   65.95%   65.76%   -0.20%     
==========================================
  Files          83       85       +2     
  Lines        4991     5088      +97     
  Branches     1581     1606      +25     
==========================================
+ Hits         3292     3346      +54     
- Misses       1672     1715      +43     
  Partials       27       27

Impacted Files	Coverage Δ
...phiql/src/components/DocExplorer/SearchResults.tsx	`0.00% <0.00%> (ø)`
packages/graphiql/src/components/ImagePreview.tsx	`0.00% <ø> (-2.33%)`	⬇️
packages/graphiql/src/components/GraphiQL.tsx	`58.26% <29.41%> (-0.53%)`	⬇️
...ql-language-service-server/src/MessageProcessor.ts	`60.28% <50.00%> (ø)`
packages/graphiql-create-fetcher/src/lib.ts	`56.00% <56.00%> (ø)`
...kages/graphiql-create-fetcher/src/createFetcher.ts	`65.62% <65.62%> (ø)`
...es/graphiql/src/components/DocExplorer/TypeDoc.tsx	`95.89% <100.00%> (ø)`
...-service-parser/src/__tests__/OnlineParserUtils.ts	`97.59% <100.00%> (ø)`
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 881d8b5...03d28b1. Read the comment docs.

acao · 2021-01-20T19:57:31Z

packages/graphiql-build-fetcher/src/lib.ts

+  try {
+    try {
+      // TODO: defaults?
+      wsClient = createClient({


@enisdenjo so, this is the default pattern, if only subscriptionsUrl is provided. the user can optionally provide their own instance of a client, which would bypass this behavior entirely.

the idea of a wsClientOptions option came to mind, however a wsClient option itself seemed even more powerful. do you think a wsClientOptions with graphql-ws's ClientOptions should be offered as well, here?

Other runtime options can be configured and I would recommend allowing the user to do so. You may have a custom server configuration and might need to fine tune the client so that they work in coherence.

But, this can be achieved by providing your own client instance, so, subscriptionsUrl + wsClient would be just enough.

Thinking about it a bit more, I'd expose the connectionParams too. I see it being used a lot because it is the "headers" of the WS connection.

ah, fantastic idea!

enisdenjo · 2021-01-21T13:14:28Z

packages/graphiql-build-fetcher/src/lib.ts

+  try {
+    try {
+      // TODO: defaults?
+      wsClient = createClient({


Other runtime options can be configured and I would recommend allowing the user to do so. You may have a custom server configuration and might need to fine tune the client so that they work in coherence.

But, this can be achieved by providing your own client instance, so, subscriptionsUrl + wsClient would be just enough.

enisdenjo · 2021-01-21T13:20:26Z

packages/graphiql-build-fetcher/package.json

+  },
+  "dependencies": {
+    "graphql-ws": "^4.1.0",
+    "graphql-transport-ws": "^1.9.0",


Why are you using the deprecated graphql-transport-ws lib? Its just an old name for graphql-ws, the two are fundamentally the same. graphql-ws@1.9.1 = graphql-transport-ws@1.9.0

I wouldnt advocate the usage of the deprecated library at all. The full focus should be on graphql-ws.

Suggested change

"graphql-transport-ws": "^1.9.0",

we want to have backwards compatibility with the old transport spec since it's so widely used?

Thats subscriptions-transport-ws.

is graphql-ws backwards compatible with older client implementations?

graphql-ws implements the new Protocol exclusively. It is not interoperable with Apollo's Protocol.

A WebSocket can use just one subprotocol; therefore, built-in interoperability is simply not feasible. Read more about this topic here and here.

If you want to support both the deprecated Apollo's lib and the new one, you have to configure a connection delegator on the server side.

Specifically this line of code ("graphql-transport-ws": "^1.9.0") should be removed, @enisdenjo authored both and renamed graphql-transport-ws to simply graphql-ws 👍

On the broader subject of subscriptions-transport-ws vs graphql-ws support; I also agree with @enisdenjo - the best approach is to try connecting with graphql-ws and if that connection fails then try again using subscriptions-transport-ws. Even though probably 95% of GraphQL APIs that support websockets currently don't support graphql-ws, in future this should shift since Apollo's protocol is officially unmaintained; we should skate to where the puck is going not where it has been. It's possible for servers to support both protocols (as the next release of PostGraphile does), in that case graphql-ws should win.

@benjie awesome! thats what we're doing in this PR already, just convincing @endisdenjo that it makes sense

I think what I meant to add here is this:

https://www.npmjs.com/package/subscriptions-transport-ws

got graphql-transport-ws and subscriptions-transport-ws mixed up sorry!

@enisdenjo my apologies for taking so long to realize this 😆 I knew something was wrong when I couldn't import the SubscriptionsClient class I was used to!

Haha, thats ok! Glad you figured it out. 😄

enisdenjo · 2021-01-21T13:26:55Z

packages/graphiql-build-fetcher/README.md

+  url,
+  wsClient: createClient({
+    url: subscriptionsUrl,
+    keepAlive: 2000,


I think that the keepAlive is superfluous here. Its mainly useful in applications which have a lot of moving parts where you dont want to waste resources (a lot of connects/disonnects to a WS server).

I would, however, recommend using the lazy: false option. This way GraphiQL would immediately connect to a server and the user can know right away if WS works (instead of being forced to issue a subscription operation just to check the basics).

Something like what PostGraphile's PostGraphiQL does: shows a status badge on the top right indicating the state of the WS connection.

Suggested change

keepAlive: 2000,

oh yeah i was just adding an arbitrary setting so people could see that more than just URL is available. lazy: false sounds like a good idea, I can add that to the default implementation

enisdenjo · 2021-01-21T13:35:37Z

packages/graphiql-build-fetcher/src/types.ts

+  /**
+   * url for websocket subscription requests
+   */
+  subscriptionsUrl?: string;


The WebSocket transport supports all GraphQL operations, including streaming queries (like the @live or @deferred directive).

I guess WS would be used only for subscriptions, which is why the prop makes sense; but, ultimately you can communicate with GQL only through WS if you wish.

Just a thought.

yes we're familiar with this pattern, however the intention of this library is to support the more widely adopted transport spec implementations, the "90%+ case" if you will.

if someone wants to ship a fetcher for that pattern in a standalone package like this one, they are welcome to

Would it be possible to create a simple option for using WebSocket for everything exclusively or an optional function configuration option that receives the operation payload and then depending on the returned value of that function (e.g. http or ws) the corresponding protocol will be used?

folks are welcome to create their own standalone fetcher packages like these for other patterns! we are only interested in adopting widely used, advanced transport specs for this effort

enisdenjo · 2021-01-21T13:36:47Z

packages/graphiql/package.json

    "fork-ts-checker-webpack-plugin": "4.1.3",
    "graphql": "15.4.0",
+    "graphql-ws": "^4.1.0",
+    "graphql-transport-ws": "^1.9.0",


Same as above, graphql-transport-ws is deprecated. I dont recommend using it!

Suggested change

"graphql-transport-ws": "^1.9.0",

acao · 2021-01-21T13:46:16Z

@enisdenjo another issue I had is that I couldn't get the graphql-ws implementation working with express-graphql. I'm focusing on the multipart side right now since I was able to get that working

enisdenjo · 2021-01-21T13:53:58Z

If you're using express-graphql out of the box, it has subscriptions-transport-ws built-in. The two libraries are not interchangeable. I think @junminstorage is cooking something up in this regards.

In the meantime, you have to configure express-graphql following this recipe in order to support the new Protocol.

acao · 2021-01-21T14:08:45Z

If you're using express-graphql out of the box, it has subscriptions-transport-ws built-in. The two libraries are not interchangeable. I think @junminstorage is cooking something up in this regards.

In the meantime, you have to configure express-graphql following this recipe in order to support the new Protocol.

i already tried the recipe and it didn't work. i’m not referring to the graphiql implementation in express-graphql obviously, since this new module is what will be used to replace that

enisdenjo · 2021-01-21T14:10:17Z

Ahh, alright then. Please share the problem you faced and I will investigate.

acao · 2021-01-21T19:12:07Z

@enisdenjo i fixed it!

the major delta from your recipie was for some reason that i needed to use the express app.listen() instead of server.listen(). probably has to do with express versions

i've been battling with graphql version clashes though, since we need this in experimental branch

acao · 2021-01-22T13:50:58Z

A bunch more changes are coming! including an implementation in netlify

junminstorage · 2021-01-23T16:22:16Z

If you're using express-graphql out of the box, it has subscriptions-transport-ws built-in. The two libraries are not interchangeable. I think @junminstorage is cooking something up in this regards.

In the meantime, you have to configure express-graphql following this recipe in order to support the new Protocol.

I am also building a fetcher support for both graphql-ws and subscriptions-transport-ws on express-graphql's graphiql IDE, it looks like this fetcher can be shared and reused? Will a umd version being published?

acao · 2021-01-23T16:25:07Z

If you're using express-graphql out of the box, it has subscriptions-transport-ws built-in. The two libraries are not interchangeable. I think @junminstorage is cooking something up in this regards.
In the meantime, you have to configure express-graphql following this recipe in order to support the new Protocol.

I am also building a fetcher support for both graphql-ws and subscriptions-transport-ws on express-graphql's graphiql IDE, it looks like this fetcher can be shared and reused? Will a umd version being published?

hey great question! this library will ship as GraphiQL.buildFetcher() only in the umd bundle for graphiql. a standalone umd would be cool, but i need to overhaul the build tooling yet again to do that properly haha

junminstorage · 2021-01-23T20:20:25Z

That could be just fine 😄 express-graphql's graphiql IDE already has dependency on graphiql umd bundle.

About this buildFetcher() method's signature , I can confirm passing subscriptionsUrl and wsClient either in graphql-ws or subscriptions-transport-ws will just be fine. Or even better, if we can pass in another string to tell this method which wsClient is being passed, in stead of "try connecting with graphql-ws and if that connection fails then try again using subscriptions-transport-ws", which is consistent with with graphile/crystal#1338 as well.

acao · 2021-01-24T19:50:53Z

good news! anyone following might want to try a field called streamable in the local dev build 😎

acao · 2021-01-24T19:54:32Z

acao · 2021-01-25T16:43:32Z

maraisr · 2021-01-26T06:21:37Z

May I suggest we wrap meros instead of fmg. A feather light runtime, that's faster, and also runs on the server. So for isomorphic apps, there is no need to wrap 2 clients.

acao · 2021-01-26T06:23:45Z

@maraisr i was just pondering that a little bit ago! i saw it linked in the spec and was gonna check it out tomorrow. would you like to create a PR against this PR to replace it?

acao · 2021-01-31T17:31:50Z

I think we're good here! Any necessary improvements can come in later iterations.

…utility (#1770) - support for `@defer` and `@stream` in `GraphiQL` itself on fetcher execution and when handling stream payloads - introduce `@graphiql/toolkit` for types and utilities used to compose `GraphiQL` and other related libraries - introduce `@graphiql/create-fetcher` to accept simplified parameters to generate a `fetcher` that covers the most commonly used `graphql-over-http` transport spec proposals. using `meros` for multipart http, and `graphql-ws` for websockets subscriptions. - use `graphql` and `graphql-express` `experimental-defer-stream` branch in development until it's merged - add cypress e2e tests for `@stream` in different scenarios - add some unit tests for `createGraphiQLFetcher` - add `changesets` because lerna publish no longer works. get rid of commitlint

…ility (#1770) - support for `@defer` and `@stream` in `GraphiQL` itself on fetcher execution and when handling stream payloads - introduce `@graphiql/toolkit` for types and utilities used to compose `GraphiQL` and other related libraries - introduce `@graphiql/create-fetcher` to accept simplified parameters to generate a `fetcher` that covers the most commonly used `graphql-over-http` transport spec proposals. using `meros` for multipart http, and `graphql-ws` for websockets subscriptions. - use `graphql` and `graphql-express` `experimental-defer-stream` branch in development until it's merged - add cypress e2e tests for `@stream` in different scenarios - add some unit tests for `createGraphiQLFetcher` - add `changesets` because lerna publish no longer works. get rid of commitlint

…utility (graphql#1770) - support for `@defer` and `@stream` in `GraphiQL` itself on fetcher execution and when handling stream payloads - introduce `@graphiql/toolkit` for types and utilities used to compose `GraphiQL` and other related libraries - introduce `@graphiql/create-fetcher` to accept simplified parameters to generate a `fetcher` that covers the most commonly used `graphql-over-http` transport spec proposals. using `meros` for multipart http, and `graphql-ws` for websockets subscriptions. - use `graphql` and `graphql-express` `experimental-defer-stream` branch in development until it's merged - add cypress e2e tests for `@stream` in different scenarios - add some unit tests for `createGraphiQLFetcher`

acao changed the title ~~feat: new encapsulation for transport spec~~ feat: @defer, @stream, websockets support in a buildGraphiQLFetcher utility Jan 19, 2021

feat: new encapsulation for transport spec

c9dab6a

acao force-pushed the feat/build-fetcher-util branch 2 times, most recently from e7244bd to 031b56b Compare January 20, 2021 19:47

acao force-pushed the feat/build-fetcher-util branch from 031b56b to a1e733e Compare January 20, 2021 19:50

acao commented Jan 20, 2021

View reviewed changes

chore: format/eslint fixes

27081f1

acao force-pushed the feat/build-fetcher-util branch from a1e733e to 27081f1 Compare January 20, 2021 20:00

acao added graphiql transport-spec labels Jan 20, 2021

enisdenjo requested changes Jan 21, 2021

View reviewed changes

acao added the language-spec label Jan 23, 2021

feat: iterate over increments in GraphiQL, stream fetcher in netlify!

ba7e7c0

acao marked this pull request as ready for review January 24, 2021 20:09

acao added the feedback wanted label Jan 24, 2021

junminstorage mentioned this pull request Jan 25, 2021

Support graphql-ws client for GraphiQL IDE graphql/express-graphql#739

Closed

5 tasks

acao force-pushed the feat/build-fetcher-util branch 2 times, most recently from e32555a to a774e93 Compare January 25, 2021 19:51

chore: types, cleanup, rename it, end to end tests

3876b5f

acao force-pushed the feat/build-fetcher-util branch from a774e93 to 3876b5f Compare January 25, 2021 20:26

chore: improve/stabilize e2e tests

5327791

acao force-pushed the feat/build-fetcher-util branch 3 times, most recently from e66366b to e75a498 Compare January 26, 2021 03:38

acao changed the title ~~feat: @defer, @stream, websockets support in a buildGraphiQLFetcher utility~~ feat: @defer, @stream, websockets support in a createGraphiQLFetcher utility Jan 26, 2021

chore: attempt netlify function

26feeeb

acao force-pushed the feat/build-fetcher-util branch from e75a498 to 26feeeb Compare January 26, 2021 03:49

acao mentioned this pull request Jan 26, 2021

Add support for defer and stream directives (feedback is welcome) graphql/express-graphql#726

Open

maraisr mentioned this pull request Jan 26, 2021

improvement: move incremental client from fmg to meros #1774

Merged

maraisr and others added 2 commits January 29, 2021 16:02

improvement: move incremental client from fmg to meros (#1774)

3292e85

chore: documentation cleanup

03d28b1

acao merged commit 766c9c3 into main Jan 31, 2021

acao deleted the feat/build-fetcher-util branch January 31, 2021 19:17

github-actions bot mentioned this pull request Feb 1, 2021

Version Packages #1779

Merged

github-actions bot mentioned this pull request Feb 3, 2021

Version Packages ChiragKasat/graphiql#1

Open

maraisr mentioned this pull request Feb 12, 2021

Support for defer/stream apollographql/apollo-client#7691

Closed

feat: @defer, @stream, websockets support in a createGraphiQLFetcher utility #1770

feat: @defer, @stream, websockets support in a createGraphiQLFetcher utility #1770

Conversation

acao commented Jan 19, 2021 • edited Loading

Todo

how to demo locally

demo in netlify?

feedback welcome!

Credit

acao commented Jan 20, 2021 • edited Loading

codecov bot commented Jan 20, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acao Jan 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enisdenjo Jan 21, 2021 • edited Loading

Choose a reason for hiding this comment

acao commented Jan 21, 2021

enisdenjo commented Jan 21, 2021

acao commented Jan 21, 2021

enisdenjo commented Jan 21, 2021

acao commented Jan 21, 2021 • edited Loading

acao commented Jan 22, 2021

junminstorage commented Jan 23, 2021 • edited Loading

acao commented Jan 23, 2021

junminstorage commented Jan 23, 2021

acao commented Jan 24, 2021 • edited Loading

acao commented Jan 24, 2021

acao commented Jan 25, 2021

maraisr commented Jan 26, 2021

acao commented Jan 26, 2021 • edited Loading

acao commented Jan 31, 2021

acao commented Jan 19, 2021 •

edited

Loading

acao commented Jan 20, 2021 •

edited

Loading

codecov bot commented Jan 20, 2021 •

edited

Loading

acao Jan 22, 2021 •

edited

Loading

enisdenjo Jan 21, 2021 •

edited

Loading

acao commented Jan 21, 2021 •

edited

Loading

junminstorage commented Jan 23, 2021 •

edited

Loading

acao commented Jan 24, 2021 •

edited

Loading

acao commented Jan 26, 2021 •

edited

Loading