From 8c690c1c23d77505b3c6826587dbd11ee037d480 Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Sun, 27 Oct 2024 12:46:09 +0100 Subject: [PATCH] Allow sharing assigns between live navigation This commit relies on a yet to be finalized feature in Phoenix Channels to perform a custom handover between channel rejoins. It changes the LV code to not leave the old channel before rejoining and also instructs Phoenix to not kill the old process before starting the new channel process. After the new channel is joined, the old one is killed. Any `assign_new` calls in the LV mount will try to fetch assigns from the old LV. This is a backwards compatible optimization, so if a version of Phoenix is used that does not support handover, it just falls back to calling the function supplied to assign_new as usual. Closes #3357. --- assets/js/phoenix_live_view/live_socket.js | 4 +- assets/js/phoenix_live_view/view.js | 18 ++++---- lib/phoenix_live_view/channel.ex | 48 ++++++++++++++++++---- lib/phoenix_live_view/socket.ex | 4 +- lib/phoenix_live_view/utils.ex | 29 ++++++++++--- mix.exs | 4 +- test/e2e/support/navigation.ex | 4 ++ test/e2e/tests/navigation.spec.js | 17 ++++++++ test/phoenix_component_test.exs | 26 ++++++++++++ 9 files changed, 128 insertions(+), 26 deletions(-) diff --git a/assets/js/phoenix_live_view/live_socket.js b/assets/js/phoenix_live_view/live_socket.js index 1b984eb87e..82c34997ff 100644 --- a/assets/js/phoenix_live_view/live_socket.js +++ b/assets/js/phoenix_live_view/live_socket.js @@ -390,7 +390,7 @@ export default class LiveSocket { let removeEls = DOM.all(this.outgoingMainEl, `[${this.binding("remove")}]`) let newMainEl = DOM.cloneNode(this.outgoingMainEl, "") this.main.showLoader(this.loaderTimeout) - this.main.destroy() + this.main.destroy(false) this.main = this.newRootView(newMainEl, flash, liveReferer) this.main.setRedirect(href) @@ -407,7 +407,7 @@ export default class LiveSocket { onDone() }) } - }) + }, true) } transitionRemoves(elements, skipSticky, callback){ diff --git a/assets/js/phoenix_live_view/view.js b/assets/js/phoenix_live_view/view.js index ff6f814fd6..ec19180d61 100644 --- a/assets/js/phoenix_live_view/view.js +++ b/assets/js/phoenix_live_view/view.js @@ -205,7 +205,7 @@ export default class View { return val === "" ? null : val } - destroy(callback = function (){ }){ + destroy(leave = true, callback = function (){ }){ this.destroyAllChildren() this.destroyed = true delete this.root.children[this.id] @@ -221,10 +221,14 @@ export default class View { DOM.markPhxChildDestroyed(this.el) this.log("destroyed", () => ["the child has been removed from the parent"]) - this.channel.leave() - .receive("ok", onFinished) - .receive("error", onFinished) - .receive("timeout", onFinished) + if(leave){ + this.channel.leave() + .receive("ok", onFinished) + .receive("error", onFinished) + .receive("timeout", onFinished) + } else { + onFinished() + } } setContainerClasses(...classes){ @@ -795,7 +799,7 @@ export default class View { return this.joinPush } - join(callback){ + join(callback, handover = false){ this.showLoader(this.liveSocket.loaderTimeout) this.bindChannel() if(this.isMain()){ @@ -806,7 +810,7 @@ export default class View { callback ? callback(this.joinCount, onDone) : onDone() } - this.wrapPush(() => this.channel.join(), { + this.wrapPush(() => this.channel.join(this.liveSocket.socket.timeout, handover), { ok: (resp) => this.liveSocket.requestDOMUpdate(() => this.onJoin(resp)), error: (error) => this.onJoinError(error), timeout: () => this.onJoinError({reason: "timeout"}) diff --git a/lib/phoenix_live_view/channel.ex b/lib/phoenix_live_view/channel.ex index 696933d0ef..c886cfe6e6 100644 --- a/lib/phoenix_live_view/channel.ex +++ b/lib/phoenix_live_view/channel.ex @@ -399,7 +399,7 @@ defmodule Phoenix.LiveView.Channel do end) end - def handle_call({@prefix, :child_mount, _child_pid, assign_new}, _from, state) do + def handle_call({@prefix, :get_assigns, _child_pid, assign_new}, _from, state) do assigns = Map.take(state.socket.assigns, assign_new) {:reply, {:ok, assigns}, state} end @@ -1123,6 +1123,10 @@ defmodule Phoenix.LiveView.Channel do transport_pid: transport_pid } = phx_socket + # TODO: change this to directly pattern match on handover_pid above + # when we require Phoenix 1.8 + handover_pid = Map.get(phx_socket, :handover_pid) + Process.put(:"$initial_call", {view, :mount, 3}) case params do @@ -1164,7 +1168,15 @@ defmodule Phoenix.LiveView.Channel do merged_session = Map.merge(socket_session, verified_user_session) lifecycle = load_lifecycle(config, route) - case mount_private(parent, root_view, assign_new, connect_params, connect_info, lifecycle) do + case mount_private( + parent, + root_view, + assign_new, + connect_params, + connect_info, + lifecycle, + handover_pid + ) do {:ok, mount_priv} -> socket = Utils.configure_socket(socket, mount_priv, action, flash, host_uri) @@ -1254,7 +1266,15 @@ defmodule Phoenix.LiveView.Channel do socket end - defp mount_private(nil, root_view, assign_new, connect_params, connect_info, lifecycle) do + defp mount_private( + nil, + root_view, + assign_new, + connect_params, + connect_info, + lifecycle, + handover_pid + ) do {:ok, %{ connect_params: connect_params, @@ -1262,12 +1282,21 @@ defmodule Phoenix.LiveView.Channel do assign_new: {%{}, assign_new}, lifecycle: lifecycle, root_view: root_view, - live_temp: %{} + live_temp: %{}, + handover_pid: handover_pid }} end - defp mount_private(parent, root_view, assign_new, connect_params, connect_info, lifecycle) do - case sync_with_parent(parent, assign_new) do + defp mount_private( + parent, + root_view, + assign_new, + connect_params, + connect_info, + lifecycle, + _handover_pid + ) do + case get_assigns(parent, assign_new) do {:ok, parent_assigns} -> # Child live views always ignore the layout on `:use`. {:ok, @@ -1278,7 +1307,8 @@ defmodule Phoenix.LiveView.Channel do live_layout: false, lifecycle: lifecycle, root_view: root_view, - live_temp: %{} + live_temp: %{}, + handover_pid: nil }} {:error, :noproc} -> @@ -1286,9 +1316,9 @@ defmodule Phoenix.LiveView.Channel do end end - defp sync_with_parent(parent, assign_new) do + def get_assigns(pid, keys) do try do - GenServer.call(parent, {@prefix, :child_mount, self(), assign_new}) + GenServer.call(pid, {@prefix, :get_assigns, self(), keys}) catch :exit, {:noproc, _} -> {:error, :noproc} end diff --git a/lib/phoenix_live_view/socket.ex b/lib/phoenix_live_view/socket.ex index 55bfcc5037..a7a3074513 100644 --- a/lib/phoenix_live_view/socket.ex +++ b/lib/phoenix_live_view/socket.ex @@ -96,7 +96,7 @@ defmodule Phoenix.LiveView.Socket do } channel "lvu:*", Phoenix.LiveView.UploadChannel - channel "lv:*", Phoenix.LiveView.Channel + channel "lv:*", Phoenix.LiveView.Channel, handover_on_rejoin: true @impl Phoenix.Socket def connect(_params, %Phoenix.Socket{} = socket, connect_info) do @@ -111,7 +111,7 @@ defmodule Phoenix.LiveView.Socket do use Phoenix.Socket channel "lvu:*", Phoenix.LiveView.UploadChannel - channel "lv:*", Phoenix.LiveView.Channel + channel "lv:*", Phoenix.LiveView.Channel, handover_on_rejoin: true def connect(params, socket, info), do: {:ok, socket} defdelegate id(socket), to: unquote(__MODULE__) diff --git a/lib/phoenix_live_view/utils.ex b/lib/phoenix_live_view/utils.ex index 50ae88dfbf..d6643c4aa9 100644 --- a/lib/phoenix_live_view/utils.ex +++ b/lib/phoenix_live_view/utils.ex @@ -48,7 +48,7 @@ defmodule Phoenix.LiveView.Utils do %{assigns: %{^key => _}} -> socket - %{private: %{assign_new: {assigns, keys}}} -> + %{private: %{assign_new: {assigns, keys}} = private} -> # It is important to store the keys even if they are not in assigns # because maybe the controller doesn't have it but the view does. socket = put_in(socket.private.assign_new, {assigns, [key | keys]}) @@ -58,7 +58,7 @@ defmodule Phoenix.LiveView.Utils do key, case assigns do %{^key => value} -> value - %{} -> fun.(socket.assigns) + %{} -> maybe_handover_assign(socket, key, private[:handover_pid], fun) end ) @@ -72,17 +72,36 @@ defmodule Phoenix.LiveView.Utils do %{assigns: %{^key => _}} -> socket - %{private: %{assign_new: {assigns, keys}}} -> + %{private: %{assign_new: {assigns, keys}} = private} -> # It is important to store the keys even if they are not in assigns # because maybe the controller doesn't have it but the view does. socket = put_in(socket.private.assign_new, {assigns, [key | keys]}) - Phoenix.LiveView.Utils.force_assign(socket, key, Map.get_lazy(assigns, key, fun)) + + Phoenix.LiveView.Utils.force_assign( + socket, + key, + Map.get_lazy(assigns, key, fn -> + maybe_handover_assign(socket, key, private[:handover_pid], fun) + end) + ) %{} -> Phoenix.LiveView.Utils.force_assign(socket, key, fun.()) end end + defp maybe_handover_assign(_socket, _key, nil, fun) when is_function(fun, 0), do: fun.() + + defp maybe_handover_assign(socket, _key, nil, fun) when is_function(fun, 1), + do: fun.(socket.assigns) + + defp maybe_handover_assign(socket, key, pid, fun) when is_pid(pid) do + case Phoenix.LiveView.Channel.get_assigns(pid, [key]) do + {:ok, %{^key => value}} -> value + _ -> maybe_handover_assign(socket, key, nil, fun) + end + end + @doc """ Forces an assign on a socket. """ @@ -194,7 +213,7 @@ defmodule Phoenix.LiveView.Utils do socket |> clear_changed() |> clear_temp() - |> drop_private([:connect_info, :connect_params, :assign_new]) + |> drop_private([:connect_info, :connect_params, :assign_new, :handover_pid]) end @doc """ diff --git a/mix.exs b/mix.exs index 4a322cce9e..e63eadbb60 100644 --- a/mix.exs +++ b/mix.exs @@ -41,7 +41,9 @@ defmodule Phoenix.LiveView.MixProject do defp deps do [ - {:phoenix, "~> 1.6.15 or ~> 1.7.0"}, + # {:phoenix, "~> 1.6.15 or ~> 1.7.0"}, + # TODO: remove before merging + {:phoenix, github: "phoenixframework/phoenix", branch: "sd-handover-assigns", override: true}, {:plug, "~> 1.15"}, {:phoenix_template, "~> 1.0"}, {:phoenix_html, "~> 3.3 or ~> 4.0 or ~> 4.1"}, diff --git a/test/e2e/support/navigation.ex b/test/e2e/support/navigation.ex index fc6ceaf32c..7ea2242574 100644 --- a/test/e2e/support/navigation.ex +++ b/test/e2e/support/navigation.ex @@ -56,6 +56,7 @@ defmodule Phoenix.LiveViewTest.E2E.Navigation.ALive do def mount(_params, _session, socket) do socket |> assign(:param_current, nil) + |> assign_new(:foo, fn -> "bar" end) |> then(&{:ok, &1}) end @@ -73,6 +74,7 @@ defmodule Phoenix.LiveViewTest.E2E.Navigation.ALive do def render(assigns) do ~H"""

This is page A

+

Foo: <%= @foo %>

Current param: <%= @param_current %>

@@ -100,6 +102,7 @@ defmodule Phoenix.LiveViewTest.E2E.Navigation.BLive do @impl Phoenix.LiveView def mount(_params, _session, socket) do socket + |> assign_new(:foo, fn -> "baz" end) |> then(&{:ok, &1}) end @@ -128,6 +131,7 @@ defmodule Phoenix.LiveViewTest.E2E.Navigation.BLive do def render(assigns) do ~H"""

This is page B

+

Foo: <%= @foo %>

e.payload.indexOf("live_patch") !== -1)).toHaveLength(2); }); + +test("sharing assigns between live navigation", async ({ page }) => { + await page.goto("/navigation/a"); + await syncLV(page); + + await expect(page.getByText("Foo:")).toContainText("bar"); + await page.getByRole("link", { name: "LiveView B" }).click(); + await syncLV(page); + await expect(page.getByText("Foo:")).toContainText("bar"); + + await page.reload(); + await syncLV(page); + await expect(page.getByText("Foo:")).toContainText("baz"); + await page.getByRole("link", { name: "LiveView A" }).click(); + await syncLV(page); + await expect(page.getByText("Foo:")).toContainText("baz"); +}); diff --git a/test/phoenix_component_test.exs b/test/phoenix_component_test.exs index d23e0c8b8e..9bd4f823ec 100644 --- a/test/phoenix_component_test.exs +++ b/test/phoenix_component_test.exs @@ -141,6 +141,32 @@ defmodule Phoenix.ComponentUnitTest do } end + test "does handover of previous assigns when handover_pid is present" do + pid = spawn(fn -> + receive do + {:"$gen_call", {pid, _} = from, {:phoenix, :get_assigns, pid, [:existing]}} -> + GenServer.reply(from, {:ok, %{existing: "existing-handover"}}) + end + end) + + socket = + put_in(@socket.private[:assign_new], {%{}, []}) + |> put_in([Access.key(:private), :handover_pid], pid) + |> assign(existing2: "existing2") + |> assign_new(:existing, fn -> "new-existing" end) + |> assign_new(:existing2, fn -> "new-existing2" end) + |> assign_new(:notexisting, fn -> "new-notexisting" end) + + assert socket.assigns == %{ + existing: "existing-handover", + existing2: "existing2", + notexisting: "new-notexisting", + live_action: nil, + flash: %{}, + __changed__: %{existing: true, notexisting: true, existing2: true} + } + end + test "has access to assigns" do socket = put_in(@socket.private[:assign_new], {%{existing: "existing-parent"}, []})