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

SSE Ext #2225: reinstantiate EventSource listeners upon reconnection logic #2272

Merged
merged 2 commits into from
Feb 25, 2024
Merged

SSE Ext #2225: reinstantiate EventSource listeners upon reconnection logic #2272

merged 2 commits into from
Feb 25, 2024

Conversation

vlad-tkachenko
Copy link
Contributor

@vlad-tkachenko vlad-tkachenko commented Feb 2, 2024

Description

Please describe what changes you made, and why you feel they are necessary. Make sure to include
code examples, where applicable.

Currently when EventSource loses connection with the server, e.g. when server restarts and later SSE ext reconnects it creates new EventSource, but is not adding any previously registered listeners. As a result while server issues new events, SSE ext no longer consumes them.

Corresponding issue:

Refs: #2225

Testing

Please explain how you tested this change manually, and, if applicable, what new tests you added. If
you're making a change to just the website, you can omit this section.

Tested on a real application by using the exact changes and patching the npm package with patch-package module first.
All the existing tests are passing:

Screenshot 2024-02-02 at 10 26 56

Checklist

  • I have read the contribution guidelines
  • I have targeted this PR against the correct branch (master for website changes, dev for
    source changes)
  • This is either a bugfix, a documentation update, or a new feature that has been explicitly
    approved via an issue
  • I ran the test suite locally (npm run test) and verified that it succeeded

Last isn't checked, as it fails to start on my machine even on a clean dev branch.

Copy link
Collaborator

@Renerick Renerick left a comment

Choose a reason for hiding this comment

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

Hello and thank you for the contribution! Your PR highlighted an issue with the whole registerSSE logic, so I took the liberty to modify it a bit more and update this PR. Your change LGTM, but I would like to also ask you to check out my changes and verify that everything still works as expected with regards to reconnection logic and everything else. Thanks!

Renerick added a commit to bigskysoftware/htmx-extensions that referenced this pull request Feb 25, 2024
@Telroshan Telroshan merged commit 21cdfcf into bigskysoftware:dev Feb 25, 2024
1 check passed
@vlad-tkachenko
Copy link
Contributor Author

Hi @Renerick,

Made a quick test, no issues found, reconnection logic works as intended.
Cheers.

@vlad-tkachenko vlad-tkachenko deleted the feat/2225-fix-missing-listeners-on-sse-reconnect branch February 26, 2024 09:11
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.

None yet

3 participants