Alter Action Cable Element to be morph-compatible (#650)

Closes [#638][] Recent changes to integrate with morphing have altered the mental model for some Turbo custom elements, including the `<turbo-cable-stream-source>` element. Custom Elements' `connectedCallback()` and `disconnectedCallback()` (along with Stimulus' `connect()` and `disconnect()`) improved upon invoking code immediately, or listening for `DOMContentLoaded` events. There are similar improvements to be made to integrate with morphing. First, [observe attribute changes][] by declaring their own `static observedAttributes` properties along with `attributeChangedCallback(name, oldValue, newValue)` callbacks. Those callbacks execute the same initialization code as their current `connectedCallback()` and `disconnectedCallback()` methods. That'll help resolve this issue. In addition to those changes, it's important to emphasize this pattern for consumer applications moving forward. JavaScript code (whether Stimulus controller or otherwise) should be implemented in a way that' resilient to both asynchronous connection and disconnection *as well as* asynchronous modification of attributes. [#638]: #638 [observe attribute changes]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Components/Using_custom_elements#responding_to_attribute_changes
hotwired · Sep 17, 2024 · 034c632 · 034c632
1 parent 9f5c846
commit 034c632
Show file tree

Hide file tree

Showing 11 changed files with 62 additions and 15 deletions.
diff --git a/app/assets/javascripts/turbo.js b/app/assets/javascripts/turbo.js
@@ -5330,6 +5330,7 @@ function walk(obj) {
 }
 
 class TurboCableStreamSourceElement extends HTMLElement {
+  static observedAttributes=[ "channel", "signed-stream-name" ];
   async connectedCallback() {
     connectStreamSource(this);
     this.subscription = await subscribeTo(this.channel, {
@@ -5341,6 +5342,13 @@ class TurboCableStreamSourceElement extends HTMLElement {
   disconnectedCallback() {
     disconnectStreamSource(this);
     if (this.subscription) this.subscription.unsubscribe();
+    this.subscriptionDisconnected();
+  }
+  attributeChangedCallback() {
+    if (this.subscription) {
+      this.disconnectedCallback();
+      this.connectedCallback();
+    }
   }
   dispatchMessageEvent(data) {
     const event = new MessageEvent("message", {

diff --git a/app/assets/javascripts/turbo.min.js b/app/assets/javascripts/turbo.min.js
diff --git a/app/assets/javascripts/turbo.min.js.map b/app/assets/javascripts/turbo.min.js.map
diff --git a/app/javascript/turbo/cable_stream_source_element.js b/app/javascript/turbo/cable_stream_source_element.js
@@ -3,6 +3,8 @@ import { subscribeTo } from "./cable"
 import snakeize from "./snakeize"
 
 class TurboCableStreamSourceElement extends HTMLElement {
+  static observedAttributes = ["channel", "signed-stream-name"]
+
   async connectedCallback() {
     connectStreamSource(this)
     this.subscription = await subscribeTo(this.channel, {
@@ -15,6 +17,14 @@ class TurboCableStreamSourceElement extends HTMLElement {
   disconnectedCallback() {
     disconnectStreamSource(this)
     if (this.subscription) this.subscription.unsubscribe()
+    this.subscriptionDisconnected()
+  }
+
+  attributeChangedCallback() {
+    if (this.subscription) {
+      this.disconnectedCallback()
+      this.connectedCallback()
+    }
   }
 
   dispatchMessageEvent(data) {

diff --git a/test/dummy/app/controllers/messages_controller.rb b/test/dummy/app/controllers/messages_controller.rb
@@ -1,6 +1,10 @@
 class MessagesController < ApplicationController
   def show
-    @message = Message.new(id: 1, content: "My message")
+    @message = Message.find(params[:id])
+
+    if (other_id = params[:other_message])
+      @other_message = Message.find(other_id)
+    end
   end
 
   def index

diff --git a/test/dummy/app/views/messages/show.html.erb b/test/dummy/app/views/messages/show.html.erb
@@ -1,3 +1,7 @@
 <h1>Message #<%= @message.id %></h1>
 
 <%= turbo_stream_from @message %>
+
+<div id="<%= dom_id @message %>">
+  <%= render @message %>
+</div>
diff --git a/test/dummy/app/views/messages/show.turbo_stream.erb b/test/dummy/app/views/messages/show.turbo_stream.erb
@@ -1,11 +1,11 @@
 <%= turbo_stream.remove @message %>
 <%= turbo_stream.replace @message %>
 <%= turbo_stream.replace @message, "Something else" %>
-<%= turbo_stream.replace "message_5", "Something fifth" %>
-<%= turbo_stream.replace "message_5", "Something fifth", method: :morph %>
-<%= turbo_stream.update "message_5", "Something fifth", method: :morph %>
-<%= turbo_stream.replace "message_5", partial: "messages/message", locals: { message: Message.new(id: 5, content: "OLLA!") } %>
+<%= turbo_stream.replace @other_message, "Something fifth" %>
+<%= turbo_stream.replace @other_message, "Something fifth", method: :morph %>
+<%= turbo_stream.update @other_message, "Something fifth", method: :morph %>
+<%= turbo_stream.replace @other_message, partial: "messages/message", locals: { message: @other_message } %>
 <%= turbo_stream.append "messages", @message %>
-<%= turbo_stream.append "messages", partial: "messages/message", locals: { message: Message.new(id: 5, content: "OLLA!") } %>
+<%= turbo_stream.append "messages", partial: "messages/message", locals: { message: @other_message } %>
 <%= turbo_stream.prepend "messages", @message %>
-<%= turbo_stream.prepend "messages", partial: "messages/message", locals: { message: Message.new(id: 5, content: "OLLA!") } %>
+<%= turbo_stream.prepend "messages", partial: "messages/message", locals: { message: @other_message } %>
diff --git a/test/dummy/config/environments/test.rb b/test/dummy/config/environments/test.rb
@@ -25,7 +25,7 @@
   config.action_controller.perform_caching = false
 
   # Raise exceptions instead of rendering exception templates.
-  config.action_dispatch.show_exceptions = false
+  config.action_dispatch.show_exceptions = :none
 
   # Disable request forgery protection in test environment.
   config.action_controller.allow_forgery_protection = false

diff --git a/test/streams/streams_controller_test.rb b/test/streams/streams_controller_test.rb
@@ -13,10 +13,9 @@ class Turbo::StreamsControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "show all turbo actions" do
-    message_1 = Message.new(id: 1, content: "My message")
-    message_5 = Message.new(id: 5, content: "OLLA!")
+    message_1, message_5 = Message.create!([{ id: 1, content: "My message" }, { id: 5, content: "OLLA!" }])
 
-    get message_path(id: 1), as: :turbo_stream
+    get message_path(message_1, other_message: message_5), as: :turbo_stream
 
     assert_dom_equal <<~HTML, @response.body
       <turbo-stream action="remove" target="message_1"></turbo-stream>

diff --git a/test/system/assertions_test.rb b/test/system/assertions_test.rb
@@ -2,7 +2,7 @@
 
 class AssertionsTest < ApplicationSystemTestCase
   test "#assert_turbo_cable_stream_source treats the locator as :signed_stream_name filter" do
-    message = Message.new(id: 1)
+    message = Message.create!
 
     visit message_path(message)
 
@@ -22,7 +22,7 @@ class AssertionsTest < ApplicationSystemTestCase
   end
 
   test "#assert_turbo_cable_stream_source supports record filters" do
-    message = Message.new(id: 1)
+    message = Message.create!
 
     visit message_path(message)
 

diff --git a/test/system/broadcasts_test.rb b/test/system/broadcasts_test.rb
@@ -2,6 +2,7 @@
 
 class BroadcastsTest < ApplicationSystemTestCase
   include ActiveJob::TestHelper
+  extend Turbo::Streams::StreamName
 
   test "Message broadcasts Turbo Streams" do
     visit messages_path
@@ -106,8 +107,29 @@ class BroadcastsTest < ApplicationSystemTestCase
     end
   end
 
+  test "the turbo-cable-stream-source observes attribute changes" do
+    original, broadcasted = Message.create! [{ content: "original content" }, { content: "broadcasted content" }]
+
+    visit message_path(original)
+    reconnect_cable_stream_source from: original, to: broadcasted
+
+    assert_text original.content
+    assert_broadcasts_text broadcasted.content, to: dom_id(original) do
+      broadcasted.broadcast_update target: dom_id(original)
+    end
+    assert_no_text original.content
+  end
+
   private
 
+  def reconnect_cable_stream_source(from:, to:)
+    cable_stream_source = find("turbo-cable-stream-source[signed-stream-name=#{signed_stream_name(from)}]")
+
+    cable_stream_source.execute_script <<~JS, signed_stream_name(to)
+      this.setAttribute("signed-stream-name", arguments[0])
+    JS
+  end
+
   def assert_broadcasts_text(text, to:, &block)
     within(:element, id: to) { assert_no_text text }