Allow a single stream response to update multiple elements

[blopker]

Hey all, I'm working with the folks integrating Turbo into Django at https://github.com/hotwire-django. Right now we're making a demo site based off of the RealWorld app spec (https://github.com/gothinkster/realworld) to show how the integration works in practice. Over all it's been a smooth experience.

However, there's one feature we're having a little trouble with. Specifically the 'Favorite button' feature. You can see it in action here. Notice the two Favorite Article buttons. They both contain the same content and both need to be updated in response to the user's action.

Ideally we would respond to the button press with a single Turbo Stream replace response to update both buttons at the same time with the new favorite count and button active state.

In this proposal PR I've made the buttons work by using querySelectorAll instead of getElementById to select all elements with the target ID on the page. Then I've updated each Stream action method to iterate over a list.

What do you think?

[shadowtime2000]

I am pretty sure that querySelectorAll is really slow, but ngl I am not that good with raw DOM ops like these and I am not a maintainer or anything

[blopker]

So I looked into it and while querySelectorAll is slower than getElementById, they both preform operations at order of millions per second. I don't know if performance is necessarily a concern here.

[dhh]

Dom IDs are supposed to be unique. I don't think we want to encourage straying from that. I'd have each button have their own ID, then just replace each with their own replace action.

[blopker]

Hey David, I do agree that the semantics around dom IDs are problematic here. I only bring this up since I do see UIs where there are identical/similar elements on the page that need updating in response to user action. Having to make the stream response aware of those elements add a bit of friction since all the other code to render the elements is identical.

If it's an issue to have multiple of the same IDs on the page for this case, then I'd like to propose a few alternative solutions.

1. Change the element's target attribute from id to something like data-stream-target. This way we lose the uniqueness semantics around IDs and it becomes more obvious that the attribute is being used for something and is not just an anchor. Or
2. Allow the stream's target attribute to be a CSS selector. This would allow the developer a little more freedom when selecting stream targets without having to change their HTML.

Both of these proposals would mean an API break, but both would allow a single stream response to update multiple elements without breaking the semantics of ID.

[dhh]

I think we could introduce a CSS selector style without API breaking by simply reserving the unadorned target to mean #target. Then if you do target=".message", we know that it's a class target. I like that idea and would support an extension for it 👍

[Audrius]

Hey, maybe this one could be off-topic, but it is a similar situation I'm trying to solve. I'm testing hotwire rails and I want to update by single action a couple of DOM elements in the page. Those elements could be placed far away from each other around the page and outside of the turbo frame. The way I did - I just placed two actions on update_move.turbo_stream.erb and it worked perfectly. Is this how it should work, right ? Thank you.

# ../update_move.turbo_stream.erb
<turbo-stream action="replace" target="player-to-move">
  <template>
    <div id="player-to-move">
      <strong>Player to move:</strong>
      <%= @board.player_to_move %>
    </div>
  </template>
</turbo-stream>

<turbo-stream action="replace" target="moves-list">
  <template>
    <div id="moves-list">
      <strong>Moves:</strong>
      <%= @board.moves %>
    </div>
  </template>
</turbo-stream>

[blopker]

OK! I've added the ability to use CSS selectors as a stream target. The implementation tries to not know anything about CSS selectors by just trying to find the target as if it were an ID, then only using the target as a CSS selector if no elements with that ID are found. This has a nice side effect of not breaking any ID targets that might already look like a CSS selector.

The implementation also continues not to allow multiple IDs unless specified as a CSS selector.

I've added a test as well. Please let me know if more coverage is desirable.

[blopker]

I've added this exception. I'm assuming not finding the target is a programming error, but maybe that's too strict?

[dhh]

Too strict, yeah. And a breaking change. You may well stream an update to something that's no longer there.

[blopker]

I'm happy to fix this up if it's still something we want.

[dhh]

Still like this. Let's shape it up.

[dhh]

Thanks for all the work on this @blopker. I ended up changing the API to avoid any potential conflicts. The existing target="dom_id" attribute is retained just for getElementById lookups, and then a new targets="css_selector" attribute is added as an alternative that'll flow through querySelectorAll.

[blopker]

I see, that's a good compromise. Glad this worked out.