From 8518e61a2d5a890066711cdc2a37e41e8b41f493 Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Sun, 27 Oct 2024 12:46:09 +0100 Subject: [PATCH 1/2] 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 | 5 ++- mix.lock | 6 +-- test/e2e/support/navigation.ex | 4 ++ test/e2e/tests/navigation.spec.js | 22 ++++++++-- test/phoenix_component_test.exs | 27 ++++++++++++ 10 files changed, 134 insertions(+), 33 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..01eb507c3d 100644 --- a/mix.exs +++ b/mix.exs @@ -41,7 +41,10 @@ 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/mix.lock b/mix.lock index 8ea34391a7..4917956bc0 100644 --- a/mix.lock +++ b/mix.lock @@ -1,6 +1,6 @@ %{ "bandit": {:hex, :bandit, "1.5.7", "6856b1e1df4f2b0cb3df1377eab7891bec2da6a7fd69dc78594ad3e152363a50", [:mix], [{:hpax, "~> 1.0.0", [hex: :hpax, repo: "hexpm", optional: false]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:thousand_island, "~> 1.0", [hex: :thousand_island, repo: "hexpm", optional: false]}, {:websock, "~> 0.5", [hex: :websock, repo: "hexpm", optional: false]}], "hexpm", "f2dd92ae87d2cbea2fa9aa1652db157b6cba6c405cb44d4f6dd87abba41371cd"}, - "castore": {:hex, :castore, "1.0.8", "dedcf20ea746694647f883590b82d9e96014057aff1d44d03ec90f36a5c0dc6e", [:mix], [], "hexpm", "0b2b66d2ee742cb1d9cb8c8be3b43c3a70ee8651f37b75a8b982e036752983f1"}, + "castore": {:hex, :castore, "1.0.9", "5cc77474afadf02c7c017823f460a17daa7908e991b0cc917febc90e466a375c", [:mix], [], "hexpm", "5ea956504f1ba6f2b4eb707061d8e17870de2bee95fb59d512872c2ef06925e7"}, "decimal": {:hex, :decimal, "2.1.1", "5611dca5d4b2c3dd497dec8f68751f1f1a54755e8ed2a966c2633cf885973ad6", [:mix], [], "hexpm", "53cfe5f497ed0e7771ae1a475575603d77425099ba5faef9394932b35020ffcc"}, "earmark_parser": {:hex, :earmark_parser, "1.4.41", "ab34711c9dc6212dda44fcd20ecb87ac3f3fce6f0ca2f28d4a00e4154f8cd599", [:mix], [], "hexpm", "a81a04c7e34b6617c2792e291b5a2e57ab316365c2644ddc553bb9ed863ebefa"}, "ecto": {:hex, :ecto, "3.12.1", "626765f7066589de6fa09e0876a253ff60c3d00870dd3a1cd696e2ba67bfceea", [:mix], [{:decimal, "~> 2.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "df0045ab9d87be947228e05a8d153f3e06e0d05ab10c3b3cc557d2f7243d1940"}, @@ -19,7 +19,7 @@ "makeup_html": {:hex, :makeup_html, "0.1.1", "c3d4abd39d5f7e925faca72ada6e9cc5c6f5fa7cd5bc0158315832656cf14d7f", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "44f2a61bc5243645dd7fafeaa6cc28793cd22f3c76b861e066168f9a5b2c26a4"}, "mime": {:hex, :mime, "2.0.6", "8f18486773d9b15f95f4f4f1e39b710045fa1de891fada4516559967276e4dc2", [:mix], [], "hexpm", "c9945363a6b26d747389aac3643f8e0e09d30499a138ad64fe8fd1d13d9b153e"}, "nimble_parsec": {:hex, :nimble_parsec, "1.4.0", "51f9b613ea62cfa97b25ccc2c1b4216e81df970acd8e16e8d1bdc58fef21370d", [:mix], [], "hexpm", "9c565862810fb383e9838c1dd2d7d2c437b3d13b267414ba6af33e50d2d1cf28"}, - "phoenix": {:hex, :phoenix, "1.7.14", "a7d0b3f1bc95987044ddada111e77bd7f75646a08518942c72a8440278ae7825", [:mix], [{:castore, ">= 0.0.0", [hex: :castore, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:phoenix_pubsub, "~> 2.1", [hex: :phoenix_pubsub, repo: "hexpm", optional: false]}, {:phoenix_template, "~> 1.0", [hex: :phoenix_template, repo: "hexpm", optional: false]}, {:phoenix_view, "~> 2.0", [hex: :phoenix_view, repo: "hexpm", optional: true]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.7", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:plug_crypto, "~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:websock_adapter, "~> 0.5.3", [hex: :websock_adapter, repo: "hexpm", optional: false]}], "hexpm", "c7859bc56cc5dfef19ecfc240775dae358cbaa530231118a9e014df392ace61a"}, + "phoenix": {:git, "https://github.com/phoenixframework/phoenix.git", "58cae7e9ca0e53bdc96c445210ce3f11bfdd5dce", [branch: "sd-handover-assigns"]}, "phoenix_ecto": {:hex, :phoenix_ecto, "4.6.2", "3b83b24ab5a2eb071a20372f740d7118767c272db386831b2e77638c4dcc606d", [:mix], [{:ecto, "~> 3.5", [hex: :ecto, repo: "hexpm", optional: false]}, {:phoenix_html, "~> 2.14.2 or ~> 3.0 or ~> 4.1", [hex: :phoenix_html, repo: "hexpm", optional: true]}, {:plug, "~> 1.9", [hex: :plug, repo: "hexpm", optional: false]}, {:postgrex, "~> 0.16 or ~> 1.0", [hex: :postgrex, repo: "hexpm", optional: true]}], "hexpm", "3f94d025f59de86be00f5f8c5dd7b5965a3298458d21ab1c328488be3b5fcd59"}, "phoenix_html": {:hex, :phoenix_html, "4.1.1", "4c064fd3873d12ebb1388425a8f2a19348cef56e7289e1998e2d2fa758aa982e", [:mix], [], "hexpm", "f2f2df5a72bc9a2f510b21497fd7d2b86d932ec0598f0210fed4114adc546c6f"}, "phoenix_html_helpers": {:hex, :phoenix_html_helpers, "1.0.1", "7eed85c52eff80a179391036931791ee5d2f713d76a81d0d2c6ebafe1e11e5ec", [:mix], [{:phoenix_html, "~> 4.0", [hex: :phoenix_html, repo: "hexpm", optional: false]}, {:plug, "~> 1.5", [hex: :plug, repo: "hexpm", optional: true]}], "hexpm", "cffd2385d1fa4f78b04432df69ab8da63dc5cf63e07b713a4dcf36a3740e3090"}, @@ -29,7 +29,7 @@ "phoenix_view": {:hex, :phoenix_view, "2.0.4", "b45c9d9cf15b3a1af5fb555c674b525391b6a1fe975f040fb4d913397b31abf4", [:mix], [{:phoenix_html, "~> 2.14.2 or ~> 3.0 or ~> 4.0", [hex: :phoenix_html, repo: "hexpm", optional: true]}, {:phoenix_template, "~> 1.0", [hex: :phoenix_template, repo: "hexpm", optional: false]}], "hexpm", "4e992022ce14f31fe57335db27a28154afcc94e9983266835bb3040243eb620b"}, "plug": {:hex, :plug, "1.16.1", "40c74619c12f82736d2214557dedec2e9762029b2438d6d175c5074c933edc9d", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "a13ff6b9006b03d7e33874945b2755253841b238c34071ed85b0e86057f8cddc"}, "plug_crypto": {:hex, :plug_crypto, "2.1.0", "f44309c2b06d249c27c8d3f65cfe08158ade08418cf540fd4f72d4d6863abb7b", [:mix], [], "hexpm", "131216a4b030b8f8ce0f26038bc4421ae60e4bb95c5cf5395e1421437824c4fa"}, - "telemetry": {:hex, :telemetry, "1.2.1", "68fdfe8d8f05a8428483a97d7aab2f268aaff24b49e0f599faa091f1d4e7f61c", [:rebar3], [], "hexpm", "dad9ce9d8effc621708f99eac538ef1cbe05d6a874dd741de2e689c47feafed5"}, + "telemetry": {:hex, :telemetry, "1.3.0", "fedebbae410d715cf8e7062c96a1ef32ec22e764197f70cda73d82778d61e7a2", [:rebar3], [], "hexpm", "7015fc8919dbe63764f4b4b87a95b7c0996bd539e0d499be6ec9d7f3875b79e6"}, "thousand_island": {:hex, :thousand_island, "1.3.5", "6022b6338f1635b3d32406ff98d68b843ba73b3aa95cfc27154223244f3a6ca5", [:mix], [{:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "2be6954916fdfe4756af3239fb6b6d75d0b8063b5df03ba76fd8a4c87849e180"}, "websock": {:hex, :websock, "0.5.3", "2f69a6ebe810328555b6fe5c831a851f485e303a7c8ce6c5f675abeb20ebdadc", [:mix], [], "hexpm", "6105453d7fac22c712ad66fab1d45abdf049868f253cf719b625151460b8b453"}, "websock_adapter": {:hex, :websock_adapter, "0.5.7", "65fa74042530064ef0570b75b43f5c49bb8b235d6515671b3d250022cb8a1f9e", [:mix], [{:bandit, ">= 0.6.0", [hex: :bandit, repo: "hexpm", optional: true]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.6", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:websock, "~> 0.5", [hex: :websock, repo: "hexpm", optional: false]}], "hexpm", "d0f478ee64deddfec64b800673fd6e0c8888b079d9f3444dd96d2a98383bdbd1"}, 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("phx_leave") !== -1)).toHaveLength(0); + await expect(webSocketEvents.filter(e => e.payload.indexOf("phx_join") !== -1)).toHaveLength(1); // we patched 2 times await expect(webSocketEvents.filter(e => 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..415b685637 100644 --- a/test/phoenix_component_test.exs +++ b/test/phoenix_component_test.exs @@ -141,6 +141,33 @@ 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"}, []}) From 2ec6c6ab369190b81fe31ce24a0f1b0f4c55a30a Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Tue, 29 Oct 2024 18:27:27 +0100 Subject: [PATCH 2/2] wip: server initiated handovers TODO: check flash handling TODO: refactor js to remove duplicatings between replaceMain and handover TODO: check if should keep the existing view class or replace it completely, passing the old channel to the new one? --- assets/js/phoenix_live_view/view.js | 40 +++++++++++ lib/phoenix_live_view/channel.ex | 107 ++++++++++++++++++++++------ test/e2e/support/navigation.ex | 20 ++++++ test/e2e/tests/navigation.spec.js | 22 ++++++ 4 files changed, 168 insertions(+), 21 deletions(-) diff --git a/assets/js/phoenix_live_view/view.js b/assets/js/phoenix_live_view/view.js index ec19180d61..70f3a3512a 100644 --- a/assets/js/phoenix_live_view/view.js +++ b/assets/js/phoenix_live_view/view.js @@ -765,6 +765,8 @@ export default class View { this.onChannel("redirect", ({to, flash}) => this.onRedirect({to, flash})) this.onChannel("live_patch", (redir) => this.onLivePatch(redir)) this.onChannel("live_redirect", (redir) => this.onLiveRedirect(redir)) + this.onChannel("live_handover", (redir) => this.startHandover(redir)) + this.onChannel("phx_handover", (payload) => this.completeHandover(payload)) this.channel.onError(reason => this.onError(reason)) this.channel.onClose(reason => this.onClose(reason)) } @@ -790,6 +792,44 @@ export default class View { onRedirect({to, flash, reloadToken}){ this.liveSocket.redirect(to, flash, reloadToken) } + startHandover(redir){ + if(!this.isMain()){ + throw new Error("unexpected handover for non main view") + } + let {to, kind, _flash} = redir + let href = this.expandURL(to) + let scroll = window.scrollY + this.liveSocket.withPageLoading({to: href, kind}, done => { + // let liveReferer = this.currentLocation.href + let removeEls = DOM.all(this.el, `[${this.binding("remove")}]`) + let newMainEl = DOM.cloneNode(this.el, "") + this.outGoingEl = this.el + this.el = newMainEl + this.showLoader(this.liveSocket.loaderTimeout) + + this.setRedirect(href) + this.liveSocket.transitionRemoves(removeEls, true) + this.handoverCallback = () => { + this.stopCallback = function(){} + this.liveSocket.requestDOMUpdate(() => { + // remove phx-remove els right before we replace the main element + removeEls.forEach(el => el.remove()) + DOM.findPhxSticky(document).forEach(el => newMainEl.appendChild(el)) + this.outGoingEl.replaceWith(this.el) + Browser.pushState(kind, {type: "redirect", id: this.id, scroll: scroll}, href) + DOM.dispatchEvent(window, "phx:navigate", {detail: {href, patch: false, pop: false}}) + this.liveSocket.registerNewLocation(window.location) + done() + }) + } + }) + } + + completeHandover(payload){ + this.stopCallback = this.handoverCallback + this.onJoin(payload) + } + isDestroyed(){ return this.destroyed } joinDead(){ this.isDead = true } diff --git a/lib/phoenix_live_view/channel.ex b/lib/phoenix_live_view/channel.ex index c886cfe6e6..099e40b56b 100644 --- a/lib/phoenix_live_view/channel.ex +++ b/lib/phoenix_live_view/channel.ex @@ -848,9 +848,12 @@ defmodule Phoenix.LiveView.Channel do opts = copy_flash(new_state, flash, opts) new_state - |> push_pending_events_on_redirect(new_socket) - |> push_live_redirect(opts, ref) - |> stop_shutdown_redirect(:live_redirect, opts) + |> maybe_handover(opts, ref, fn new_state -> + new_state + |> push_pending_events_on_redirect(new_socket) + |> push_live_redirect(opts, ref) + |> stop_shutdown_redirect(:live_redirect, opts) + end) {:live, :patch, %{to: _to, kind: _kind} = opts} when root_pid == self() -> {params, action} = patch_params_and_action!(new_socket, opts) @@ -1079,6 +1082,34 @@ defmodule Phoenix.LiveView.Channel do end end + # handover from previous LV in the same live session + defp mount({:handover, new_verified, route, url, params}, from, phx_socket) + when is_map_key(phx_socket, :handover_pid) and is_pid(phx_socket.handover_pid) and + phx_socket.handover_pid == new_verified.root_pid do + %Phoenix.Socket{private: %{connect_info: connect_info}} = phx_socket + %Session{view: view} = new_verified + + new_verified = %{new_verified | root_pid: self()} + + case load_live_view(view) do + {:ok, config} -> + verified_mount( + new_verified, + config, + route, + url, + params, + from, + phx_socket, + connect_info + ) + + {:error, _reason} -> + GenServer.reply(from, {:error, %{reason: "stale"}}) + {:stop, :shutdown, :no_state} + end + end + defp mount(%{}, from, phx_socket) do Logger.error("Mounting #{phx_socket.topic} failed because no session was provided") GenServer.reply(from, {:error, %{reason: "stale"}}) @@ -1098,7 +1129,7 @@ defmodule Phoenix.LiveView.Channel do end defp verified_mount( - %Session{} = verified, + %Session{} = verified_session, config, route, url, @@ -1110,13 +1141,11 @@ defmodule Phoenix.LiveView.Channel do %Session{ id: id, view: view, - root_view: root_view, parent_pid: parent, root_pid: root_pid, session: verified_user_session, - assign_new: assign_new, router: router - } = verified + } = verified_session %Phoenix.Socket{ endpoint: endpoint, @@ -1138,7 +1167,7 @@ defmodule Phoenix.LiveView.Channel do connect_params = params["params"] # Optional verified parts - flash = verify_flash(endpoint, verified, params["flash"], connect_params) + flash = verify_flash(endpoint, verified_session, params["flash"], connect_params) # connect_info is either a Plug.Conn during tests or a Phoenix.Socket map socket_session = Map.get(connect_info, :session, %{}) @@ -1169,9 +1198,7 @@ defmodule Phoenix.LiveView.Channel do lifecycle = load_lifecycle(config, route) case mount_private( - parent, - root_view, - assign_new, + verified_session, connect_params, connect_info, lifecycle, @@ -1186,7 +1213,7 @@ defmodule Phoenix.LiveView.Channel do |> Utils.maybe_call_live_view_mount!(view, params, merged_session, url) |> build_state(phx_socket) |> maybe_call_mount_handle_params(router, url, params) - |> reply_mount(from, verified, route) + |> reply_mount(from, verified_session, route) |> maybe_subscribe_to_live_reload() rescue exception -> @@ -1267,9 +1294,8 @@ defmodule Phoenix.LiveView.Channel do end defp mount_private( - nil, - root_view, - assign_new, + %Session{parent_pid: nil, root_view: root_view, assign_new: assign_new} = + verified_session, connect_params, connect_info, lifecycle, @@ -1277,6 +1303,7 @@ defmodule Phoenix.LiveView.Channel do ) do {:ok, %{ + verified_session: verified_session, connect_params: connect_params, connect_info: connect_info, assign_new: {%{}, assign_new}, @@ -1288,19 +1315,19 @@ defmodule Phoenix.LiveView.Channel do end defp mount_private( - parent, - root_view, - assign_new, + %Session{parent_pid: parent_pid, root_view: root_view, assign_new: assign_new} = + verified_session, connect_params, connect_info, lifecycle, _handover_pid ) do - case get_assigns(parent, assign_new) do + case get_assigns(parent_pid, assign_new) do {:ok, parent_assigns} -> # Child live views always ignore the layout on `:use`. {:ok, %{ + verified_session: verified_session, connect_params: connect_params, connect_info: connect_info, assign_new: {parent_assigns, assign_new}, @@ -1366,8 +1393,10 @@ defmodule Phoenix.LiveView.Channel do {:noreply, post_verified_mount(new_state)} {:live_redirect, opts, new_state} -> - GenServer.reply(from, {:error, %{live_redirect: opts}}) - {:stop, :shutdown, new_state} + maybe_handover(new_state, opts, nil, fn new_state -> + GenServer.reply(from, {:error, %{live_redirect: opts}}) + {:stop, :shutdown, new_state} + end) {:redirect, opts, new_state} -> GenServer.reply(from, {:error, %{redirect: opts}}) @@ -1609,4 +1638,40 @@ defmodule Phoenix.LiveView.Channel do %{} end end + + defp handover? do + phoenix_vsn = to_string(Application.spec(:phoenix)[:vsn]) + Version.match?(phoenix_vsn, ">= 1.8.0-dev") + end + + defp maybe_handover(state, redirect_opts, ref, fallback) do + %{socket: %{parent_pid: parent, private: %{verified_session: session}} = socket} = state + %{to: to} = redirect_opts + # get the full uri to verify the new session + destructure [path, query], :binary.split(to, "?") + to = %{socket.host_uri | path: path, query: query} + params = (query && Plug.Conn.Query.decode(query)) || %{} + + if diff = Diff.get_push_events_diff(socket), do: push_diff(state, diff, ref) + + # we can only handover on Phoenix >= 1.8.0 and when we are mounted at the router + with true <- handover?(), + nil <- parent, + {:ok, new_verified, route, url} <- + authorize_session( + session, + socket.endpoint, + %{"redirect" => to} + ) do + %{topic: topic, join_ref: join_ref} = state + state = push(state, "live_handover", redirect_opts) + + msg_payload = {:handover, new_verified, route, url, params} + send(socket.transport_pid, {:handover, msg_payload, self(), topic, join_ref}) + + {:noreply, state} + else + _ -> fallback.(state) + end + end end diff --git a/test/e2e/support/navigation.ex b/test/e2e/support/navigation.ex index 7ea2242574..b4a07fb347 100644 --- a/test/e2e/support/navigation.ex +++ b/test/e2e/support/navigation.ex @@ -70,6 +70,11 @@ defmodule Phoenix.LiveViewTest.E2E.Navigation.ALive do |> then(&{:noreply, &1}) end + @impl Phoenix.LiveView + def handle_event("push_navigate", _params, socket) do + {:noreply, push_navigate(socket, to: "/navigation/b")} + end + @impl Phoenix.LiveView def render(assigns) do ~H""" @@ -81,6 +86,8 @@ defmodule Phoenix.LiveViewTest.E2E.Navigation.ALive do <.styled_link patch={"/navigation/a?param=#{@param_next}"}>Patch this LiveView <.styled_link patch={"/navigation/a?param=#{@param_next}"} replace>Patch (Replace) <.styled_link navigate="/navigation/b#items-item-42">Navigate to 42 + + <.styled_link phx-click="push_navigate">push_navigate """ end @@ -127,12 +134,25 @@ defmodule Phoenix.LiveViewTest.E2E.Navigation.BLive do assign(socket, :id, id) end + @impl Phoenix.LiveView + def handle_event("push_navigate", _params, socket) do + {:noreply, push_navigate(socket, to: "/navigation/a")} + end + @impl Phoenix.LiveView def render(assigns) do ~H"""

This is page B

Foo: <%= @foo %>

+ + push_navigate + + { await expect(page.getByText("Foo:")).toContainText("bar"); await page.getByRole("link", { name: "LiveView B" }).click(); await syncLV(page); + await expect(page).toHaveURL("/navigation/b"); await expect(page.getByText("Foo:")).toContainText("bar"); await page.reload(); @@ -252,5 +253,26 @@ test("sharing assigns between live navigation", async ({ page }) => { await expect(page.getByText("Foo:")).toContainText("baz"); await page.getByRole("link", { name: "LiveView A" }).click(); await syncLV(page); + await expect(page).toHaveURL("/navigation/a"); await expect(page.getByText("Foo:")).toContainText("baz"); }); + +test("sharing assigns between live navigation (push_navigate)", async ({ page }) => { + await page.goto("/navigation/a"); + await syncLV(page); + + await expect(page.getByText("Foo:")).toContainText("bar"); + await page.getByRole("link", { name: "push_navigate" }).click(); + await syncLV(page); + await expect(page).toHaveURL("/navigation/b"); + 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: "push_navigate" }).click(); + await syncLV(page); + await expect(page).toHaveURL("/navigation/a"); + + await expect(page.getByText("Foo:")).toContainText("baz"); +}); \ No newline at end of file