Incorrect page content when a Turbo "replace" stream action is followed by a refresh #1262

aprescott · 2024-05-22T15:37:25Z

I'll be using Rails along with turbo-rails helpers to describe the problem here but I don't think this is specific to that gem.

Reproduced with:

Rails 7.1.3.2
turbo-rails 2.0.5 (which appears to vendor Turbo 8.0.4)

Description

If a Turbo stream action with action="replace" is received, followed by a refresh action, then the resulting page content after the refresh request can be "out of sync" if there are data-turbo-permanent elements present. "Out of sync" meaning that child elements don't align with their expected parents, causing sometimes subtle, hard-to-identify bugs.

The problem does not appear if there is no replace action and only a refresh is used.

For example, consider some HTML (ERB) like this:

<%= turbo_stream_from("foobar") %>
<div id="replace-target">initial replace target content</div>

<div class="abc">
  <div id="el-1">
    <div>el-1</div>

    <div id="perma-1" data-turbo-permanent>perma-1</div>
  </div>

  <div id="el-2">
    <div>el-2</div>

    <div id="perma-2" data-turbo-permanent>perma-2</div>
  </div>

  <div id="el-3">
    <div>el-3</div>

    <div id="perma-3" data-turbo-permanent>perma-3</div>
  </div>
</div>

The structure here is:

#replace-target

.abc
  #el-1
    #perma-1
  #el-2
    #perma-2
  #el-3
    #perma-3

Next, consider these steps in sequence:

A "replace" action targets #replace-target, replacing its content.
The server stops including #el-1 in responses. (For example, suppose this is a list of incomplete todo items, and the first item was completed.)
A refresh is received and processed, requesting new content from the server and updating the page.

The page structure I'd expect after the refresh is:

#replace-target

.abc
  #el-2
    #perma-2
  #el-3
    #perma-3

The actual page structure, however, is:

#replace-target

.abc
  #el-2
    #perma-1
  #el-3
    #perma-2

The #el-* elements are out of sync with their corresponding #perma-* elements.

This is quite surprising, and can lead to subtle bugs. For instance, if #perma-N contains a form element referencing record N but is displayed as associated with some other record (#el-X), then a user might submit the form thinking they're updating, in this case, say, the record for #el-3, but in reality they're updating the record for #el-2.

The way the bug manifests appears to depend on HTML structure, and a change in the placement of the "replace" target affects whether the bug appears.

If the replace action is never sent/received, then the page structure is as expected, which maybe indicates there's some kind of conflicting state somewhere.

If #replace-target is moved below .abc, then the desync doesn't happen and the resulting structure is as expected:

.abc
  #el-2
    #perma-2
  #el-3
    #perma-3

#replace-target

In other situations, the structure of .abc appears to affect the resulting desync. In a production app the desync was seen as changing from (el, perma) pairs from:

(1, 1)
(2, 2)
(3, 3)

To:

(2, 2)
(3, 2)

Instead of:

(2, 2)
(3, 3)

Reproduction

Here's a setup script to reproduce it with a fresh Rails app. A file in /tmp is used to control whether #el-1 is included or not, which is a little hacky but avoids needing to edit templates.

cd /tmp
ruby -v
# ruby 3.2.3 (2024-01-18 revision 52bb2ac0a6) [arm64-darwin23]
gem install rails -v '7.1.3.2'
rails _7.1.3.2_ new turbo_desync_bug
# uses turbo-rails 2.0.5
cd turbo_desync_bug

cat <<END_CONTROLLER > app/controllers/test_controller.rb
class TestController < ApplicationController
  def foo
    # Hacky way to conditionalize the content.
    @waiting_for_refresh = (File.read("/tmp/waiting-for-refresh").strip != "yes")

    render inline: "", layout: "application"
  end
end
END_CONTROLLER

cat <<END_LAYOUT > app/views/layouts/application.html.erb
<!DOCTYPE html>
<html>
  <head>
    <title>test</title>

    <%= turbo_refreshes_with(method: :morph, scroll: :preserve) %>
    <%= content_for :head %>

    <%= javascript_importmap_tags %>
  </head>

  <body>
    <%= turbo_stream_from("foobar") %>
    <div id="replace-target">initial replace target content</div>

    <div class="abc">
      <% if @waiting_for_refresh %>
        <div id="el-1">
          <div>el-1</div>

          <div data-turbo-permanent>perma-1</div>
        </div>
      <% end %>

      <div id="el-2">
        <div>el-2</div>

        <div data-turbo-permanent>perma-2</div>
      </div>

      <div id="el-3">
        <div>el-3</div>

        <div data-turbo-permanent>perma-3</div>
      </div>
    </div>
  </body>
</html>
END_LAYOUT

cat <<END_ROUTES > config/routes.rb
Rails.application.routes.draw do
  get "/foo" => "test#foo"
end
END_ROUTES

cat <<END_APPLICATION_JS > app/javascript/application.js
import "@hotwired/turbo-rails"
END_APPLICATION_JS

cat <<END_IMPORTMAP > config/importmap.rb
pin "application", preload: true
pin "@hotwired/turbo-rails", to: "turbo.min.js", preload: true
END_IMPORTMAP

mkdir db/migrate
cat <<END_MIGRATION > db/migrate/20240501000001_create_users.rb
class CreateUsers < ActiveRecord::Migration[7.1]
  def change
    create_table :users do |t|
      t.timestamps
    end
  end
end
END_MIGRATION

bundle exec rails db:migrate

cat <<END_MODEL > app/models/user.rb
class User < ActiveRecord::Base
end
END_MODEL

Now, create an empty toggle file and start the server:

touch /tmp/waiting-for-refresh
bundle exec rails s

In a separate shell/terminal, run bundle exec rails c:

# in `bundle exec rails c`:
user = User.last || User.create!

def set_waiting(bool)
  val = bool ? "yes" : "no"

  File.open("/tmp/waiting-for-refresh", "w") do |file|
    file.puts val
    file.fsync
  end
end

# Reset the state.
set_waiting(false)

Now visit http://localhost:3000/foo in the browser. The structure shown will be:

#replace-target

.abc
  #el-1
    #perma-1
  #el-2
    #perma-2
  #el-3
    #perma-3

Next, generate the replace action in the Rails console:

User.last.broadcast_replace_later_to(
  "foobar",
  target: "replace-target",
  html: "REPLACED"
); nil

Then tell the server to stop including #el-1:

# Mark the first element on the page as removed.
set_waiting(true)

Check the browser page, without refreshing the tab, to confirm the replace was processed, and that "REPLACED" appears.

Finally, send a Turbo refresh for the same page:

User.last.broadcast_refresh_later_to("foobar"); nil

The page structure will then be desync'd:

#replace-target

.abc
  #el-2
    #perma-1
  #el-3
    #perma-2

As mentioned above:

If you edit the layout file to place #replace-target after .abc, the bug won't appear.
If broadcast_replace_later_to is never used, then the bug won't appear.

The text was updated successfully, but these errors were encountered:

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Incorrect page content when a Turbo "replace" stream action is followed by a refresh #1262

Incorrect page content when a Turbo "replace" stream action is followed by a refresh #1262

aprescott commented May 22, 2024

Incorrect page content when a Turbo "replace" stream action is followed by a refresh #1262

Incorrect page content when a Turbo "replace" stream action is followed by a refresh #1262

Comments

aprescott commented May 22, 2024

Description

Reproduction