Convert DOM IDs before submitting ActionBroadcastJob

[dirkjonker]

When broadcasting to a target with a suffix, i.e.

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.

[seanpdoyle]

Have you tried re-using the existing Message model declared elsewhere in the test suite? It's ActiveModel::Conversion-compliant:

diff --git a/test/streams/streams_channel_test.rb b/test/streams/streams_channel_test.rb
index f212266..5d47345 100644
--- a/test/streams/streams_channel_test.rb
+++ b/test/streams/streams_channel_test.rb
@@ -1,6 +1,5 @@
 require "test_helper"
 require "action_cable"
-require "active_model"
 
 class Turbo::StreamsChannelTest < ActionCable::Channel::TestCase
   include ActiveJob::TestHelper, Turbo::Streams::ActionHelper
@@ -194,15 +193,9 @@ class Turbo::StreamsChannelTest < ActionCable::Channel::TestCase
   test "broadcasting action later with ActiveModel array target" do
     options = { partial: "messages/message", locals: { message: "hello!" } }
 
-
-    class ::MyModel
-      include ActiveModel::Model
-      attr_accessor :id
-    end
-
-    my_model = MyModel.new(id: 42)
-    target = [my_model, "opt"]
-    expected_target = "opt_my_model_42"
+    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
@@ -215,15 +208,9 @@ class Turbo::StreamsChannelTest < ActionCable::Channel::TestCase
   test "broadcasting action later with multiple ActiveModel targets" do
     options = { partial: "messages/message", locals: { message: "hello!" } }
 
-
-    class ::MyModel
-      include ActiveModel::Model
-      attr_accessor :id
-    end
-
-    one = MyModel.new(id: 1)
+    one = Message.new(id: 1)
     targets = [one, "messages"]
-    expected_targets = "#messages_my_model_1"
+    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

[dirkjonker]

Ohhh thanks, that's nice, I wasn't looking hard enough :)

[brunoprietog]

Hello @dirkjonker, sorry for the lateness. Would you mind resolving the conflicts?

[dirkjonker]

@brunoprietog thanks for bringing this up again, I resolved the conflicts

[brunoprietog]

Thanks @dirkjonker!