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

lastEventId not implemented? #50

Closed
valadaptive opened this issue Dec 10, 2023 · 1 comment
Closed

lastEventId not implemented? #50

valadaptive opened this issue Dec 10, 2023 · 1 comment

Comments

@valadaptive
Copy link
Contributor

valadaptive commented Dec 10, 2023

The spec says that server-sent events should have a lastEventId property, which is set to the value of the last id field that was sent through the stream (or the empty string if there was none). It doesn't necessarily have to belong to the current event; it's just the last event ID witnessed within that stream.

This library does something vaguely similar, but with some deviations:

  • The property is named id, not lastEventId
  • It's reset with each new event sent, rather than persisting
  • It's set to null by default, whereas lastEventId defaults to the empty string
  • lastEventId forbids null bytes; the id property allows them

I want to bring this in line with the spec, but I'm concerned about backwards compatibility with previous versions of the library. There are a few options:

  • Leave id as-is and document the differences.
  • Add lastEventId as a separate property, but keep id and its behavior. This would complicate the codebase, but would allow for better standards compatibility and backwards compatibility.
  • Do a major version bump, and fix this along with some other spec deviations (e.g. firing CustomEvents instead of MessageEvents; not setting the event's origin; lack of reconnection logic).
@mpetazzoni
Copy link
Owner

lastEventId is indeed not currently supported (see https://github.com/mpetazzoni/sse.js?tab=readme-ov-file#todos-and-caveats), but it shouldn't be hard to add without backwards compatibility issues.

I'm not sure I fully understand your analysis here, especially the id vs lastEventId part, since sse.js makes no attempt (so far) to implement lastEventId; the id field on events is something else entirely.

I'll send a PR with a drat proposal implementation.

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

No branches or pull requests

2 participants