feat: allow GraphQL subscriptions #603
Conversation
|
| @@ -14,7 +14,8 @@ export class GraphQLParser extends ExpressParser { | |||
| return GqlExecutionContext.create(context); | |||
| } | |||
| getRequest(context: ExecutionContext): Request { | |||
| return this.getContext(context).getContext().req; | |||
| // For subscriptions, we need to get the connection from the context, not the req. | |||
| return this.getContext(context).getContext().req || this.getContext(context).getContext().connection; | |||
There was a problem hiding this comment.
Is this all that's necessary? Do the rest of the methods get the data they properly expect? And if so can you add an integration test to show this works the way we'd expect it to?
There was a problem hiding this comment.
TBH, i was refactoring our codebase and found that we made a new version of the GQLParser just for this one line and did not submit it back ;) In the end we do not use the subscriptions but i thought i would be waste to let is rot in our codebase.
Totally fine to close this and keep it for reference!
There was a problem hiding this comment.
If it works, and works regardless of the http adapter, I'm happy to merge this in. Just wasn't aware if this would be the only change required. I may pull this down and enhance it a bit with some tests over the weekend. Thanks for not letting the code rot away
| @@ -14,7 +14,8 @@ | |||
| return GqlExecutionContext.create(context); | |||
| } | |||
| getRequest(context: ExecutionContext): Request { | |||
| return this.getContext(context).getContext().req; | |||
| // For subscriptions, we need to get the connection from the context, not the req. | |||
| return this.getContext(context).getContext().req || this.getContext(context).getContext().connection; | |||
There was a problem hiding this comment.
Codacy has a fix for the issue: Replace this.getContext(context).getContext().req·||·this.getContext(context).getContext().connection with (⏎······this.getContext(context).getContext().req·||·this.getContext(context).getContext().connection⏎····)
| return this.getContext(context).getContext().req || this.getContext(context).getContext().connection; | |
| return ( |
|
Any chance to have this merged? |
No description provided.