-
Notifications
You must be signed in to change notification settings - Fork 565
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
graphql-ws integration adjustments and tests (V3) #1609
Conversation
🦋 Changeset detectedLatest commit: 12fa22c The changes in this PR will be included in the next version bump. This PR includes changesets to release 31 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
https://github.com/dotansimha/graphql-yoga/tree/fix-graphql-ws-compat |
Fixed with https://github.com/dotansimha/graphql-yoga/tree/fix-graphql-ws-compat. Made me think, maybe we should implement WebSockets over Fetch API? There's nothing out there for it yet the Fetch Standard has a section for WebSockets. 🤔 |
@enisdenjo You are right. There is only |
Closes #1513
Fix "should execute mutation" test which is failing because the
usePreventMutationViaGET
plugin is preventing mutations from running over GETs and the WebSocket upgrade request is a GET.I was considering checking the headers and skipping the check if
Upgrade: websocket
; however, therequest
in the context is a Node request... @ardatan suggestions? UsenormalizeNodeRequest from @whatwg-node/server
? Includesocket
in context and skip check ifsocket
is set (this would require WebSocket type)?P.S. the test is not failing on
v3-graphql-http
branch because I dropped the plugin and left the check to graphql-http.