Ensure turbo-stream[action="remove"] does not render

Closes [#679][]

Prior to this change, calls to
`Turbo::Streams::Broadcasts#broadcast_remove_to` would render a
view partial that corresponds to the model *regardless* of whether or
not the contents of that render would be included in the broadcast
`<turbo-stream>` element.

Not only is it wasteful to render HTML and then do nothing with it, it
also poses the risk of failing due to errors raised during the rendering
process.

This commit restructures the contents of the
`Turbo::Streams::Broadcasts#broadcast_action_to` method to skip
extraneous rendering if `render: false` is passed. In addition, it
ensures that calls to `broadcast_remove_to` pass `render: false` by
default.

[#679]: #679

app/channels/turbo/streams/broadcasts.rb

@@ -6,7 +6,7 @@ module Turbo::Streams::Broadcasts
   include Turbo::Streams::ActionHelper
 
   def broadcast_remove_to(*streamables, **opts)
-    broadcast_action_to(*streamables, action: :remove, **opts)
+    broadcast_action_to(*streamables, action: :remove, render: false, **opts)
   end
 
   def broadcast_replace_to(*streamables, **opts)
@@ -38,10 +38,18 @@ def broadcast_refresh_to(*streamables, **opts)
   end
 
   def broadcast_action_to(*streamables, action:, target: nil, targets: nil, attributes: {}, **rendering)
-    broadcast_stream_to(*streamables, content: turbo_stream_action_tag(action, target: target, targets: targets, template:
-      rendering.delete(:content) || rendering.delete(:html) || (rendering[:render] != false && rendering.any? ? render_format(:html, **rendering) : nil),
-      **attributes
-    ))
+    content = rendering.delete(:content)
+    html = rendering.delete(:html)
+    render = rendering.delete(:render)
+
+    template =
+      if render == false
+        nil
+      else
+        content || html || (render_format(:html, **rendering) if rendering.present?)
+      end
+
+    broadcast_stream_to(*streamables, content: turbo_stream_action_tag(action, target: target, targets: targets, template: template, **attributes))
   end
 
   def broadcast_replace_later_to(*streamables, **opts)

test/dummy/app/views/messages/_raises_error.html.erb

@@ -0,0 +1 @@
+<%= raise "Failed to render" %>

test/streams/broadcastable_test.rb

@@ -5,6 +5,12 @@
 class Turbo::BroadcastableTest < ActionCable::Channel::TestCase
   include ActiveJob::TestHelper, Turbo::Streams::ActionHelper
 
+  class MessageThatRendersError < Message
+    def to_partial_path
+      "messages/raises_error"
+    end
+  end
+
   setup { @message = Message.new(id: 1, content: "Hello!") }
 
   test "broadcasting ignores blank streamables" do
@@ -37,6 +43,14 @@ class Turbo::BroadcastableTest < ActionCable::Channel::TestCase
     end
   end
 
+  test "broadcasting remove does not render contents" do
+    message = MessageThatRendersError.new(id: 1)
+
+    assert_broadcast_on message.to_gid_param, turbo_stream_action_tag("remove", target: dom_id(message)) do
+      message.broadcast_remove
+    end
+  end
+
   test "broadcasting replace to stream now" do
     assert_broadcast_on "stream", turbo_stream_action_tag("replace", target: "message_1", template: render(@message)) do
       @message.broadcast_replace_to "stream"
@@ -127,6 +141,14 @@ class Turbo::BroadcastableTest < ActionCable::Channel::TestCase
     end
   end
 
+  test "broadcasting refresh does not render contents" do
+    message = MessageThatRendersError.new(id: 1)
+
+    assert_broadcast_on message.to_gid_param, turbo_stream_action_tag("refresh") do
+      message.broadcast_refresh
+    end
+  end
+
   test "broadcasting refresh later is debounced" do
     assert_broadcast_on @message.to_gid_param, turbo_stream_refresh_tag do
       assert_broadcasts(@message.to_gid_param, 1) do