hotwired / turbo

The speed of a single-page web application without having to write any JavaScript
https://turbo.hotwired.dev
MIT License
6.74k stars 430 forks source link

Input field missing from form with errors, even though it is in the HTML response #1291

Open pedantic-git opened 3 months ago

pedantic-git commented 3 months ago

Hi Turbo developers!

I'm experiencing a weird issue I don't know how to debug. You can see it for yourself here: https://devvelo.com/users/sign_up

If you get any error when submitting the form (for example, a password too short or confirmation doesn't match password), the form is re-rendered with the error message but the input itself is removed from the DOM. If you inspect the response, you can see the input is returned as part of the 422 response, but Turbo chooses not to render it / to remove it.

Disabling Turbo on the form causes it to behave as expected.

This is a standard Rails FormBuilder field, so it is wrapped with a .field_with_errors in the response - might this have something to do with it? Have I made some silly mistake? What tools would you use to debug this?

Thanks so much in advance for your help!

4lllex commented 2 months ago

I think you just have a browser plugin that interfering with morphing. For example, if I disable LastPass your page works fine. Here is a reproduction test of what I think is happening:

require "bundler/inline"

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

  gem "rails"
  gem "propshaft"
  gem "puma"
  gem "sqlite3", "~> 1.4"
  gem "turbo-rails"

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

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

require "active_record/railtie"
# require "active_storage/engine"
require "action_controller/railtie"
require "action_view/railtie"
# require "action_mailer/railtie"
# require "active_job/railtie"
require "action_cable/engine"
# require "action_mailbox/engine"
# require "action_text/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" => "async"}
  config.turbo.draw_routes = false

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

  routes.append do
    root to: "application#index"
    post "/", to: "application#create"
  end
end

Rails.application.initialize!

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

  def index
    render inline: <<~TEMPLATE, formats: :html
      <html>
        <head>
          <meta name="turbo-refresh-method" content="morph">
          <%= csrf_meta_tags %>
          <script type="importmap" data-turbo-track="reload">
            { "imports": { "@hotwired/turbo-rails": "<%= asset_path("turbo.js") %>" } }
          </script>
          <script type="module">
            import "@hotwired/turbo-rails"
          </script>
        </head>
        <body>

          <form action="/" accept-charset="UTF-8" method="post">
            <label for="user_password">Password</label><div>
              <input type="password" name="user[password]" id="user_password"/>
              <div class="shadowroot"></div>
            </div>
            <input type="submit" name="commit" value="Submit"/>
          </form>

          <script charset="utf-8">
            const shadow = document.querySelector(".shadowroot").attachShadow({mode: "open"});
            const observer = new MutationObserver((mutationList) => {
              mutationList.forEach((mutation) => {
                mutation.removedNodes.forEach((node) => {
                  if (node.id === "user_password") {
                    shadow.host.remove();
                  }
                });
              });
            });
            observer.observe(document.querySelector("form"), {subtree: true, childList: true})
          </script>

        </body>
      </html>
    TEMPLATE
  end

  def create
    render inline: <<~TEMPLATE, formats: :html, status: 422
      <html>
        <head>
          <meta name="turbo-refresh-method" content="morph">
          <%= csrf_meta_tags %>
          <script type="importmap" data-turbo-track="reload">
            { "imports": { "@hotwired/turbo-rails": "<%= asset_path("turbo.js") %>" } }
          </script>
          <script type="module">
            import "@hotwired/turbo-rails"
          </script>
        </head>
        <body>

          <form action="/" accept-charset="UTF-8" method="post">
            <div class="field_with_errors">
              <label for="user_password">Password</label>
            </div>
            <div>
              <div class="field_with_errors">
                <input type="password" name="user[password]" id="user_password"/>
              </div>
            </div>
            <input type="submit" name="commit" value="Submit"/>
          </form>

        </body>
      </html>
    TEMPLATE
  end
end

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

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

require "rails/test_help"

class TurboSystemTest < ApplicationSystemTestCase
  test "a bug" do
    visit "/"
    click_button "Submit"

    assert_selector "#user_password"
  end
end

A few things to note:

pedantic-git commented 2 months ago

This is so helpful thanks! I had noticed disabling the Lastpass extension "fixed" it but didn't know how to proceed from there. I'm using Phlex as my view templates and they have a built-in mechanism to add spacing so I'll try that!

Presumably this is still a bug in turbo or idiomorph? Or is it the Lastpass extension that has the bug?

4lllex commented 2 months ago

considering that result depends on a single insignificant whitespace between tags, it looks like an idiomorph bug to me.

pedantic-git commented 2 months ago

@4lllex This worked! I literally added a line that said:

whitespace # https://github.com/hotwired/turbo/issues/1291#issuecomment-2319513075

to the label method in my Forms::InputGroupComponent and that worked around the issue. I'll re-raise this with idiomorph and refer back to this. Thank you so much for the detailed reproduction steps!