e2e tests

test/e2e/support/issues/issue_3647.ex

@@ -0,0 +1,186 @@
+defmodule Phoenix.LiveViewTest.E2E.Issue3647Live do
+  # https://github.com/phoenixframework/phoenix_live_view/issues/3647
+  #
+  # The above issue was caused by LV uploads relying on DOM attributes like
+  # data-phx-active-refs="1,2,3" being in the DOM to track uploads. The problem
+  # arises when the upload input is inside a form that is locked due to another,
+  # unrelated change. The following would happen:
+  #
+  # 1. User clicks on a button to upload a file
+  # 2. A hook calls this.uploadTo(), which triggers a validate event and locks the form
+  # 3. The hook also changes another input in ANOTHER form, which also triggers a separate validate
+  #    event and locks the form
+  # 4. The first validate completes, but the attributes are patched to the clone of the form,
+  #    the real DOM does not contain it.
+  # 5. LiveView tries to start uploading, but does not find any active files.
+  #
+  # This case is special in that the upload input belongs to a separate form (<input form="form-id">),
+  # so it's not the upload input's form that is locked.
+  #
+  # The fix for this is to only try to upload when the closest locked element starting from
+  # the upload input is unlocked.
+  #
+  # There was a separate problem though: LiveView relied on a separate DOM patching mechanism
+  # when patching cloned trees that did not fully share the same logic as the default DOMPatch.
+  # In this case, it did not merge data-attributes on elements that are ignored (phx-update="ignore" / data-phx-update="ignore"),
+  # therefore, the first fix alone would not work.
+  # Now, we use the same patching logic for regular DOM patches and element unlocks.
+  #
+  # This difference in DOM patching logic also caused other issues, notably:
+  #   * https://github.com/phoenixframework/phoenix_live_view/issues/3591
+  #   * https://github.com/phoenixframework/phoenix_live_view/issues/3651
+  use Phoenix.LiveView
+
+  defmodule User do
+    import Ecto.Changeset
+    use Ecto.Schema
+
+    schema "users" do
+      field(:name)
+    end
+
+    def change_user(user, params \\ %{}) do
+      user |> cast(params, [:name])
+    end
+  end
+
+  def render("live.html", assigns) do
+    ~H"""
+    <meta name="csrf-token" content={Plug.CSRFProtection.get_csrf_token()} />
+    <script src="/assets/phoenix/phoenix.min.js">
+    </script>
+    <script type="module">
+      import {LiveSocket} from "/assets/phoenix_live_view/phoenix_live_view.esm.js"
+
+      let csrfToken = document.querySelector("meta[name='csrf-token']").getAttribute("content");
+      let liveSocket = new LiveSocket("/live", window.Phoenix.Socket, {params: {_csrf_token: csrfToken}, hooks: {
+        JsUpload: {
+          mounted() {
+            this.el.addEventListener("click", () => {
+              const fillBefore = "before" in this.el.dataset
+              if (fillBefore) this.fill_input()
+              this.js_upload()
+              if (!fillBefore) this.fill_input()
+            })
+          },
+
+          js_upload() {
+            const content = "x".repeat(1024).repeat(1024)
+            const file = new File([content], "1mb_of_x.txt", { type: "text/plain" })
+            const input = document.querySelector("input[type=file]")
+            this.uploadTo(input.form, input.name, [file])
+          },
+
+          fill_input() {
+            const input = document.querySelector("input[type=text]")
+            input.value = input.value + input.value.length
+            const event = new Event("input", { bubbles: true })
+            input.dispatchEvent(event)
+          }
+        }
+      }})
+      liveSocket.connect()
+      window.liveSocket = liveSocket
+    </script>
+    <style>
+      * { font-size: 1.1em; }
+    </style>
+
+    <main>{@inner_content}</main>
+    """
+  end
+
+  @impl Phoenix.LiveView
+  def mount(_params, _session, socket) do
+    {:ok,
+     socket
+     |> assign(form: to_form(User.change_user(%User{})))
+     |> assign(:uploaded_files, [])
+     |> allow_upload(:avatar,
+       accept: ~w(.txt .md),
+       max_entries: 2,
+       auto_upload: true,
+       progress: &handle_progress/3
+     ), layout: {__MODULE__, :live}}
+  end
+
+  # with auto_upload: true we can consume files here
+  defp handle_progress(:avatar, entry, socket) do
+    if entry.done? do
+      uuid =
+        consume_uploaded_entry(socket, entry, fn _meta ->
+          {:ok, entry.uuid}
+        end)
+
+      {:noreply, update(socket, :uploaded_files, &[uuid | &1])}
+    else
+      {:noreply, socket}
+    end
+  end
+
+  @impl Phoenix.LiveView
+  def handle_params(_params, _uri, socket) do
+    {:noreply, socket}
+  end
+
+  @impl Phoenix.LiveView
+  def handle_event("validate-user", %{"user" => params}, socket) do
+    form =
+      %User{}
+      |> User.change_user(params)
+      |> to_form(action: :validate)
+
+    {:noreply, assign(socket, form: form)}
+  end
+
+  def handle_event("validate", _params, socket) do
+    {:noreply, socket}
+  end
+
+  @impl Phoenix.LiveView
+  def handle_event("cancel-upload", %{"ref" => ref}, socket) do
+    {:noreply, cancel_upload(socket, :avatar, ref)}
+  end
+
+  @impl Phoenix.LiveView
+  def render(assigns) do
+    ~H"""
+    <.form for={@form} phx-change="validate-user" id="user-form">
+      <input id={@form[:name].id} name={@form[:name].name} value={@form[:name].value} type="text" />
+      <button id="x" type="button" phx-hook="JsUpload">
+        Upload then Input
+      </button>
+      <button id="y" type="button" phx-hook="JsUpload" data-before>
+        Input then Upload
+      </button>
+      <.live_file_input upload={@uploads.avatar} form="auto-form" />
+    </.form>
+
+    <form id="auto-form" phx-change="validate"></form>
+    <section class="pending-uploads" phx-drop-target={@uploads.avatar.ref} style="min-height: 100%;">
+      <h3>Pending Uploads ({length(@uploads.avatar.entries)})</h3>
+
+      <%= for entry <- @uploads[:avatar].entries do %>
+        <div>
+          <progress value={entry.progress} max="100">{entry.progress}%</progress>
+          <div>
+            {entry.uuid}<br />
+            <a
+              href="#"
+              phx-click="cancel-upload"
+              phx-value-ref={entry.ref}
+              class="upload-entry__cancel"
+            >
+              Cancel Upload
+            </a>
+          </div>
+        </div>
+      <% end %>
+    </section>
+
+    <ul>
+      <li :for={file <- @uploaded_files}><a href={file}>{Path.basename(file)}</a></li>
+    </ul>
+    """
+  end
+end

test/e2e/support/issues/issue_3651.ex

@@ -0,0 +1,103 @@
+defmodule Phoenix.LiveViewTest.E2E.Issue3651Live do
+  # https://github.com/phoenixframework/phoenix_live_view/issues/3651
+  use Phoenix.LiveView
+
+  def mount(_params, _session, socket) do
+    if connected?(socket) do
+      send(self(), :change_id)
+    end
+
+    # assigns for pre_script
+    assigns = %{}
+
+    socket =
+      socket
+      |> assign(id: 1, counter: 0)
+      |> assign(
+        :pre_script,
+        ~H"""
+        <script>
+          window.hooks.OuterHook = {
+            mounted() {
+              this.pushEvent("lol")
+            },
+          }
+          window.hooks.InnerHook = {
+            mounted() {
+              console.log("MOUNTED", this.el);
+              this.handleEvent('myevent', this._handleEvent(this));
+            },
+            destroyed() {
+              document.getElementById("notice").innerHTML = "";
+              console.log("DESTROYED", this.el);
+            },
+            _handleEvent(self) {
+              return () => {
+                setTimeout(() => {
+                  console.warn("reloading", self.el);
+                  self.pushEvent("reload", {})
+                }, 50)
+              }
+            }
+          }
+        </script>
+        """
+      )
+      |> push_event("myevent", %{})
+
+    {:ok, socket}
+  end
+
+  def handle_info(:change_id, socket) do
+    {:noreply, assign(socket, id: 2)}
+  end
+
+  def handle_event("lol", _params, socket) do
+    {:noreply, socket}
+  end
+
+  def handle_event("reload", _params, socket) do
+    counter = socket.assigns.counter + 1
+
+    socket =
+      socket
+      |> push_event("myevent", %{})
+      |> assign(counter: counter)
+
+    socket =
+      if counter > 4096 do
+        raise "that's enough, bye!"
+      else
+        socket
+      end
+
+    {:noreply, socket}
+  end
+
+  def render(assigns) do
+    ~H"""
+    <div id="main" phx-hook="OuterHook">
+      <div phx-hook="InnerHook" id={"id-#{@id}"} />
+      This is an example of nested hooks resulting in a "ghost" element
+      that isn't on the DOM, and is never cleaned up. In this specific example
+      a timeout is used to show how the number of events being sent to the server
+      grows exponentially.
+      <p>Doing any of the following things fixes it:</p>
+      <ol>
+        <li>Setting the `phx-hook` to use a fixed id.</li>
+        <li>Removing the `pushEvent` from the OuterHook `mounted` callback.</li>
+        <li>Deferring the pushEvent by wrapping it in a setTimeout.</li>
+      </ol>
+    </div>
+    <div>
+      To prevent blowing up your computer, the page will reload after 4096 events, which takes ~12 seconds
+    </div>
+    <div style="color: blue; font-size: 20px" id="counter">
+      Total Event Calls: <span id="total">{@counter}</span>
+    </div>
+    <div style="color: red; font-size: 72px" id="notice" phx-update="ignore">
+      I will disappear if the bug is not present.
+    </div>
+    """
+  end
+end

test/e2e/test_helper.exs

@@ -32,8 +32,12 @@ defmodule Phoenix.LiveViewTest.E2E.Layout do
   def render("live.html", assigns) do
     ~H"""
     <meta name="csrf-token" content={Plug.CSRFProtection.get_csrf_token()} />
+    <script>
+      window.hooks = {}
+    </script>
     <script src="/assets/phoenix/phoenix.min.js">
     </script>
+    {assigns[:pre_script]}
     <script type="module">
       import {LiveSocket} from "/assets/phoenix_live_view/phoenix_live_view.esm.js"
 
@@ -49,7 +53,10 @@ defmodule Phoenix.LiveViewTest.E2E.Layout do
         }
       }
       let csrfToken = document.querySelector("meta[name='csrf-token']").getAttribute("content");
-      let liveSocket = new LiveSocket("/live", window.Phoenix.Socket, {params: {_csrf_token: csrfToken}, hooks: Hooks})
+      let liveSocket = new LiveSocket("/live", window.Phoenix.Socket, {
+        params: {_csrf_token: csrfToken},
+        hooks: {...Hooks, ...window.hooks}
+      })
       liveSocket.connect()
       window.liveSocket = liveSocket
     </script>
@@ -151,6 +158,7 @@ defmodule Phoenix.LiveViewTest.E2E.Router do
       live "/3448", Issue3448Live
       live "/3496/a", Issue3496.ALive
       live "/3496/b", Issue3496.BLive
+      live "/3651", Issue3651Live
     end
   end
 
@@ -177,6 +185,7 @@ defmodule Phoenix.LiveViewTest.E2E.Router do
       live "/3047/a", Issue3047ALive
       live "/3047/b", Issue3047BLive
       live "/3169", Issue3169Live
+      live "/3647", Issue3647Live
     end
   end
 

test/e2e/tests/issues/3647.spec.js

@@ -0,0 +1,17 @@
+const {test, expect} = require("../../test-fixtures")
+const {syncLV} = require("../../utils")
+
+// https://github.com/phoenixframework/phoenix_live_view/issues/3647
+test("upload works when input event follows immediately afterwards", async ({page}) => {
+  await page.goto("/issues/3647")
+  await syncLV(page)
+
+  await expect(page.locator("ul li")).toHaveCount(0)
+  await expect(page.locator("input[name=\"user[name]\"]")).toHaveValue("")
+
+  await page.getByRole("button", {name: "Upload then Input"}).click()
+  await syncLV(page)
+
+  await expect(page.locator("ul li")).toHaveCount(1)
+  await expect(page.locator("input[name=\"user[name]\"]")).toHaveValue("0")
+})

test/e2e/tests/issues/3651.spec.js

@@ -0,0 +1,14 @@
+const {test, expect} = require("../../test-fixtures")
+const {syncLV} = require("../../utils")
+
+// https://github.com/phoenixframework/phoenix_live_view/issues/3651
+test("locked hook with dynamic id is properly cleared", async ({page}) => {
+  await page.goto("/issues/3651")
+  await syncLV(page)
+
+  await expect(page.locator("#notice")).toBeHidden()
+
+  // we want to wait for some events to have been pushed
+  await page.waitForTimeout(100)
+  expect(await page.evaluate(() => parseInt(document.querySelector("#total").textContent))).toBeLessThanOrEqual(50)
+})