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

Handle multi-line data while composing SSE #250

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

atesgoral
Copy link

@atesgoral atesgoral commented Oct 6, 2023

Data with new line characters should be emitted as adjacent data fields.

Instead of:

data: hello\nworld\n\n

which goes through the wire as:

data: hello
world

It should be:

data: hello\ndata: world\n\n

which goes through the wire as:

data: hello
data: world

Without proper handling of new line characters, people using this library in the wild are forced to encode their data as JSON or Base-64 (and back after reading events from an SSE client).

Demonstration of the issue this PR is fixing (using a spec-compliant parser instead of focusing on individual chunks):

https://runkit.com/atesgoral/multi-line-sse-data

Spec:

https://html.spec.whatwg.org/multipage/server-sent-events.html#event-stream-interpretation


let { value: event } = await reader.read();
expect(event).toEqual(new TextEncoder().encode("event: message\n"));
expect(decoder.decode(event)).toEqual("event: message\n");
Copy link
Author

@atesgoral atesgoral Oct 6, 2023

Choose a reason for hiding this comment

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

Using a decoder on the actual value instead of using an encoder on the expected value gives human-readable diffs of strings. It's very hard to understand what's wrong by looking at the Int8 array diff.

@atesgoral
Copy link
Author

@sergiodxa bump

@vrish88
Copy link

vrish88 commented Apr 2, 2024

I'd also be interested in getting this merged, @sergiodxa anything we can do to help?

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.

2 participants