remove replaceRootHistory (#3625)

* remove replaceRootHistory

Relates to: #3529

`replaceRootHistory` was initially added by @josevalim in
36edb48,
but lost its purpose after less than three months with 04aaedc
removing the only call that would actually set `root: true` in the history
state.

Me not knowing what `root: true` is supposed to do reused replaceRootHistory
in #3335, reintroducing
the case where `root: true`` is set in the history state. While #3335 was reverted
later due to issues (#3508),
I reworked the back/forward navigation problem in
#3539, which again
used `replaceRootHistory`. As `root: true` would now be set in the history,
we'd call `replaceRootHistory` on live navigation. The problem is that it
was setting `type: "patch"` in the history, which leads to LiveView assuming
that it can patch when navigating using popstate, while the actual navigation
was `type: "navigate"`. After looking into it, I don't really see a reason
for replaceRootHistory to exist any more.

* add e2e test for #3529

* update changelog

CHANGELOG.md

@@ -117,6 +117,7 @@ generated user module:
 ### Bug fixes
 * Fix LiveComponents in nested LiveViews not updating under certain conditions ([#3626](https://github.com/phoenixframework/phoenix_live_view/issues/3626))
 * Fix client-side hooks not being cleared properly ([#3628](https://github.com/phoenixframework/phoenix_live_view/issues/3628))
+* Fix regression where browser back/forward buttons used `patch` instead of `navigate`, failing to update the page ([#3529](https://github.com/phoenixframework/phoenix_live_view/issues/3529))
 
 ## 1.0.2 (2025-01-09)
 

assets/js/phoenix_live_view/live_socket.js

@@ -695,7 +695,7 @@ export default class LiveSocket {
     })
     window.addEventListener("popstate", event => {
       if(!this.registerNewLocation(window.location)){ return }
-      let {type, backType, id, root, scroll, position} = event.state || {}
+      let {type, backType, id, scroll, position} = event.state || {}
       let href = window.location.href
 
       // Compare positions to determine direction
@@ -709,15 +709,11 @@ export default class LiveSocket {
 
       DOM.dispatchEvent(window, "phx:navigate", {detail: {href, patch: type === "patch", pop: true, direction: isForward ? "forward" : "backward"}})
       this.requestDOMUpdate(() => {
+        const callback = () => { this.maybeScroll(scroll) }
         if(this.main.isConnected() && (type === "patch" && id === this.main.id)){
-          this.main.pushLinkPatch(event, href, null, () => {
-            this.maybeScroll(scroll)
-          })
+          this.main.pushLinkPatch(event, href, null, callback)
         } else {
-          this.replaceMain(href, null, () => {
-            if(root){ this.replaceRootHistory() }
-            this.maybeScroll(scroll)
-          })
+          this.replaceMain(href, null, callback)
         }
       })
     }, false)
@@ -838,15 +834,6 @@ export default class LiveSocket {
     })
   }
 
-  replaceRootHistory(){
-    Browser.pushState("replace", {
-      root: true,
-      type: "patch",
-      id: this.main.id,
-      position: this.currentHistoryPosition // Preserve current position
-    })
-  }
-
   registerNewLocation(newLocation){
     let {pathname, search} = this.currentLocation
     if(pathname + search === newLocation.pathname + newLocation.search){

assets/js/phoenix_live_view/view.js

@@ -318,8 +318,12 @@ export default class View {
       this.formsForRecovery = this.getFormsForRecovery()
     }
     if(this.isMain() && window.history.state === null){
-      // set initial history entry if this is the first page load
-      this.liveSocket.replaceRootHistory()
+      // set initial history entry if this is the first page load (no history)
+      Browser.pushState("replace", {
+        type: "patch",
+        id: this.id,
+        position: this.liveSocket.currentHistoryPosition
+      })
     }
 
     if(liveview_version !== this.liveSocket.version()){

test/e2e/support/issues/issue_3529.ex

@@ -0,0 +1,23 @@
+defmodule Phoenix.LiveViewTest.E2E.Issue3529Live do
+  # https://github.com/phoenixframework/phoenix_live_view/issues/3529
+
+  use Phoenix.LiveView
+
+  alias Phoenix.LiveView.JS
+
+  def mount(_params, _session, socket) do
+    {:ok, assign(socket, :mounted, DateTime.utc_now())}
+  end
+
+  def handle_params(_params, _uri, socket) do
+    {:noreply, assign(socket, :next, :rand.uniform())}
+  end
+
+  def render(assigns) do
+    ~H"""
+    <h1>Mounted at {@mounted}</h1>
+    <.link navigate={"/issues/3529?param=#{@next}"}>Navigate</.link>
+    <.link patch={"/issues/3529?param=#{@next}"}>Patch</.link>
+    """
+  end
+end

test/e2e/test_helper.exs

@@ -158,6 +158,7 @@ defmodule Phoenix.LiveViewTest.E2E.Router do
       live "/3448", Issue3448Live
       live "/3496/a", Issue3496.ALive
       live "/3496/b", Issue3496.BLive
+      live "/3529", Issue3529Live
       live "/3651", Issue3651Live
     end
   end

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

@@ -0,0 +1,45 @@
+const {test, expect} = require("../../test-fixtures")
+const {syncLV} = require("../../utils")
+
+const pageText = async (page) => await page.evaluate(() => document.querySelector("h1").innerText)
+
+// https://github.com/phoenixframework/phoenix_live_view/issues/3529
+// https://github.com/phoenixframework/phoenix_live_view/pull/3625
+test("forward and backward navigation is handled properly (replaceRootHistory)", async ({page}) => {
+  await page.goto("/issues/3529")
+  await syncLV(page)
+
+  let text = await pageText(page)
+  await page.getByRole("link", {name: "Navigate"}).click()
+  await syncLV(page)
+
+  // navigate remounts and changes the text
+  expect(await pageText(page)).not.toBe(text)
+  text = await pageText(page)
+
+  await page.getByRole("link", {name: "Patch"}).click()
+  await syncLV(page)
+  // patch does not remount
+  expect(await pageText(page)).toBe(text)
+
+  // now we go back (should be patch again)
+  await page.goBack()
+  await syncLV(page)
+  expect(await pageText(page)).toBe(text)
+
+  // and then we back to the initial page and use back/forward
+  // this should be a navigate -> remount!
+  await page.goBack()
+  await syncLV(page)
+  expect(await pageText(page)).not.toBe(text)
+
+  // navigate
+  await page.goForward()
+  await syncLV(page)
+  text = await pageText(page)
+
+  // now back again (navigate)
+  await page.goBack()
+  await syncLV(page)
+  expect(await pageText(page)).not.toBe(text)
+})