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

FEATURE: Swap HTML on Server Sent Events (like WebSockets) improved version #185

Closed
wants to merge 11 commits into from
Closed

FEATURE: Swap HTML on Server Sent Events (like WebSockets) improved version #185

wants to merge 11 commits into from

Conversation

benpate
Copy link
Collaborator

@benpate benpate commented Sep 4, 2020

Since this is starting to get some scrutiny, I think this is actually the better code to review. It also has the largest changes, so it NEEDS more work to review it.

This follows the design I laid out in my original PR for SSE handling and uses a big chunk of work from @tomberek that pulls a lot of node replacement logic out of the AJAX call and into its own function. This should be thoroughly vetted by someone with more experience than us, but it could lay the groundwork for updating the original AJAX calls as well, which would save a bunch of code.

<!-- Server Sent Events: URLs and Event Names-->
<div hx-sse="/my-url"> Works on untyped "message" events
<div hx-sse="/my-url EventName"> Works on "EventName" events
<div hx-sse="/my-url EventName1 EventName2"> Works on both "EventName1" and "EventName2" events

<!-- hx-swap and hx-target-->
<div hx-sse="/my-url" hx-target="#nodeId"> fragment is swapped into a different node
<div hx-sse="/my-url" hx-swap="outerHTML"> changes how the fragment is swapped
<div hx-sse="/my-url" hx-swap="innerHTML settle:1s"> additional swap arguments work.  This one adds a settling timer.

<!-- hx-swap-oob in response fragments -->
<div id="anotherId" hx-swap-oob="true">
  This fragment is generated on the server.  When it arrives on the browser, 
  it is swapped into "anotherId" instead of the node that originated the call.
</div>

<!-- multple tags / connection pooling -->
In a single document, several DOM nodes can all connect to a single URL.  In the example below, 
events Event1, Event2, and Event3 are distributed to each tag from a single connection.  If one node 
is removed, then its event handlers are removed, but the handlers for the remaining nodes are 
untouched.  If all nodes are removed, then the connection is closed
<div hx-sse="/my-url Event1">
<div hx-sse="/my-url Event2">
<div hx-sse="/my-url Event2 Event3"> 

I know this leaves behind your "multiple event types" idea, but this syntax seems much clearer to me. What do you think?

It would be really good to get some other eyes on this and to confirm that it's behaving correctly. We've made a lot of changes so far, and we shouldn't go too much further without a solid check. In case anyone has trouble verifying this, I've made a tiny server program (in go) that returns a bunch of random strings to HTMX at random (<2s) intervals. I left it running for over an hour with no noticeable leaks in the browser.

Next Steps

  • Thorough vetting of doSwap to confirm that we're not making any subtle changes to HTMX's original behavior.

Future Work

  • Backport doSwap into WebSockets and regular AJAX requests. This will save a lot of duplication in the code.
  • Possibly use the listener pool to handle polling, mentioned in Does processPolling() leak? #164
  • Possibly expose SSE functions to the outside page, so that external scripts can also use this connection pool.

benpate and others added 10 commits August 14, 2020 21:32
First pass at making a new SSE interface.

SUPPORTS:
- New syntax `<div hx-sse="/url EventName" hx-swap="outerHTML" hx-target="somewhere">`

DOES NOT SUPPORT:
- triggers. Original syntax has been removed.
- unnamed events (can't make onmessage work)
- connection pooling (required before use)
- extra parameters on hx-swap, like: swap, settle, scroll, show (this should *probably* be implemented before use)
This may match the spec: https://html.spec.whatwg.org/multipage/server-sent-events.html#server-sent-events-intro
```
The default event type is "message".
```
Adding default “message” eventType if none is specified.  This matches the specifications Tom provided from the WHATWG.
- "works" with limited testing.
- Adding/Removing EventSources to/from the pool works correctly.
- Events coming through to page correctly.
- Much testing, cleanup, and code style needed.
- Haven't yet tested multiple SSEs on a single page.
This is a first draft of a better kind of connection pooling.  Since the number of connections is unlikely to grow very large, it seems easier and more efficient to just put them all into one array, and scan through it whenever we need.

This enables a new function `auditEventListeners` which verifies that the (probably < 4) remote connections are still supporting active DOM nodes, and saves us the extra work of walking an entire sub-tree hunting for them.

I think this same structure could save us even more work if applied to WebSockets, too.  But, that effort should wait until we've confirmed that we like this for SSE.
- rewriting connection sharing to use a single array instead of a more complicated map of urls/connections.
This is much more comprehensive than my original `handleContent` function.

Still more work required to verify that this is handling all of the features we need, and then backport it into WebSocket handlers and AJAX handlers.
Sorry these are all manual cases.  If someone has suggestions on how to make these automated, I'm happy to improve them.
@1cg
Copy link
Contributor

1cg commented Sep 8, 2020

This change has gotten large enough that I'd like to move it to its own branch. I want to read through the documentation and understand the conceptual idea before I commit to merging it. I don't want to change the core code too much, so if it is possible to avoid a major refactor I'd prefer that.

I'm pretty slammed this week, but will likely have time to look at it next week. This is the last major change I'm looking to integrate before a 1.0-alpha

@benpate
Copy link
Collaborator Author

benpate commented Sep 8, 2020

Yes. And, we were coming up with a lot of new syntax in the fly, then writing this as a proof of concept that it would work.

Before going any further, it would be great to get a target syntax that you believe would fit well into HTMX. That would help to dictate other architectural decisions that flow from that.

If it would help to explain the code in this PR, I'm happy to meet online some time to walk through it and ask/answer questions about the implementation.

I'm also perfectly happy to scrap the whole PR and help rewrite it from scratch, if that makes it fit better with the existing code.

@clarkevans
Copy link

clarkevans commented Sep 9, 2020

Use of websockets is pretty important to data science communities, such as Julia, which use "reactive" front-end models that tightly integrate user experience with back-end computations. Communication needs to be fluid in both ways, where moving a slider causes server side events which cause a series of updates to front end, as calculations are performed. Conversely, system monitoring dashboards, think of a HTML version of "top", are also a good application of sockets. This project, HTMX, is pretty attractive to data science people, if we could use within SVG diagrams as well as HTML, and not be tied to a heavy-weight framework such as React or Vue.

@benpate
Copy link
Collaborator Author

benpate commented Sep 10, 2020

Hey Clark,

I'm not heavy into data science, but I agree with your statement on all fronts. I dug into both SSE and WebSockets (see PR #150) and realized that my projects could probably get away with a lighter-weight, unidirectional SSE.

This PR is working to bring SSE up to par with what already works in WebSockets, although my first pass is rather barbaric on HTMX's internals to do it. Luckily, I believe the adults have arrived, and @1cg is looking through it now to make a real recommendation on how to move forward. So, hopefully, we'll have an "official" version of a feature similar to this soon.

@1cg
Copy link
Contributor

1cg commented Sep 11, 2020

@benpate @clarkevans @tomberek please see my design proposal for an updated SSE implementation here:

https://gist.github.com/1cg/5dd26530aeec92a9389b367747bbb5ee

Comments welcome

@tomberek
Copy link

The spec talks about SSE messages without an event field be defaulted to “message”. Would it be possible for “swap on message” to apply to those data-only messages? In that case there is less of an arbitrary distinction between Named and Data-Only. It also produces a smoother transition when iterating a design between no event-type and adding one.

@1cg
Copy link
Contributor

1cg commented Sep 11, 2020

@tomberek I have updated the proposal based on your feedback and extended the definition so that an element can declare and SSE end point and also consume events from it.

@tomberek
Copy link

The extraction of the handling code from Ajax and applied to all of Ajax, sse, and WebSockets would be a good thing to keep/look into. It would ensure consistent behavior and be easier to maintain.

@benpate
Copy link
Collaborator Author

benpate commented Sep 13, 2020

Closing in favor of new SSE design

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

4 participants