Skip to content

Conversation

@jquense
Copy link
Member

@jquense jquense commented May 18, 2021

Adds an option for a pure websocket server.

@jquense jquense requested review from itajaja and r1b May 18, 2021 19:21
}

class GraphQLSocket extends EventEmitter {
protocol: 'graphql-transport-ws' | 'socket-io';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plan is to support: GraphQL over websockets protocol next

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am a little confused, this GraphQLSocket only seems to be used by the websocket, so why have the socket-io protocol as an allowed value?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah sorry. So in this case "socket-io" is really "4C's bespoke protocol of connect and authenticate calls" I don't remember why i named it "socket-io" maybe just because it depends on the socket-io handshake events

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and by "I named it" what do you mean by that? where is this name specified? do you think it's too late to change? maybe we can add a comment to avoid confusion

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's made up by me. this theoretically supposed to match the web socket sub protocol. 'socket-io' is a subprotocol used by socket-io which is why i just named it that for the pure web socket but it's not actually that protocol so i should disambiguate and name it 4c-subscription-server

@jquense jquense mentioned this pull request May 20, 2021
this.io.on('connection', (socket: io.Socket) => {
const request = Object.create((express as any).request);
Object.assign(request, socket.request);
this.opened(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onOpenConnection or initConnection

}

class GraphQLSocket extends EventEmitter {
protocol: 'graphql-transport-ws' | 'socket-io';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am a little confused, this GraphQLSocket only seems to be used by the websocket, so why have the socket-io protocol as an allowed value?

BREAKING CHANGE: the subscription server export is now an abstract class
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.

3 participants