hotwired / turbo-rails

Use Turbo in your Ruby on Rails app
https://turbo.hotwired.dev
MIT License
2.13k stars 329 forks source link

Should broadcasts_to, action: :prepend target the dom_id on updates? #695

Closed justinpaulson closed 4 weeks ago

justinpaulson commented 1 month ago

I am using a line like this to update messages that are being generated by an LLM in a background job:

broadcasts_to ->(message) { "messages" }, action: :prepend

I've noticed (and I think this is intentional) that the create uses the pluralized model name as the target, and then the update uses the dom_id of the message itself as the target for the prepend action. Here are examples of the first two broadcasts received when a message is created and then updated:

When message is created:

<turbo-stream action="prepend" target="messages"><template><div id="message_123">\n  Hello \n</div></template></turbo-stream>

And when the message is updated:

<turbo-stream action="prepend" target="message_123"><template><div id="message_123">\n Hello there \n</div></template></turbo-stream>"

The first stream creates a div for the message like so:

<turbo-frame id="messages">
  <div id="message_123">
    Hello
  </div>
</div>

which looks great. But then the second stream does not accurately update the correct div, instead it creates a new one:

<turbo-frame id="messages">
  <div id="message_123">
    <div id="message_123">
      Hello there
    </div>
    Hello
  </div>
</div>

Subsequent calls will correctly find and update the child message_123 div, but the first prepend that targets message_123 will always create a new div.

I have noticed if I manually update the model to use prepend and target messages for both create and update on the model, then it properly updates the dom.

like so:

after_create_commit -> { broadcast_prepend_to "messages"}
after_update_commit -> { broadcast_prepend_to "messages"}

which will create the following two streams:

<turbo-stream action="prepend" target="messages"><template><div id="message_123" class="message assistant-message">\n  Hello \n</div></template></turbo-stream>
<turbo-stream action="prepend" target="messages"><template><div id="message_123" class="message assistant-message">\n  Hello there \n</div></template></turbo-stream>

I also see a test for this particular circumstance in the turbo code, and it seems to target the parent in the test. I wrote a test in the turbo codebase targeting the element itself in the same way that the rails code does, and it also duplicated the element with the id.

So my question, is this a bug? Is it a bug that should be fixed in the rails gem to use the parent as the target for prepend or should it be fixed in the turbo code to not create a new element when the target exists and it is what you are updating?

Thank you!

justinpaulson commented 1 month ago

Here is a gist using the bug report template. You will need to add the message partial as well.

https://gist.github.com/justinpaulson/2296102a22e043b180ecca7d635043ab

justinpaulson commented 4 weeks ago

Ah i see my issue, ignore me!

I was using action: :prepend instead of inserts_by: :prepend