Giter VIP home page Giter VIP logo

Comments (4)

enisdenjo avatar enisdenjo commented on May 22, 2024 3

You're very welcome! I am always happy to hear that my work helps. 😄

Given the fact that the client presents supported sub-protocols on the initial WebSocket handshake - the server can choose where to delegate the request (old vs new protocol).

Keeping this in mind, I dont plan on "polluting" a fresh protocol with superfluous additions of an outdated one (same for this reference implementation graphql-ws).

Sorry if this is not the answer you expected... I am fully aware of the transitioning phase, backwards compatibility and the, somewhat, rocky road to adaptation - but changing a working system "for convenience only" is not what I intend to do.

Furthermore, even the main readme of subscriptions-transport-ws recommends transitioning to this lib.

A proper solution is doing a PR to Apollo's stack integrating support for the new protocol and allowing backwards compatibility to the old one.

from graphql-ws.

enisdenjo avatar enisdenjo commented on May 22, 2024 1

Its much less work to support both transports in userland, than integrating interoperability here and maintaining that additional complexity introduced in favour of laziness.

I don't plan on fixing anything subscriptions-transport-ws (not on their side, not on mine, not ever), and following the abundance of open issues and PRs, seems like no one is... With this, it is not worth supporting both transports - you may want to add backwards compatibility only in favour of easing the transition of a hot system. Nothing else.

In my humble opinion, if you decide to support both in new projects: you're doing it wrong!

Furthermore, a minimal legacy-friendly library boils down to 60 lines of code (you can find the Recipe in the readme too):

import https from 'https';
import ws from 'ws'; // yarn add ws
import { execute, subscribe } from 'graphql';
import { GRAPHQL_TRANSPORT_WS_PROTOCOL } from 'graphql-ws';
import { useServer } from 'graphql-ws/lib/use/ws';
import { SubscriptionServer, GRAPHQL_WS } from 'subscriptions-transport-ws';
import { schema } from 'my-graphql-schema';

// graphql-ws
const graphqlWs = new ws.Server({ noServer: true });
useServer(
  {
    schema,
    execute,
    subscribe,
  },
  graphqlWs,
);

// subscriptions-transport-ws
const subTransWs = new ws.Server({ noServer: true });
SubscriptionServer.create(
  {
    schema,
    execute,
    subscribe,
  },
  subTransWs,
);

// create https server
const server = https.createServer(function weServeSocketsOnly(_, res) {
  res.writeHead(404);
  res.end();
});

// listen for upgrades and delegate requests according to the WS subprotocol
server.on('upgrade', (req, socket, head) => {
  // extract websocket subprotocol from header
  const protocol = req.headers['sec-websocket-protocol'];
  const protocols = Array.isArray(protocol)
    ? protocol
    : protocol?.split(',').map((p) => p.trim());

  // decide which websocket server to use
  const wss =
    protocols?.includes(GRAPHQL_WS) && // subscriptions-transport-ws subprotocol
    !protocols.includes(GRAPHQL_TRANSPORT_WS_PROTOCOL) // graphql-ws subprotocol
      ? subTransWs
      : // graphql-ws will welcome its own subprotocol and
        // gracefully reject invalid ones. if the client supports
        // both transports, graphql-ws will prevail
        graphqlWs;
  wss.handleUpgrade(req, socket, head, (ws) => {
    wss.emit('connection', ws, req);
  });
});

from graphql-ws.

morris avatar morris commented on May 22, 2024

Thanks for your response - totally understand and agree with your reasoning about the spec! Especially since subscriptions-transport-ws was never a standard to begin with, it should in no way hold back defining a future-proof and secure spec.

My line of thought was more a practical one. I believe these simple user stories hold:

  • Server implementors want to support as many GraphQL WebSocket agents (and versions thereof) as possible.
  • Client implementors want to use whatever agent fits their stack, without server-specific modifications (e.g. the modifications described in the graphql-ws README).

Currently, the only viable transition for these users I see is

  1. to install both graphql-ws and subscriptions-transport-ws on the server-side and splitting by the sub-protocol selected by client agents (as you suggested),
  2. ask clients to switch to graphql-ws-based agents or have them implement server-specific modifications,
  3. and keep subscriptions-transport-ws on server-side as long as there are (relevant) non-complying clients.

That's totally feasible, but obviously less than ideal in terms of effort. I believe for currently live projects (1.) may not be avoidable as legacy clients might have locally optimized against subscriptions-transport-ws in subtle ways and you'll not want to invite WebSocket-sub-protocol-level problems by switching libraries.

However, even in new projects, as a server implementor I will still have to do (1.) in order to support the numerous GraphQL agents in the ecosystem - or force the clients to use the tricks from the graphql-ws README. If I had a server-side library at hand that supported both protocols, and possibly even fixed some issues with subscription-transport-ws, that would reduce our combined invest in this significantly.

Anyways, I just wanted to report that graphql-ws could be this library with some simple modifications, not at all that it should. After all, if nothing breaks, this might actually slow down adoption, as there's no need to transition. (I don't like this thought as I'm an interoperability fan, but it could be true nonetheless.)

We could have a separate, legacy-friendly library once the protocol is finalized, so I guess the more important work is to review the protocol in as much detail as possible and make it somewhat standard soon 🚀

Thanks again for your hard work, I'll be looking out for future developments for sure!

from graphql-ws.

enisdenjo avatar enisdenjo commented on May 22, 2024

Closing due to inactivity.

from graphql-ws.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.