✨ Added graphql-ws

[GauBen]

Working to fix #288

• [x] Added graphql-ws
• [x] Preserve the current behavior (named legacy, because non-standard)
• [x] Implement open and off, if they still make sense
• [x] Write the documentation
• [x] Add graphql-ws as an optional peer dependency

[GauBen]

Ping @aexol, there are a few merge conflicts now, are you working on this? Can I be of any help?

[LMaxence]

Hello @aexol, could you update us on the status of this please ? There are no reviews, no feedbacks, and it is hard for us to know if we can expect this feature that we really really really need merged at some point or if we have to fork the project on our side.

[aexol]

Sorry I thought it is not completed. Why not make a plugin instead of forcing graphql-ws to the main code?

[GauBen]

It looks like Zeus support subscriptions out of the box but that's not the case because the implementation of apiSubscription is non standard. This PR is more of a fix to add the standard implementation to the core, as a non-default feature flag to keep the current behavior, rather a new feature. This PR will make Zeus compatible with Apollo and Yoga subscriptions that both follow this standard.

I haven't written the documentation yet because I wanted to make sure that you were okay with changes to the core. If you agree on this design, I'll complete the todo list.

Also, I chose to implement it as a non-breaking change; both the current implementation and graphql-ws currently coexist in generated.ts: https://github.com/graphql-editor/graphql-zeus/pull/301/files#diff-96a2264dfde03a31fbad8f2aac04c691dd6a7291919fbcbf258ba30782f394daR664

Then, you may or may not make graphql-ws the default in the next major version.

[aexol]

If it won't break I am ok go ahead and finish it. It looks like you are implementing ts with strings. You don't have to do it anymore. Just use pure ts files and produce-lib script

[GauBen]

If it won't break I am ok go ahead and finish it.

Great news! I'll try to have it ready by the end of the week.

you are implementing ts with strings

Not really, I did it the same way as you:

• Created a TS file in src/TreeToTS/functions/apiSubscription/graphql-ws.ts
• Added a bit of magic to src/CLI/libBuilder.ts to handle the new files

[GauBen]

Ready for review!

I feel like a lot of things are wrong:

The type SubscriptionToGraphQL is not correctly implemented

In the current implementation, this is the open function:

return {
    // ...
    open: (e: () => void) => {
        ws.onopen = e;
    }
}

link

And this is the signature:

export type SubscriptionToGraphQL<Z, T, SCLR extends ScalarDefinition> = {
  // ...
  open: () => void;
};

link

I committed documentation artifacts

Is this ok?

There is no way to properly cleanup

I implemented the cleanup function with a Proxy over ws.close, but it feels dirty. Graphql-ws offers a proper way to unsubscribe and clean up:

https://github.com/enisdenjo/graphql-ws/blob/77682cd3d9bac748b5b24e2f8cf88e1d98c7a448/src/client.ts#L434-L442

We should abstract the underlying transport

There are two competing standards, graphql-ws and graphql-sse, who share the exact same interface. We might be interested in moving from concrete APIs (ws, open...) to more abstract ones (transport, cleanup...), in order to make Zeus implementation-agnostic.

[aexol]

Thanks @GauBen I'll try to review next days

[GauBen]

Thank you! I'll sort the merge conflicts out soon.

[GauBen]

Thanks for the review! You'll find all the changes in https://github.com/graphql-editor/graphql-zeus/pull/301/commits/6be6e474c311da6c14692bce414ea0f9b6fc271f .

[GauBen]

🎉

[aexol]

Please test 5.1.7 when ready :)

[aexol]

Thank you!

[GauBen]

Everything works on my side! 🎉