SSE Ext #2225: reinstantiate EventSource listeners upon reconnection logic#2272
Merged
Telroshan merged 2 commits intobigskysoftware:devfrom Feb 25, 2024
vlad-tkachenko:feat/2225-fix-missing-listeners-on-sse-reconnect
Merged
SSE Ext #2225: reinstantiate EventSource listeners upon reconnection logic#2272Telroshan merged 2 commits intobigskysoftware:devfrom vlad-tkachenko:feat/2225-fix-missing-listeners-on-sse-reconnect
Telroshan merged 2 commits intobigskysoftware:devfrom
vlad-tkachenko:feat/2225-fix-missing-listeners-on-sse-reconnect
Conversation
upon reconnection logic refs #2225
Renerick
approved these changes
Feb 25, 2024
Collaborator
Renerick
left a comment
There was a problem hiding this comment.
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
Contributor
Author
|
Hi @Renerick, Made a quick test, no issues found, reconnection logic works as intended. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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-packagemodule first.All the existing tests are passing:
Checklist
masterfor website changes,devforsource changes)
approved via an issue
npm run test) and verified that it succeededLast isn't checked, as it fails to start on my machine even on a clean
devbranch.