✨ feat: add crossws adapter

[m1212e]

Adds an adapter for crossws.

I know this has been discussed in #546 but with SvelteKit on the way of implementing native WS support via crossws I thought this might be useful nonetheless. We need to wait for h3js/crossws#149 before proceeding with this.

[enisdenjo]

Great stuff, thanks! Please make sure to also add it to the test servers so that the full server test suite can make sure it works as expected.

[m1212e]

Hey, I'm trying to wrap around how the tests work and have two questions:

1. https://github.com/m1212e/graphql-ws/blob/cca14ac3a3e9b3a8ea869aca2b7be52a97527620/tests/utils/tservers.ts#L966 What exactly are these config test things for and how do they have to be set?

2. What is a proper way of converting the NodeAdapter from crossws to some form the test setup can work with? https://github.com/m1212e/graphql-ws/blob/cca14ac3a3e9b3a8ea869aca2b7be52a97527620/tests/utils/tservers.ts#L768 As far as I can see the crossws NodeAdapter is a reduced version of WebSocketServer but I can't really figure a way to make this work.

I'd be very thankful for your input on this!

[enisdenjo]

1. The skipX and itForX are tests that either skip or run only for the given server. If you do skipWS('should do things', () => {}) the test will run for all servers except the WS server; the inverse is true for itForWS('should do things', () => {}), the test will run only for WS skipping other servers.
2. Sorry but I am not familiar with crossws so I cant really answer your question. If crossws wraps the ws module in Node environments, then there should be a way. However, if there really isnt, you can set null and update the comment here - and of course make sure the tests needing the server pass.

Thank you for your work on this!

[m1212e]

Ok, thanks! I tested a lot locally and with various example projects and it works pretty nice like this. Unfortunately I did not fully understand the test setup yet so I'll have to look into that when I find the time.

[enisdenjo]

Thanks! The server testing suite is not all that complicated, you have to:

• Implement the TServer interface with use/crossws
• Add it to the tServers array
• And make sure all tests of use.test.ts pass

Having the tests is a necessity for merging this PR. No hurry though, whenever you get time.

P.S. there are other failing checks, please see the CI

[m1212e]

Hey, thanks for your fast response times, really helps me out/lets me stay motivated. Probably also not beneficial to do this after a full 8h work day all the time so please excuse all those questions.
But I think I might have a quick and not totally stupid solution for the tests? Maybe not, curious what your take on this is but I kinda just hijacked the existing uws test, put the already existing uws crossw adapter in between and got nearly all tests passing, only 2 are left failing:

 FAIL  tests/use.test.ts > crossws > Keep-Alive > should dispatch pings after the timeout has passed
 FAIL  tests/use.test.ts > crossws > Keep-Alive > should terminate the socket if no pong is sent in response to a ping

I know this isn't 'really' testing the specifics of the crossws adapter but since these are integration tests anyway and crossws isn't a real adapter for a runtime on its own but more of a glue between the two systems this might suffice your requirements. Let me hear what you think!

Regarding the failed tests: These refer to this part of the spec, right? I'm not tooo into WS and graphql-transport-ws so I'm wondering if this a specific graphql-transport-ws or a general WS thing. I'm asking because the deno and bun client do not have explicit logic on this, it seems to me like the clients do that automatically? Would the crossws adapter be the right place to implement a ping logic and what if the client already does(like bun?), should the adapter check that and prevent double pings in these cases?

[m1212e]

Hey, did you by any chance have the time to take a look or some feedback on this?

[pi0]

(crossws maintainer here) LMK if could do any help on this ❤️

[enisdenjo]

Other than this one remark, this looks good to me.

Please create a changeset using yarn changeset in order to preare a release.

I'll also have to check why's the CI not running here. 🤔

[enisdenjo]

Aside from adding the example in the changelog, it'd be a good idea to add it to the get started documentation too to advertise the support.

In the get-started.mdx, add a new entry under "Start the server" called "With crossws".

[enisdenjo]

Oh and don't forget to run yarn and update the lockfile, so that we can have the CI run.

[enisdenjo]

Please fix the formatting, run yarn format.

Regarding the failing tests and your comment, those tests are important and should not be ignored. Those keep-alive pings and pongs are not from the GraphQL over WS spec, but are from the WebSocket spec itself.

You can advise the ws implementation to see how the keepalive is implemented there:

graphql-ws/src/use/ws.ts

Lines 108 to 131 in 5161bd9

	// keep alive through ping-pong messages
	let pongWait: ReturnType<typeof setTimeout> | null = null;
	const pingInterval =
	keepAlive > 0 && isFinite(keepAlive)
	? setInterval(() => {
	// ping pong on open sockets only
	if (socket.readyState === socket.OPEN) {
	// terminate the connection after pong wait has passed because the client is idle
	pongWait = setTimeout(() => {
	socket.terminate();
	}, keepAlive);
	// listen for client's pong and stop socket termination
	socket.once('pong', () => {
	if (pongWait) {
	clearTimeout(pongWait);
	pongWait = null;
	}
	});
	socket.ping();
	}
	}, keepAlive)
	: null;

Disclaimer, I don't fully understand crossws and how/whether this can be implemented in crossws, maybe @pi0 can help? What are your thoughts here? How do you see handling keepalive?

[pi0]

Escalated to h3js/crossws#154 for keep alive support

[enisdenjo]

Ok cool, thanks @pi0.

@m1212e since this is a missing feature in crossws, you can go ahead and skip those two tests for crossws until #154 gets resolved and then we can implement it here if necessary. Thank you for your hard work here! We're close, lets get this to the finish line!

[enisdenjo]

Great work @m1212e! Thank you!