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

Fix zeus generated file when using graphql-ws subscriptions and Node #390

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gthau
Copy link

@gthau gthau commented Feb 16, 2024

The file generated by Zeus is missing a few things when using both the --n (target Node) and --subscriptions graphql-ws (subscriptions with graphql-ws lib instead of the legacy websocket client):

  1. We must import the Headers type from @types/node-fetch as the type definition is not compatible with the one from TS lib DOM (Headers in the browser).
  2. for Node, according the graphql-ws recipes (https://the-guild.dev/graphql/ws/recipes), the webSocketImpl param must be passed to the createClient function, as Node doesn't have an out-of-the-box implementation of WebSocket as the browsers do

Point 1 creates a tsc compile time error.
Point 2 creates a runtime graphql-ws error.

Open question:
What to do with the apiSubscription function? I didn't know out to distinguish between the node or the browser for this function and how we could pass it a WebSocket implementation in the case of Node and graphql-ws.

Ghislain Thau added 2 commits February 16, 2024 18:31
Explanation:
- according to graphql-ws recipes (https://the-guild.dev/graphql/ws/recipes), using `createClient` on Node requires to explicitly pass the WebSocket implementation, contrary to the browser.
- when using Node, we want to import the type definition for Headers from 'node-fetch', if not the type definition from TS lib DOM is used, and it raises an Error when typechecking with tsc.
@aexol
Copy link
Collaborator

aexol commented Apr 4, 2024

Good point. You need to regenerated generated file however before commiting this. You should not edit generated file, but edit the files that are used as an output of generation

@gthau
Copy link
Author

gthau commented Apr 4, 2024

Good point. You need to regenerated generated file however before commiting this. You should not edit generated file, but edit the files that are used as an output of generation

Ohhh, I didn't see that it's a 2-level generation. I'll have a look at the source files that the libBuilder.ts file processes and bundles to create this generated.ts file.

@gthau
Copy link
Author

gthau commented Apr 16, 2024

I'm hitting a wall here.
From what I understand, the files under packages/graphql-zeus-core/TreeToTS/functions/new are not generated but are actual source code files bundled by libBundler.ts, along with files under packages/graphql-zeus-core/TreeToTS/functions/apiSubscription, with a bit of processing to remove imports and such.
I'm not sure how to integrate the env variable that's necessary to take the proper implementation of WebSocket (native in browser or from ws package in node world).

Any more detailed instructions are welcome.

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