Skip to content
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

Fix event stream formatter so that support graphql-sse protocol #5473

Merged
merged 2 commits into from
Oct 8, 2022

Conversation

sunghwan2789
Copy link
Collaborator

Fix EventStreamResultFormatter to format an event stream for decoding.

Copy link
Member

@michaelstaib michaelstaib left a comment

Choose a reason for hiding this comment

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

Hey there, thanks for contributing.

Did you check with the graphql-sse protocol specification?
The payloads no longer look to conform with the spec.

One other thing I spotted is that you removed the compiler memory optimizations that we were using.

@michaelstaib
Copy link
Member

The protocol spec can be found here:
https://github.com/enisdenjo/graphql-sse/blob/master/PROTOCOL.md

@michaelstaib
Copy link
Member

OK, read into the link you posted... so I interpreted it wrongly. I will revert the changes you did to static fields so that we do not carry a static array around, but leave your fix in place.

@michaelstaib
Copy link
Member

Ah damnit :) I see why you did what you did :D

@michaelstaib michaelstaib merged commit 64dda45 into ChilliCream:main Oct 8, 2022
@michaelstaib
Copy link
Member

Thank you for your help!

@michaelstaib
Copy link
Member

michaelstaib commented Oct 8, 2022

I have integrated your fix into preview 68 ... should be available in the next 60 minutes:
https://github.com/ChilliCream/hotchocolate/releases/tag/13.0.0-preview.68

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants