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) #155

Closed
wants to merge 2 commits into from
Closed

FEATURE: Swap HTML on Server Sent Events (like WebSockets) #155

wants to merge 2 commits into from

Conversation

benpate
Copy link
Collaborator

@benpate benpate commented Aug 4, 2020

This pull request updates SSE content to swap HTML into the current page, just like WebSockets, without removing the existing hx-trigger behavior that also uses SSE data.

I believe it addresses issue #66, although it does not use the syntax that you described there. Instead, it looks like this:

<div hx-sse="connect /my-events EventNameToListenFor">

Now, the hx-sse attribute takes a third (optional) parameter that specifies an event name to listen for. When events using this name come through the SSE channel, that is treated as an "out-of-band swap" and swapped into the underlying DOM (just like WebSockets do now).

To do this, I added the optional event handler to the processSSEInfo function, and moved some of the existing WebSockets code into a common place -- a new function called processFragments -- that is called by both the WebSockets implementation and the SSE implementation.

Hopefully, this code fits the style and spirit that you're building in HTMX. I'm excited to see all of the potential applications of this toolkit and will be happy to work on any enhancement suggestions, corrections, improvements to this code.

🤘

Separated out some functions to be used in common between WebSockets and SSE, but SSE is not working correctly.  Old content is removed, but the new content is not added to the DOM.

I've traced the errors down into the swapOuterHTML function, but am not sure what's causing the problem.

Checking in so that I can try something else on another branch.
Removing debug code.  It turns out this was working the whole time, but the problem was in my SERVER code.
@benpate benpate marked this pull request as ready for review August 4, 2020 04:29
@tomberek
Copy link

tomberek commented Aug 9, 2020

Functions as described. At first i expected the hx-sse div's DOM itself to be replaced, but eventually found that the sent content must be something like <div id="moreContent" hx-swap-oob="true">Things to say</div>

@benpate
Copy link
Collaborator Author

benpate commented Aug 9, 2020

Yes. It is different from a regular swap. I was trying to mimic the way that the web socket implementation works, which was mentioned in one of the original discussions on this topic.

In addition, SSEs can carry an event name, so one URL can deliver different kinds of events. (I just learned that while doing this PR). This (potentially) makes them different from a regular swap.

I didn’t want to break the existing functionality, so this seemed like the cleanest addition. But, maybe it raises a better question — what would be the BEST api, regardless of backwards compatibility?

For instance, if I want to swap content in to many different places, perhaps the swap-oob syntax isn’t the best approach. Instead, HTMX could pool a single URL connection and then put tokens in to listen for specific events. This might be more flexible, and more in line with the rest of HTMX expectations.

I’m open to suggestions, and will be happy to rework this PR if a better design is agreed upon.

@tomberek
Copy link

tomberek commented Aug 9, 2020

The two approaches are interesting. Either can subsume the other, so it's not a matter of the exact power provided, only UX. And this discussion has some overlap with WebSockets design.

What about allowing both the simplicity of a simple swap and the power of listening to multiple events? Can we adopt the same convention as normal GET's?

Default

By default, swap in to the same element. (equivalent to hx-swap=innerHTML). Think a constantly updating status.

<div hx-sse="connect /someUrl">replace me</div>

sending something like <p>New content<p>

using hx-swap

Append item. Think about updating log lines.

<ol hx-sse="connect /someUrl" hx-swap="beforeend"><li>item 1</li></ol>

sending something like <li>item2</li>

Listening to a specific event

<div hx-sse="connect /someUrl someEvent">replace me</div>

This is nice when you want to filter down what kind of events are accepted. This is sometimes nice to have debug events and a main data stream.

Syntax Question:

<div hx-sse="connect /someUrl someEvent someEvent2 someEvent3">replace me</div>

This could be like an OR statement for the type of event?

Oob-swap

This is very powerful and is what suspect is current behavior. This seems like it can coexist with the previous syntax.

<div hx-sse="connect /someUrl someEvent">doesn't change</div>
<div id="somewhereElse">replace me</div>

sending something like <p id="somewhereElse" hx-oob-swap="true" >New content<p>
If the content has hx-swap-oob attribute at the top-level, you can redirect the swap elsewhere in the DOM. This is powerful but that forces the server to have much more knowledge and intertwines things too much (though is still a powerful escape hatch).

Reuse connection

It would be nice to re-use the connection rather than forcing unique streams to both get events and filter them. The OR syntax above can listen to multiple events, but how would you control behavior. That control seems to require an oob-swap, but there may be a declarative way to do this.

What about specifying behavior like this:

<ol hx-sse="connect /someUrl someLog someStatus" 
                  hx-target="this somewhereElse"
                  hx-swap="beforeend innerHtml">
    <li>item 1</li>
</ol>
<section id="somewhereElse">
... status pending
</section>

Where the someLog event matches up with the this target and beforeend behavior, while someStatus will go somewhereElse and replace the innerHtml.

Thoughts?

@benpate
Copy link
Collaborator Author

benpate commented Aug 9, 2020

Instead of giving a thoughtful response (like you did), I'm just going to shoot from the hip and amend my comments later :)

In general, the oob-swaps are strange to me, and I'd much prefer a simpler syntax as you suggested. If changing the syntax is on the table, then I think this would be much better.

I would also like to have multiple options for listening to "all" vs. "specific event names" as you mentioned, but I think this may not be really useful without sharing a single connection across multiple DOM nodes. I think this might also take the place of the oob-swap syntax. It would perform the same function of updating multiple parts of the page, but be easier to visualize and troubleshoot what's happening.

Connection sharing would be awesome but is probably more difficult to build because it would require that we make a centralized registry of SSE connections, instead of just storing them in the DOM node as HTMX currently does. It would be better, but also harder. So, I'd probably vote to make this a follow-on feature after the current syntax is updated.

So, I think here's my vote:

YES for your "Default" syntax. This would be a change to current SSE behavior, so it would need to be done carefully.

YES for adding hx-swap to this as well.

LATER - on connection pooling, because we shouldn't bite off too much all at once.

LATER - on adding one or more event names to filter events into specific DOM nodes.

NO to the current OOB-SWAP syntax.

@benpate
Copy link
Collaborator Author

benpate commented Aug 15, 2020

I haven't updated this PR, but I am working on another branch that uses the syntax we've been discussing.

It removes the "connect" command in the hx-sse attribute, and supports optional hx-target and hx-swap as well. I really wanted to make the "eventName" optional, too, but can't figure out how to make EventSource.onmessage functions work. My initial impression is that this feels great, and fits the rest of the HTMX idioms well.

Calls could look like this:

<div hx-sse="/url eventName" hx-target="#domNode" hx-swap="outerHTML"></div>

NOTES ON CONNECTION POOLING
Looking at the code that I have now, this really should support connection pooling to be useful. Since we can't match all events in a single DOM node, we're stuck making new connections for each different event type. I think it's doable, but this is a good stopping point for now. My first thought would be to make something like this:

<div hx-sse="/my-url Event1"></div>
<div hx-sse="/my-url Event2" hx-target="#id"></div>
<div hx-sse="/my-url Event3" hx-swap="beforeend"></div>

I think this may be easier to read and troubleshoot than packing several kinds of events into a single DOM node.

HTMX should just be able to recognize that we're calling a duplicate EventSource URL, and attach a new EventListener to it without too much trouble. We'll need to be careful disconnecting listeners, but it's not impossible.

@tomberek
Copy link

onmessage is specifically for messages that do not have an event and is equivalent to:

evtSource.addEventListener("message",function(event) {
             console.log(event)
})

https://developer.mozilla.org/en-US/docs/Web/API/Server-sent_events/

So if the user doesn't specify an eventName then it can default to "message". The EventSource spec does NOT support listening to ALL messages anyway, so we don't have to either! There is no way to express in Javascript: "please call this handler on ANY message".

We can allow a default of "message", or allow someone to attach extra by name:

<div hx-sse="/my-url Event1 Event2 Event3"></div>

is easy to interpret by making multiple addEventListener calls. It also is easy to understand that it is one connection.

Pooling vs "opportunistic pooling"

The above example is easy to implement as a single connection with multiple event listeners. This is simple pooling because it's all on the same DOM node and adding the listeners can all be done in the same place.

The next are where the difficulties lie:

opportunistic pooling: style 1, duplicate event source, unique events

<div hx-sse="/my-url Event1"></div>
<div hx-sse="/my-url Event2 Event3"></div>

This would require a "global" listener pool where you add more handlers onto the existing one.

opportunistic pooling: style 2, duplicate event soure, same events

<div hx-sse="/my-url Event1"></div>
<div hx-sse="/my-url Event1"></div>

If you had a global listener pool, this should work as expected, because it matches the semantics of "addEventListener". But if one of them is removed, you'd have to remove that listener to avoid having some functions unexpectedly called if those events show up.

Removing

You can remove via a similar mechanism as add.
https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/removeEventListener

Note: i'll take a look at your new branch, i'm not super familiar with the HTMX codebase, but we'll see....

@tomberek
Copy link

pooling: recommendation

To clarify, the browser does not do "global opportunistic connection pooling" either. So perhaps we leave that feature out for now?

multiple event types

regarding this syntax:

<ol hx-sse="connect /someUrl someLog someStatus" 
                  hx-target="this somewhereElse"
                  hx-swap="beforeend innerHtml">
    <li>item 1</li>
</ol>
<section id="somewhereElse">
... status pending
</section>

Looking at your branch, this part of the addEventListener call would by default only support multiple event types making the same target+swap callbacks.

var swapSpec = getSwapSpecification(elt)
var target = getTarget(elt)
var fragment = makeFragment(e.data);
var settleInfo = makeSettleInfo(elt);

Can those have an extra parameter so that the multipleEvent syntax would work? (where i is the i'th eventType matching up with the i'th target and i'th swap parameter.

var swapSpec = getSwapSpecification(elt,i)
var target = getTarget(elt,i)
var fragment = makeFragment(e.data);
var settleInfo = makeSettleInfo(elt);

@tomberek
Copy link

ah, it looks like hx-target could in theory support the multiple targets by adding some splitOnWhitespace logic. But hx-swap cannot because it can be a compound specification. It would have to splitOnSemicolon or something like that. But then our syntax is confusing and not consistent.

<ol hx-sse="connect /someUrl someLog;someStatus" 
                  hx-target="this;somewhereElse"
                  hx-swap="beforeend;innerHtml">
    <li>item 1</li>
</ol>
<section id="somewhereElse">
... status pending
</section>

Another approach is to allow a parameter on the attribute itself (you can't have multiple attributes with the same name

<ol hx-sse="connect /someUrl someLog someStatus" 
                  hx-target="this"
                  hx-swap="beforeend swap:1s"
                  hx-target-2="somewhereElse"
                  hx-swap-2="innerHtml settle:1s"
>
    <li>item 1</li>
</ol>
<section id="somewhereElse">
... status pending
</section>

@benpate
Copy link
Collaborator Author

benpate commented Aug 15, 2020

Wow! Thanks for all of the super-helpful information. I'm working to digest all of what you've said and will try to respond to everything you raised. If I miss something, please assume sloppiness, not malice :)

onmessage

First, thanks for the education on the onmessage handler. I tried to figure out the difference but was stumped why I couldn't get it to fire. Obviously, if Javascript doesn't support an "all" message handler, then we don't need to either. The code I have should be pretty easy to update to accommodate null type messages.

Given the above, I also like the idea of attaching multiple event names to a single handler. It's a simple enhancement that should be easy to implement and it makes the toolkit more powerful.

Hooks into Existing Code

Before I go further, I should say that I'm just some dude on the Internet who jumped into another person's codebase and started breaking things. Virtually all of the code in my pull request was written by @1cg or other people who actually know HTMX. So, functions like getSwapSpecification() getTarget() makeFragment() makeSettleInfo() and others were pre-existing, and I'm just trying to use them with as little disruption to the existing code as possible. I do think the enhancements we're discussing could benefit from a code refactor, but I don't want to rock the boat too hard. Also, @1cg is pretty unavailable until at least September, so these kinds of ideas should really wait until he's had a chance to weigh in. (sorry @1cg for assuming your gender)

Opportunistic Pooling

Your breakdown of "opportunistic pooling" is very helpful, and it highlights all of the ways that this could work. As this feature matures, I think a global pool will eventually be necessary, one way or the other. It should not take too much code to implement, and will clear up a lot of complexity for developers who use HTMX.

So, I'd vote for adding multiple event names and possibly restoring oob-swap handling for event data. That should cover most of the use cases I can think of.

Multiple Values in HX-Target and HX-Swap

I'd also vote to not implement multiple values in hx-target and hx-swap because there are so many ambiguous ways it could be handled. For example, one challenge I see with the "splitOnSemicolon" approach (and the hx-swap-2, hx-target-2 syntax) is trying to interpret the developer's intent when things get weird. How should the code handle a situation where we have three event names, two targets, and one swap directive?

My Next Steps

So, I'll find some time soon to try to make progress on this syntax. Let me know if you think this will work for now :)

<!-- hx-sse 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-swap="outerHTML" hx-target="#nodeId">
hx-swap and hx-target function *almost* normally, but don't yet support additional parameters on hx-swap

<!-- hx-swap-oob in response fragments -->
<div id="anotherId" hx-swap-oob="true">
  This content would follow regular swap-oob rules, being swapped into node with id "anotherId" 
  instead of the calling node
</div>

When that is done, I'll look at "connection pooling" to support this syntax:

<!-- Multple Tags -->
In a single document, multiple SSE connections could be created to the same URL, and HTMX will 
manage multiple event handlers on the same connection
<div hx-sse="/my-url Event1">
<div hx-sse="/my-url Event2">

<div hx-sse="/my-url Event2 Event3"> 
In rare cases where one event name is called on multiple DOM nodes (Event2, in this example) handlers will
be removed from the pool individually.  If all handlers are removed from an EventSource, then the EventSource
would be close()-ed to conserve server resources.

@tomberek
Copy link

Individual events: looks great

Multiple events: I agree that this is where it get's ugly. I was able to get the hx-target-2 syntax working in my PR to your branch. But it might be too much of a change and a bit gross. Consider that PR a work-in-progress/experimental.

At the least though the doSwap re-use between XHR and SSE seems to be nice to work with.

@benpate
Copy link
Collaborator Author

benpate commented Aug 16, 2020

That’s awesome. I’ll get started on individual events, and check out your PR for multiple events.

I tried to plug in to as much existing code as possible. There’s a lot happening under the hood that is still magic to me. For instance, there are several features (like the additional swap parameters, and swap-oob) that are not working here, yet, because they are embedded directly in other handler functions. This is (one of) the refactorings that I mentioned before. I think the first step might be to make a couple of higher-level functions that centralize all of the swap handling, that would just need to be called with an element, and the new content. The new function would do all of the work to handle all of the amazing edge cases that HTMX already does. This could be called whenever we get an SSE event, and eventually retrofitted back into the AJAX handlers and WebSocket event handlers. The code is all there — just some of it is not accessible right now.

I think that would be something to tackle once we have a clear direction on the syntax to use going forward. @1cg — how’s that day job treating you? :D

@benpate
Copy link
Collaborator Author

benpate commented Aug 16, 2020

I just added another branch that moves all SSE EventSources into a single, global array. The connection pooling code came out nicer than I expected. I still have more testing to do before making a PR, but it seems really promising.

One big benefit is that it saves us from having to walk the sub-tree of DOM nodes searching for open connections (bound to individual DOM nodes) every time we swap in new content. Instead, we just scan through our registry of connections, verifying that their nodes still exist in the DOM. For large pages, and pages that DON'T use SSE, this could be a huge win.

If we like this, I think there would be a big benefit to moving WebSockets into this structure, too.

It seems like my procrastination reflexes are kicking in hard on the swap-oob code. There's a lot happening in there, so I think it's scary for me to jump into. But, that's still on the TODO: list.

@benpate
Copy link
Collaborator Author

benpate commented Aug 17, 2020

I'm punting on the other tags for now, and adding issue #165 to ask for help making a common function to handle this -- it's above my pay grade.

@benpate
Copy link
Collaborator Author

benpate commented Aug 17, 2020

@tomberek your doSwap function is working fantastically in the new branch. It's a drop-in replacement that magically handles all of the remaining features that we had discussed. Here's what works right now:

<!-- 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 benpate changed the base branch from master to dev August 31, 2020 02:09
@1cg 1cg changed the title Swap HTML on Server Sent Events (like WebSockets) FEATURE: Swap HTML on Server Sent Events (like WebSockets) Sep 2, 2020
@1cg
Copy link
Contributor

1cg commented Sep 2, 2020

OK, what are you crazy kids doing in here...

👀

@benpate
Copy link
Collaborator Author

benpate commented Sep 2, 2020

I swear it made sense at the time.

Maybe consider this as a proof of concept (and a learning project for me) on what SSE could be. I think it could really use some guidance from you (@1cg) on where you think SSE should go, along with the syntax to support it. From there, we have the raw materials to make just about anything work.

My favorite changes are actually on this branch that was an "improvement" on this one. I've also put together a demo site on http://sseplaceholder.openfollow.org/ that demos some of these features, if that helps...

@benpate
Copy link
Collaborator Author

benpate commented Sep 4, 2020

Since this is getting more attention right now, I think I'd like to close this PR in favor of #185, which is the enhanced version of this code. I've also added some manual test cases that demo this new functionality (with an external dependency on my SSEplaceholder website).

I won't close it for now, because @1cg should have the chance to weigh in on the syntax and design of this feature. But I think #185 is the best one, and the one that HTMX should adopt going forward.

@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

3 participants