Track refreshes per request and URL to support persistent modals with page refreshes

[davidalejandroaguilar]

Description

This PR enables Turbo to handle the common pattern of keeping modals/slideouts open while refreshing the page behind them without extra code needed. When updating content in a persistent modal, any broadcasts from the server will correctly refresh the stale background content, making modern UI patterns work out-of-the-box.

This eliminates the need for manual Turbo Stream updates when building slideouts, dialogs, and inline editors that need to stay open while updating both their own content and the page behind them.

i.e. it makes the happy path, even happier.

Details

Turbo is currently focused on modals/slideouts that close when you submit them, because it assumes you'll redirect to the current page, getting the freshest content.

Imagine you have a standard Rails controller action:

def update
  @conversation.update!(conversation_params)
  redirect_to conversations_path
end

And a Turbo frame that gets the conversation content when you click on one:

<a data-turbo-frame="modal" href="/conversations/1">Conversation 1</a>
<turbo-frame id="modal"></turbo-frame>

Redirecting to /conversations with standard body replacement

1.mov

Redirecting to /conversations with page refresh (morph)

2.mov

What happens though, if you want to leave the modal/slideout open, and want both the modal and the content behind it to be updated after you submit the form?

If you keep redirecting to the conversations_path, the modal will be "closed", because the incoming HTML has an empty Turbo frame, so the modal content will be replaced by empty HTML.

So you instead redirect to the conversation show path:

def update
  @conversation.update!(conversation_params)
  redirect_to conversation_path(@conversation)
end

This will keep the modal "open" and update its contents, but the content behind it will no longer be updated:

3.mov

It is at this point that you have to reach out to Turbo streams to manually update the content behind the modal, and to quote @jorgemanrubia's A happier happy path in Turbo with morphing:

"...that's the whole point here: not having to write those".

"See, your life gets more complex whenever you add partial updates to the mix. Now you have to care about screen regions, the elements they contain, and how interactions affect them. Good abstractions help, but you can’t shake the additional complexity off. You are just in a more complex realm.

This is why we say that Turbo is progressive: go with the happy Turbo Drive path by default — and deviate from it when you need higher fidelity for specific screens or interactions."

With this PR, the controller remains unchanged. Turbo is now smart enough to detect that a page refresh is needed and allows it to be processed if the server sends it, for example, with a model broadcasting page refreshes when updated with:

class Conversation < ApplicationRecord
  broadcasts_refreshes
end

4.mov

Note that since the page is refreshed, the refresh would "close" the modal by replacing or morphing the Turbo frame contents with the empty HTML from the conversations page. So we just add data-turbo-permanent to the modal:

<turbo-frame id="modal" data-turbo-permanent></turbo-fame>

Inline Rails script reproduction

Apart from the Turbo tests in this PR (which more deeply test things, such as not processing duplicate page refreshes or not processing them when it doesn't have to), I have prepared an inline Rails script that you can save to a file, e.g. rails.rb run via:

ruby rails.rb

The tests all pass with this PR.

If you want to run them against Turbo latest release, just comment/uncomment these lines, which will make tests fail:

// Original
-// import "https://unpkg.com/@hotwired/turbo@8.0.18/dist/turbo.es2017-esm.js"
+import "https://unpkg.com/@hotwired/turbo@8.0.18/dist/turbo.es2017-esm.js"
// PR:
-import "https://cdn.jsdelivr.net/gh/davidalejandroaguilar/turbo@track-refreshes-per-request-and-url-dist/dist/turbo.es2017-esm.js"
+// import "https://cdn.jsdelivr.net/gh/davidalejandroaguilar/turbo@track-refreshes-per-request-and-url-dist/dist/turbo.es2017-esm.js"

You can also comment out the tests and uncomment the "playground" test for you to test manually.

The inline Rails script is below:

Inline Rails script

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  gem "rails"
  gem "propshaft"
  gem "puma"
  gem "sqlite3"
  gem "turbo-rails"
  gem "solid_cable"

  gem "capybara"
  gem "cuprite", require: "capybara/cuprite"
end

ENV["DATABASE_URL"] = "sqlite3::memory:"
ENV["RAILS_ENV"] = "test"

require "active_model/railtie"
require "active_record/railtie"
require "action_controller/railtie"
require "action_view/railtie"
require "active_job/railtie"
require "action_cable/engine"
require "rails/test_unit/railtie"

class App < Rails::Application
  config.load_defaults Rails::VERSION::STRING.to_f

  config.root = __dir__
  config.hosts << "example.org"
  config.eager_load = false
  config.session_store :cookie_store, key: "cookie_store_key"
  config.secret_key_base = "secret_key_base"
  config.consider_all_requests_local = true
  config.action_cable.cable = {
    "adapter" => "solid_cable"
  }
  config.active_job.queue_adapter = :async

  Rails.logger = config.logger = Logger.new($stdout)

  routes.append do
    root to: "conversations#index"
    resources :conversations, only: [:index, :show] do
      member do
        put :update_and_redirect_to_conversation_show
        put :update_and_redirect_to_conversation_index
        put :update_and_redirect_to_root_path
        put :update_failing_and_rendering_show_with_unprocessable_entity
      end
    end
  end
end

Rails.application.initialize!

ActiveRecord::Schema.define do
  create_table :conversations, force: true do |t|
    t.text :name, null: false
  end
end

class Conversation < ActiveRecord::Base
  include Turbo::Broadcastable

  broadcasts_refreshes
end

class ApplicationController < ActionController::Base
  include Rails.application.routes.url_helpers

  def self.shared_head_template
    @shared_head_template ||= ERB.new(<<~HTML)
      <!--
      <meta name="turbo-refresh-method" content="morph">
      <meta name="turbo-refresh-scroll" content="preserve">
      -->
      <%= csrf_meta_tags %>
      <script type="module">
        // Original:
        // import "https://unpkg.com/@hotwired/turbo@8.0.18/dist/turbo.es2017-esm.js"
        // PR:
        import "https://cdn.jsdelivr.net/gh/davidalejandroaguilar/turbo@track-refreshes-per-request-and-url-dist-internal/dist/turbo.es2017-esm.js"
        import { createConsumer } from "https://unpkg.com/@rails/actioncable@7.2.0/app/assets/javascripts/actioncable.esm.js"

        const consumer = createConsumer("/cable")

        document.addEventListener("DOMContentLoaded", () => {
          document.querySelectorAll("turbo-cable-stream-source").forEach(element => {
            const channel = element.getAttribute("channel")
            const signedStreamName = element.getAttribute("signed-stream-name")

            if (channel && signedStreamName) {
              consumer.subscriptions.create(
                {
                  channel: channel,
                  signed_stream_name: signedStreamName
                },
                {
                  received(data) {
                    Turbo.renderStreamMessage(data)
                  }
                }
              )

              element.setAttribute("connected", "true")
            }
          })
        })
      </script>

      <style>
        form {
          border: 1px solid;
          margin: 15px;
          padding: 10px;
        }

        .modal {
          position: fixed;
          top: 0%;
          left: 50%;
          transform: translateX(-50%);
          background: white;
          border: 1px solid #ccc;
          box-shadow: 0 4px 8px rgba(0, 0, 0, 0.2);
          padding: 20px;
          z-index: 10;
          max-width: 500px;
          width: 100%;
          height: 50rem;
          overflow-y: scroll;
        }

        .conversation-item {
          margin-bottom: 50rem;
        }
      </style>
    HTML
  end

  helper_method :render_shared_head

  def render_shared_head
    self.class.shared_head_template.result(
      view_context.instance_eval { binding }
    ).html_safe
  end

  class_attribute :index_template, default: <<~'HTML'
    <!DOCTYPE html>
    <html>
      <head>
        <%= render_shared_head %>
      </head>

      <body>
        <h1>Conversations</h1>
        <ul>
          <% @conversations.reverse.each do |conversation| %>
            <%= turbo_stream_from conversation %>
            <li class="conversation-item">
              <%= link_to "Conversation from index #{conversation.name}", conversation_path(conversation), data: {"turbo-frame" => "modal"} %>
            </li>
          <% end %>
        </ul>

        <%= turbo_frame_tag "modal", data: {"turbo-permanent" => true} %>
      </body>
    </html>
  HTML

  class_attribute :show_template, default: <<~HTML
    <!DOCTYPE html>
    <html>
      <head>
        <%= render_shared_head %>
      </head>

      <body>
        <%= turbo_frame_tag "modal" do%>
          <div class="modal">
            <h1>Conversation from show: <%= @conversation.name %></h1>

            <h2>#1 Update conversation and redirect to conversation show inside modal frame</h2>
            <%= form_with model: @conversation, url: update_and_redirect_to_conversation_show_conversation_path(@conversation), method: :put, html: {id: "form-to-update-conversation-and-redirect-to-conversations-show-inside-modal-frame"} do |form| %>
              <%= form.text_field :name %>
              <%= form.submit "Update Conversation" %>

              <p>On the next page, a refesh should happen because only the content inside the Turbo frame will be updated and thus, the content outside it is stale. Only one refresh should be processed.</p>
              <p>This is the recommended approach is you want to update the conversation on both the modal and the conversations index.</p>
            <% end %>

            <h2>#2 Update conversation and redirect to conversation show outside modal frame</h2>
            <%= form_with model: @conversation, url: update_and_redirect_to_conversation_show_conversation_path(@conversation), method: :put, data: {"turbo-frame" => "_top"}, html: {id: "form-to-update-conversation-and-redirect-to-conversations-show-outside-modal-frame"} do |form| %>
              <%= form.text_field :name %>
              <%= form.submit "Update Conversation" %>

              <p>On the next page, a refesh should NOT happen because we're exiting the Turbo frame and thus the content is already fresh.</p>
              <p>This approach would take you to the conversation show page, outside the modal's Turbo frame.</p>
            <% end %>

            <h2>#3 Update conversation and redirect to conversation index inside modal frame</h2>
            <%= form_with model: @conversation, url: update_and_redirect_to_conversation_index_conversation_path(@conversation), method: :put, html: {id: "form-to-update-conversation-and-redirect-to-conversations-index-inside-modal-frame"} do |form| %>
              <%= form.text_field :name %>
              <%= form.submit "Update Conversation" %>

              <p>On the next page, a refresh should happen because, since we're still inside the Turbo frame, the incoming updated conversation's index page only has an empty Turbo frame and since the updated content is outside it, it will be discarded. Only one refresh should be processed.</p>
              <p>This approach would only update the conversations index, not the modal.</p>
              <p>It will also "close" the slideout/modal because the incoming HTML has an empty Turbo frame and we're not exiting the Turbo frame, so the slideout/modal content will be replaced by empty HTML.</p>
            <% end %>

            <h2>#4 Update conversation and redirect to conversation index outside modal frame</h2>
            <%= form_with model: @conversation, url: update_and_redirect_to_conversation_index_conversation_path(@conversation), method: :put, data: {"turbo-frame" => "_top"}, html: {id: "form-to-update-conversation-and-redirect-to-conversations-index-outside-modal-frame"} do |form| %>
              <%= form.text_field :name %>
              <%= form.submit "Update Conversation" %>

              <p>On the next page, a refresh should NOT happen, because we exited the Turbo frame and thus the content is already fresh.</p>
              <p>This approach would only update the conversations index, not the modal.</p>
              <p>It will not "close" the slideout/modal because we're exiting the Turbo frame, and even though the incoming HTML has an empty Turbo frame, the data-turbo-permanent attribute will prevent its content from being replaced by empty HTML.</p>
            <% end %>

            <h2>#5 Update conversation and redirect to root path inside modal frame</h2>
            <%= form_with model: @conversation, url: update_and_redirect_to_root_path_conversation_path(@conversation), method: :put, html: {id: "form-to-update-conversation-and-redirect-to-root-path-inside-modal-frame"} do |form| %>
              <%= form.text_field :name %>
              <%= form.submit "Update Conversation" %>
              <p>This is the same thing as #3</p>
            <% end %>

            <h2>#6 Update conversation and redirect to root path outside modal frame</h2>
            <%= form_with model: @conversation, url: update_and_redirect_to_root_path_conversation_path(@conversation), method: :put, data: {"turbo-frame" => "_top"}, html: {id: "form-to-update-conversation-and-redirect-to-root-path-outside-modal-frame"} do |form| %>
              <%= form.text_field :name %>
              <%= form.submit "Update Conversation" %>
              <p>This is the same thing as #4</p>
            <% end %>

            <h2>#7 Update conversation and fail and render show with unprocessable entity inside modal frame</h2>
            <%= form_with model: @conversation, url: update_failing_and_rendering_show_with_unprocessable_entity_conversation_path(@conversation), method: :put, html: {id: "form-to-update-conversation-and-fail-and-render-show-with-unprocessable-entity-inside-modal-frame"} do |form| %>
              <%= form.text_field :name %>
              <%= form.submit "Update Conversation" %>
              <p>This will fail and render the show template with a 422 status code. A page refresh should NOT happen because what was rendered from the server is already fresh. Refreshing the page would just get the same content again.</p>
            <% end %>

            <h2>#8 Update conversation and fail and render show with unprocessable entity outside modal frame</h2>
            <%= form_with model: @conversation, url: update_failing_and_rendering_show_with_unprocessable_entity_conversation_path(@conversation), method: :put, data: {turbo_frame: "_top"}, html: {id: "form-to-update-conversation-and-fail-and-render-show-with-unprocessable-entity-outside-modal-frame"} do |form| %>
              <%= form.text_field :name %>
              <%= form.submit "Update Conversation" %>
              <p>This will fail and render the show template with a 422 status code. A page refresh should NOT happen because what was rendered from the server is already fresh. Refreshing the page would just get the same content again.</p>
            <% end %>
          </div>
        <% end %>
      </body>
    </html>
  HTML
end

class ConversationsController < ApplicationController
  def index
    @conversations = Conversation.all
    render inline: index_template, formats: :html
  end

  def show
    @conversation = Conversation.find(params[:id])
    render inline: show_template, formats: :html
  end

  def update_and_redirect_to_conversation_show
    update_conversation!
    redirect_to conversation_path(@conversation)
    broadcast_extra_refreshes
  end

  def update_and_redirect_to_conversation_index
    update_conversation!
    redirect_to conversations_path
    broadcast_extra_refreshes
  end

  def update_and_redirect_to_root_path
    update_conversation!
    redirect_to root_path
    broadcast_extra_refreshes
  end

  def update_failing_and_rendering_show_with_unprocessable_entity
    @conversation = Conversation.find(params[:id])
    @conversation.attributes = conversation_params
    render inline: show_template, status: :unprocessable_entity, formats: :html
    broadcast_extra_refreshes
  end

  private

  def conversation_params
    params.require(:conversation).permit(:name)
  end

  def update_conversation!
    @conversation = Conversation.find(params[:id])
    @conversation.update!(conversation_params)
  end

  def broadcast_extra_refreshes
    # Apart from model's brodcasts_refreshes, we trigger these additional
    # refreshes. Turbo will only process one refresh per request.
    @conversation.broadcast_refresh
    @conversation.broadcast_refresh_later
  end
end

class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
  # driven_by :cuprite, using: :chrome, screen_size: [1400, 1400], options: {js_errors: true}
  driven_by :cuprite, using: :chrome, screen_size: [1400, 1400], options: {js_errors: true, headless: false}
end

Capybara.configure do |config|
  config.server = :puma, {Silent: true}
  config.default_normalize_ws = true
end

require "rails/test_help"

class TurboSystemTest < ApplicationSystemTestCase
  def setup
    3.times do |i|
      Conversation.create(name: "Conversation #{i + 1}")
    end

    visit root_path

    assert_text "Conversations"
    3.times do |i|
      assert_text "Conversation from index Conversation #{i + 1}"
    end

    click_on "Conversation from index Conversation 1"
    assert_text "Conversation from show: Conversation 1"
    assert_current_path root_path
    assert_modal_open
  end

  def teardown
    sleep ARGV.first.to_i
  end

  def assert_modal_closed
    assert_no_selector ".modal"
  end

  def assert_modal_open
    assert_selector ".modal"
  end

  test "example #1: From conversations Index, opening a Turbo frame slideout/modal to conversation Show, submitting a form inside the slideout/modal that redirects to conversations Show still inside the Turbo frame should refresh the page because only the content inside the Turbo frame was updated and thus, the content outside it is stale. Only one refresh should be processed." do
    within("#form-to-update-conversation-and-redirect-to-conversations-show-inside-modal-frame") do
      fill_in "conversation[name]", with: "Conversation 1 Updated"
      click_on "Update Conversation"
    end

    sleep 0.25
    assert_modal_open
    assert_text "Conversation from show: Conversation 1 Updated"
    assert_text "Conversation from index Conversation 1 Updated"
    assert_current_path root_path
  end

  test "example #2: From conversations Index, opening a Turbo frame slideout/modal to conversation Show, submitting a form inside the slideout/modal that redirects to conversations show outside the Turbo frame should not refresh the page because we're exiting the Turbo frame and thus the content is already fresh." do
    within("#form-to-update-conversation-and-redirect-to-conversations-show-outside-modal-frame") do
      fill_in "conversation[name]", with: "Conversation 1 Updated"
      click_on "Update Conversation"
    end

    sleep 0.25
    assert_text "Conversation from show: Conversation 1 Updated"
    assert_no_text "Conversation from index Conversation 1 Updated"
    assert_current_path conversation_path(Conversation.find_by(name: "Conversation 1 Updated"))
  end

  test "example #3: From conversations Index, opening a Turbo frame slideout/modal to conversation Show, submitting a form inside the slideout/modal that redirects to conversations Index still inside the Turbo frame should refresh the page because, since we're still inside the Turbo frame, the incoming updated conversation's index page only has an empty Turbo frame and since the updated content is outside it, it will be discarded. Only one refresh should be processed." do
    within("#form-to-update-conversation-and-redirect-to-conversations-index-inside-modal-frame") do
      fill_in "conversation[name]", with: "Conversation 1 Updated"
      click_on "Update Conversation"
    end

    sleep 0.25
    assert_modal_closed
    assert_no_text "Conversation from show: Conversation 1 Updated"
    assert_text "Conversation from index Conversation 1 Updated"
    assert_current_path root_path
  end

  test "example #4: From conversations Index, opening a Turbo frame slideout/modal to conversation Show, submitting a form inside the slideout/modal that redirects to conversations Index outside the Turbo frame should not refresh the page because we exited the Turbo frame and thus the content is already fresh." do
    within("#form-to-update-conversation-and-redirect-to-conversations-index-outside-modal-frame") do
      fill_in "conversation[name]", with: "Conversation 1 Updated"
      click_on "Update Conversation"
    end

    sleep 0.25
    assert_modal_open
    assert_text "Conversation from show: Conversation 1"
    assert_text "Conversation from index Conversation 1 Updated"
    assert_current_path conversations_path
  end

  test "example #5: From conversations Index, opening a Turbo frame slideout/modal to conversation Show, submitting a form inside the slideout/modal that redirects to root path still inside the Turbo frame should refresh the page because, since we're still inside the Turbo frame, the incoming updated conversation's index page only has an empty Turbo frame and since the updated content is outside it, it will be discarded. Only one refresh should be processed." do
    within("#form-to-update-conversation-and-redirect-to-root-path-inside-modal-frame") do
      fill_in "conversation[name]", with: "Conversation 1 Updated"
      click_on "Update Conversation"
    end

    assert_modal_closed
    assert_no_text "Conversation from show: Conversation 1 Updated"
    assert_text "Conversation from index Conversation 1 Updated"
    assert_current_path root_path
  end

  test "example #6: From conversations Index, opening a Turbo frame slideout/modal to conversation Show, submitting a form inside the slideout/modal that redirects to root path outside the Turbo frame should not refresh the page because we exited the Turbo frame and thus the content is already fresh." do
    within("#form-to-update-conversation-and-redirect-to-root-path-outside-modal-frame") do
      fill_in "conversation[name]", with: "Conversation 1 Updated"
      click_on "Update Conversation"
    end

    sleep 0.25
    assert_modal_open
    assert_text "Conversation from show: Conversation 1"
    assert_text "Conversation from index Conversation 1 Updated"
    assert_current_path root_path
  end

  test "example #7: From conversations Index, opening a Turbo frame slideout/modal to conversation Show, submitting a form inside the slideout/modal that fails and renders :show with unprocessable_entity inside modal frame should refresh the page because the content is stale. Submitting it correctly should refresh the page and update both the slideout/modal and the conversations index." do
    within("#form-to-update-conversation-and-fail-and-render-show-with-unprocessable-entity-inside-modal-frame") do
      fill_in "conversation[name]", with: "Conversation 1 Updated"
      click_on "Update Conversation"
    end

    sleep 0.25
    assert_modal_open
    assert_text "Conversation from show: Conversation 1 Updated"
    assert_no_text "Conversation from index Conversation 1 Updated"
    assert_text "Conversation from index Conversation 1"
    assert_current_path root_path

    # Submitting it correctly should refresh the page

    within("#form-to-update-conversation-and-redirect-to-conversations-show-inside-modal-frame") do
      fill_in "conversation[name]", with: "Conversation 1 Updated"
      click_on "Update Conversation"
    end

    sleep 0.25
    assert_modal_open
    assert_text "Conversation from show: Conversation 1 Updated"
    assert_text "Conversation from index Conversation 1 Updated"
    assert_current_path root_path
  end

  test "example #8: From conversations Index, opening a Turbo frame slideout/modal to conversation Show, submitting a form inside the slideout/modal that fails and renders :show with unprocessable_entity outside modal frame should refresh the page because the content is stale. Submitting it correctly should refresh the page, close the slideout/modal because we're exiting the Turbo frame, and update the conversations index." do
    within("#form-to-update-conversation-and-fail-and-render-show-with-unprocessable-entity-outside-modal-frame") do
      fill_in "conversation[name]", with: "Conversation 1 Updated"
      click_on "Update Conversation"
    end

    sleep 0.25
    assert_modal_open
    assert_text "Conversation from show: Conversation 1 Updated"
    assert_no_text "Conversation from index Conversation 1 Updated"
    assert_current_path root_path

    # Submitting it correctly should refresh the page

    within("#form-to-update-conversation-and-redirect-to-conversations-show-inside-modal-frame") do
      fill_in "conversation[name]", with: "Conversation 1 Updated"
      click_on "Update Conversation"
    end

    sleep 0.25
    assert_modal_closed
    assert_no_text "Conversation from show: Conversation 1 Updated"
    assert_text "Conversation from index Conversation 1 Updated"
    assert_current_path root_path
  end

  # test "playground" do
  #   sleep 500
  # end
end

How it works

Originally, Turbo only kept track of request ids.

Original approach

1. Submit a form
2. Generate a requestUID
3. Save it on a recentRequests variable
4. Attach it as a header to the request
5. Server obtains requestUID, Rails sets it in Turbo.current_request_id
6. Server redirects you
7. Server triggers broadcasts with the requestUID set
8. Turbo receives broadcasts
9. If requestID is in recent requests, ignore refresh

This worked for normal form submission that redirected to the same page, since the server just redirected you there, there's no need to process a refresh, you already have the freshest content:

Scenario A: Redirect outside the modal to close it and update things outside

1. User is in /conversations
2. Clicks a link to open /conversations/1
3. The content is placed in a data-turbo-permanent Turbo frame as a modal/slideout
4. Submits a form
5. Turbo generates requestUID "123"
6. The server redirects to /conversations
7. The modal/slideout "closes" (because /conversations has an empty Turbo frame)
8. The content in the /conversations page is fresh.
9. The server broadcasts one or many page refreshes with `requestUID "123"
10. Turbo discards them because it already has them on recentRequests

However, in the case of modals/slideouts within a Turbo frame that you want to keep open and update BOTH the modal content and the one behind the modal, this doesn't work:

Scenario B: Redirect to the modal to both update it and the content outside

1. User is in /conversations
2. Clicks a link to open /conversations/1
3. The content is placed in a data-turbo-permanent Turbo frame as a modal/slideout
4. Submits a form
5. Turbo generates requestUID "123"
6. The server redirects to /conversations/1
7. The modal/slideout stays "open" and the deal information within it gets updated and is fresh
8. The content behind it is stale.
9. The server broadcasts one or many page refreshes with requestUID "123"
10. Turbo discards them because it already has them on recentRequests

In order to fix this, we now keep track of request ids and URLs.

New approach

1. Submit a form
2. Generate a requestUID
3. Save it on a recentRequests variable
4. Attach it as a header to the request.
5. Server obtains requestUID, Rails sets it in Turbo.current_request_id
6. Server redirects you.
7. Turbo checks if response is a successful redirect not part of a Turbo frame
8. If so, it marks the request URL as refreshed for that requestID, this prevents processing refreshes for redirects that the browser will follow and thus content will be fresh
9. Turbo checks if the response is failed with status 422
10. If so, it marks the current URL as refreshed for that requestID, this prevents processing refreshes for failed requests, since whatever we have on screen or what was rendered from the server is the freshest content. Since the request failed, refreshing would just render the same content for anything else on screen (outside a modal, for example)
11. Server triggers broadcasts with the requestUID set
12. Turbo receives broadcasts
13. If requestID is in recent requests for the current URL, ignore refresh

This fixes the persistent modal scenario:

Scenario B: Redirect to the modal to both update it and the content outside

1. User is in /conversations
2. Clicks a link to open /conversations/1
3. The content is placed in a data-turbo-permanent Turbo frame as a modal/slideout
4. Submits a form
5. Turbo generates requestUID "123"
6. The server redirects to /conversations/1
7. The modal/slideout stays "open" and the deal information within it gets updated and is fresh
8. The content behind it is stale.
9. The server broadcasts one or many page refreshes with requestUID "123"
10. Turbo hasn't processed a successful refresh for requestUID "123" for URL /conversations, so it processes it
11. Any subsequent refreshes for requestID "123" and URL /conversations are discarded

The first "normal" scenario mentioned above still works:

Scenario A: Redirect outside the modal to close it and update things outside

1. User is in /conversations
2. Clicks a link to open /conversations/1
3. The content is placed in a data-turbo-permanent Turbo frame as a modal/slideout
4. Submits a form
5. Turbo generates requestUID "123"
6. The server redirects to /conversations
7. The modal/slideout "closes" (because /conversations has an empty Turbo frame)
8. The content in the /conversations page is fresh.
9. The server broadcasts one or many page refreshes with requestUID "123"
10. Turbo discards them because redirect was for /conversations, which the browser already followed and used for fresh content, and thus, requestUID "123" for URL /conversations was already marked as refreshed
11. Any subsequent refreshes for requestID "123" and URL /conversations are discarded

Here's a scenario for failed requests:

Scenario C: Failed request with 422 to render errors, then successful request

Previous approach

1. User is in /conversations
2. Clicks a link to open /conversations/1
3. The content is placed in a data-turbo-permanent Turbo frame as a modal/slideout
4. Submits a form
5. Turbo generates requestUID "123"
6. The request fails and server renders content for /conversations/1
7. The modal/slideout stays "open" and the deal information within it gets updated with form errors and is fresh
8. The content behind it is stale
9. The server broadcasts one or many page refreshes with requestUID "123"
10. Turbo discards page refresh for requestID "123"
11. Any subsequent refreshes for requestID "123" are discarded
12. User corrects errors and re-submits form
13. Turbo generates requestID "456"
14. The server redirects to /conversations/1
15. The modal/slideout stays "open" and the deal information within it gets updated and is fresh
16. The content behind it is stale.
17. The server broadcasts one or many page refreshes with requestUID "456"
18. Turbo discards them because it already has them on recentRequests

This leads to stale content behind the modal.

New approach

1. User is in /conversations
2. Clicks a link to open /conversations/1
3. The content is placed in a data-turbo-permanent Turbo frame as a modal/slideout
4. Submits a form
5. Turbo generates requestUID "123"
6. The request fails and server renders content for /conversations/1
7. The modal/slideout stays "open" and the deal information within it gets updated with form errors and is fresh
8. The content behind it is stale
9. The server broadcasts one or many page refreshes with requestUID "123"
10. Turbo discards page refresh for requestID "123" and URL /conversations because processing them would just render the same content
11. Any subsequent refreshes for requestID "123" and URL /conversations are discarded
12. User corrects errors and re-submits form
13. Turbo generates requestID "456"
14. The server redirects to /conversations/1
15. The modal/slideout stays "open" and the deal information within it gets updated and is fresh
16. The content behind it is stale
17. The server broadcasts one or many page refreshes with requestUID "456"
18. Turbo hasn't processed a successful refresh for requestUID "456" for URL /conversations, so it processes it
19. Any subsequent refreshes for requestID "456" and URL /conversations are discarded

The content on both the modal and behind it are fresh.

Note that this PR aims to NOT break existing behavior nor introduce unwanted additional page refreshes, but only progressively enhance the developer experience in a way that makes sense and it's expected:

e.g. "I'm redirecting to this modal to update it, and my model broadcasts refreshes, why is the page not being refreshed to update the content behind it?"

In order to do so, this PR adds test coverage for many different scenarios. It does so by introducing fixtures under src/tests/fixtures/conversations/ to be a seamless addition to the "messages" concept we use throughout, as well as making more sense to others, since it's an easier mental model to "open a modal for a conversation" than to "open modal for a message".

[davidalejandroaguilar]

Note that I went with the approach of awaiting the fetch here since it's something that we want to do for every request (like we were doing for generating the requestUID and appending it to the headers).

An alternative is to add it to each FetchRequest's requestSucceededWithResponse delegate method, but that'd make things more complex and cause us to potentially miss some cases as new delegates are added.

[davidalejandroaguilar]

Main change was:

-window.Turbo.session.recentRequests.add("123")
+window.Turbo.session.recentRequests.markUrlAsRefreshed("123", currentUrl)
  })

But I'm also doing a minor "refactor" to use the new assertPageRefresh function.

[davidalejandroaguilar]

For these 2 existing page refresh tests:

test("test action=refresh", async () => {
test("test action=refresh discarded when matching request id", async () => {

I did a minor "refactor" to use the new assertRefreshProcessed and triggerRefresh functions for clarity.

Added 4 additional ones.

[davidalejandroaguilar]

Existing /redirect added extraneous params to the redirected URL, such as enctype.

[brunoprietog]

Nice change! This is a known issue, with people removing the request ID to avoid this caution.

[brunoprietog]

I think it would be worth extracting this condition into a method. In any case, checking that the response is OK and is a redirect is not enough, since a page refresh can also occur when you receive 422 errors. It is not a redirect, but it does return to the same page. We are losing those.

[davidalejandroaguilar]

Thanks for taking the time to review @brunoprietog, excited to get this in!

I have extracted it to separate functions to avoid polluting the main fetch logic. Given the additional logic for 422, it's much better to have it in a separate place. We could potentially extract it to a separate class, but not sure if it warrants its own extraction yet.

Regarding 422 errors, we weren't losing them, but rather we were unnecessarily processing page refreshes for that failed request, so thanks for the heads up and good call!

Since the server couldn't process the request, and whatever it rendered back is the freshest content, processing a page refresh would just display the same content (outside the modal, if any, or in the page if not using a modal). Afterwards, successfully processing the request will get us a new requestUID and thus we'll now update both the modal and the content behind it as intended.

I have added test cases for this as well as updated the inline Rails script adding 2 new test cases, which both fail without this PR, because while the previous implementation correctly didn't process refreshes in this case, when successfully making a new request, the content behind the modal was still stale.

[brunoprietog]

We have to remove LimitedSet in that case, since this was the only place where it was used. And maybe move LimitedMap a bit further out? This feels like a primitive that is much less associated with something strictly related to Turbo Drive core. I know it was there before, but I personally would move it out.

[davidalejandroaguilar]

RecentRequestsTracker uses LimitedMap and LimitedSet internally to track the list of URLs for a given request. It's unlikely that a request will have more than 20 URLs, but it's rather defensive programming. Let me know if you think we should use a regular Set here.

I have moved both to src/util.js.

[davidalejandroaguilar]

The comments in the functions above make this look like a bigger change than it is. Wondering if I should just remove them since we don't usually add comments in the codebase like this.

[davidalejandroaguilar]

Hey @brunoprietog, wanted to see if there's anything blocking this. Hopefully you've given it a spin in a 37Signals app. I've been running it in prod for a while now!

Please let me know if there's anything else I can do to get this going 😺

[davidalejandroaguilar]

@brunoprietog Happy new year! Would you have any news about this?

On my side, I have 2 apps running since I wrote this (4 months ago):

• One with this change
• One without this change

The one without this change has to hack around with Turbo.current_request_id = nil and broadcast_refresh_later which doesn't work when multiple requests are done. So the use experience is rather yucky and you get the feeling of "oh it's a Turbo app, of course things aren't updated, you have to reload the page", which really undersells the Rails/Turbo front end DevEx.

The one with this change works flawlessly.

Hope you understand that it's a very high cognitive effort to come back to work on Turbo after a while, since it's rather a bit complex, tedious and time consuming to set up an environment where you can work on this locally with a real app, as well as having to load up everything in your mind again.

Let me know what you need to move this along the pipeline, happy to help in any way!