Skip to content

Commit

Permalink
Convert DOM IDs before submitting ActionBroadcastJob
Browse files Browse the repository at this point in the history
When broadcasting to a target with a suffix, i.e.
```ruby
target: [active_record_instance, :some_attribute]
```

The array will get passed to `ActionView::RecordIdentifier.dom_id`
which turn it into something like:
`"active_record_instance_1234_some_attribute"`.

This works fine when broadcasting synchronously, but because array
gets serialized into a parameter for the `ActionBroadcastJob`, it
ends up forming an illegal target, something like:
`"#<ActiveRecordInstance:0x00000> some_attribute"`

This seems to include a memory address so it should be pretty much
guaranteed that it does not result in any predictable changes to the
DOM.

This behavior difference is a bit confusing and hard to debug.

This change generates the `target` names before serializing the job
parameters, to keep the behavior consistent between the regulater and
`later` methods.

Behavior for existing applications may or may not change, depending on
whether people expect this not to work.
  • Loading branch information
dirkjonker committed Jun 23, 2024
1 parent c57122c commit 1cb3d06
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 0 deletions.
2 changes: 2 additions & 0 deletions app/channels/turbo/streams/broadcasts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ def broadcast_action_later_to(*streamables, action:, target: nil, targets: nil,
streamables.compact_blank!

if streamables.present?
target = convert_to_turbo_stream_dom_id(target)
targets = convert_to_turbo_stream_dom_id(targets, include_selector: true)
Turbo::Streams::ActionBroadcastJob.perform_later \
stream_name_from(streamables), action: action, target: target, targets: targets, attributes: attributes, **rendering
end
Expand Down
30 changes: 30 additions & 0 deletions test/streams/streams_channel_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,36 @@ class Turbo::StreamsChannelTest < ActionCable::Channel::TestCase
end
end

test "broadcasting action later with ActiveModel array target" do
options = { partial: "messages/message", locals: { message: "hello!" } }

message = Message.new(id: 42)
target = [message, "opt"]
expected_target = "opt_message_42"

assert_broadcast_on "stream", turbo_stream_action_tag("prepend", target: expected_target, template: render(options)) do
perform_enqueued_jobs do
Turbo::StreamsChannel.broadcast_action_later_to \
"stream", action: "prepend", target: target, **options
end
end
end

test "broadcasting action later with multiple ActiveModel targets" do
options = { partial: "messages/message", locals: { message: "hello!" } }

one = Message.new(id: 1)
targets = [one, "messages"]
expected_targets = "#messages_message_1"

assert_broadcast_on "stream", turbo_stream_action_tag("prepend", targets: expected_targets, template: render(options)) do
perform_enqueued_jobs do
Turbo::StreamsChannel.broadcast_action_later_to \
"stream", action: "prepend", targets: targets, **options
end
end
end

test "broadcasting render now" do
assert_broadcast_on "stream", turbo_stream_action_tag("replace", target: "message_1", template: "Goodbye!") do
Turbo::StreamsChannel.broadcast_render_to "stream", partial: "messages/message"
Expand Down

0 comments on commit 1cb3d06

Please sign in to comment.