Fix pending link update race condition (#3710)

applyPendingDiffs had a race condition where updates were silently
dropped if when trying to apply them there was already a new pending
link. We fix this by skipping applyPendingDiffs when a link is still
pending.

Bug originally reported by @Gazler:
https://gist.github.com/Gazler/80dac42d3134ff55219853f1cb105e78

Author: SteffenDE

assets/js/phoenix_live_view/view.js

@@ -754,6 +754,12 @@ export default class View {
   }
 
   applyPendingUpdates(){
+    // prevent race conditions where we might still be pending a new
+    // navigation after applying the current one;
+    // if we call update and a pendingDiff is not applied, it would
+    // be silently dropped otherwise, as update would push it back to
+    // pendingDiffs, but we clear it immediately after
+    if(this.liveSocket.hasPendingLink() && this.root.isMain()){ return }
     this.pendingDiffs.forEach(({diff, events}) => this.update(diff, events))
     this.pendingDiffs = []
     this.eachChild(child => child.applyPendingUpdates())

test/e2e/support/issues/issue_3709.ex

@@ -0,0 +1,43 @@
+defmodule Phoenix.LiveViewTest.E2E.Issue3709Live do
+  # https://github.com/phoenixframework/phoenix_live_view/issues/3709
+  use Phoenix.LiveView
+
+  defmodule SomeComponent do
+    use Phoenix.LiveComponent
+
+    def render(assigns) do
+      ~H"""
+      <div>
+        Hello
+      </div>
+      """
+    end
+  end
+
+  def mount(_params, _session, socket) do
+    {:ok, assign(socket, id: nil)}
+  end
+
+  def handle_params(params, _, socket) do
+    {:noreply, assign(socket, :id, params["id"])}
+  end
+
+  def render(assigns) do
+    ~H"""
+    <ul>
+      <li :for={i <- 1..10}>
+        <.link patch={"/issues/3709/#{i}"}>Link {i}</.link>
+      </li>
+    </ul>
+    <div>
+      <.live_component module={SomeComponent} id={"user-#{@id}"} /> id: {@id}
+      <div>
+        Click the button, then click any link.
+        <button onclick="document.querySelectorAll('li a').forEach((x) => x.click())">
+          Break Stuff
+        </button>
+      </div>
+    </div>
+    """
+  end
+end

test/e2e/test_helper.exs

@@ -163,6 +163,8 @@ defmodule Phoenix.LiveViewTest.E2E.Router do
       live "/3656", Issue3656Live
       live "/3658", Issue3658Live
       live "/3684", Issue3684Live
+      live "/3709", Issue3709Live
+      live "/3709/:id", Issue3709Live
     end
   end
 

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

@@ -0,0 +1,28 @@
+const {test, expect} = require("../../test-fixtures")
+const {syncLV} = require("../../utils")
+
+// https://github.com/phoenixframework/phoenix_live_view/issues/3709
+test("pendingDiffs don't race with navigation", async ({page}) => {
+  const logs = []
+  page.on("console", (e) => logs.push(e.text()))
+  const errors = []
+  page.on("pageerror", (err) => errors.push(err))
+
+  await page.goto("/issues/3709/1")
+  await syncLV(page)
+  await expect(page.locator("body")).toContainText("id: 1")
+
+  await page.getByRole("button", {name: "Break Stuff"}).click()
+  await syncLV(page)
+
+  expect(logs).not.toEqual(expect.arrayContaining([expect.stringMatching("Cannot read properties of undefined (reading 's')")]))
+
+  await page.getByRole("link", {name: "Link 5"}).click()
+  await syncLV(page)
+  await expect(page.locator("body")).toContainText("id: 5")
+
+  expect(logs).not.toEqual(expect.arrayContaining([expect.stringMatching("Cannot set properties of undefined (setting 'newRender')")]))
+
+  // no uncaught exceptions
+  expect(errors).toEqual([])
+})