fastify websockets breaking change

[bhamon]

A breaking change has been added to @fastify/websockets@10 (fastify/fastify-websocket#289). It breaks the fastify integration.

The wsHandler first parameter type is now WebSocket rather than a NodeJS stream.
To create a stream from it an explicit call to ws.createWebSocketStream(socket) is required.

A dirty patch to make it work with the new version:

    wsHandler: (socket, req) => {
      const connection = ws.createWebSocketStream(socket);
      connection.socket = socket;
      return wsHandler.call(_app, connection, req);
    }

I'm concerned about how to properly propagate errors for the newly created stream.

[enisdenjo]

Hey hey, thanks for reporting this! We could detect the socket location during runtime, but the hard part is actually the TS types...

IDK how to approach this. Maybe release a new major version of graphql-ws that has no support for the legacy fastify? What do you think?

[bhamon]

It may be better yes, a runtime check doesn't feel right.
I'm a bit surprised by their change: even if under a new major version it is only referenced as a "fix" on the release. But the correction breaks the interface with much more to do on the user side.

[charsleysa]

@enisdenjo would you be open to having v10 support behind a config option? e.g. rawWebSocket and when it's set to true the first parameter of the handler is treated as the websocket instead of a connection object.

When the config option is used, the extra data can switch from providing connection to providing socket.

This would keep functionality the same for v9 and below while allowing v10 and above to be supported with only a minor release as these changes are technically not a breaking change due to having to explicitly opt-in.

Example usage for v10:

fastify.get('/graphql', { websocket: true }, makeHandler({ schema, rawWebSocket: true }));

[enisdenjo]

@charsleysa the issue is not only in runtime, but also with TS types - they dont work when using the latest fastify websockets.