diff --git a/lib/phoenix_live_view/async.ex b/lib/phoenix_live_view/async.ex index 1e705b3097..aa8f88d59b 100644 --- a/lib/phoenix_live_view/async.ex +++ b/lib/phoenix_live_view/async.ex @@ -3,41 +3,50 @@ defmodule Phoenix.LiveView.Async do alias Phoenix.LiveView.{AsyncResult, Socket, Channel} - defp validate_function_env(func, op, env) do - warn = if Version.match?(System.version(), ">= 1.14.0") do - fn msg -> IO.warn(msg, env) end - else - fn msg -> IO.warn(msg) end + if Version.match?(System.version(), ">= 1.14.0") do + defp warn(msg, env) do + IO.warn(msg, env) + end + else + defp warn(msg, _env) do + IO.warn(msg) end + end - # prevent false positives, for example - # start_async(socket, :foo, function_that_returns_the_anonymous_function(socket)) - if match?({:&, _, _}, func) or match?({:fn, _, _}, func) do - Macro.prewalk(Macro.expand(func, env), fn - {:socket, _, nil} -> - warn.( - """ - you are accessing the LiveView Socket inside a function given to #{op}. + defp warn_socket_access(op, env) do + warn( + """ + you are accessing the LiveView Socket inside a function given to #{op}. - This is an expensive operation because the whole socket is copied to the new process. + This is an expensive operation because the whole socket is copied to the new process. - Instead of: + Instead of: - #{op}(socket, :key, fn -> - do_something(socket.assigns.my_assign) - end) + #{op}(socket, :key, fn -> + do_something(socket.assigns.my_assign) + end) - You should do: + You should do: - my_assign = socket.assigns.my_assign + my_assign = socket.assigns.my_assign - #{op}(socket, :key, fn -> - do_something(my_assign) - end) + #{op}(socket, :key, fn -> + do_something(my_assign) + end) - For more information, see https://hexdocs.pm/elixir/1.16.1/process-anti-patterns.html#sending-unnecessary-data. - """ - ) + For more information, see https://hexdocs.pm/elixir/1.16.1/process-anti-patterns.html#sending-unnecessary-data. + """, + env + ) + end + + defp validate_function_env(func, op, env) do + # prevent false positives, for example + # start_async(socket, :foo, function_that_returns_the_anonymous_function(socket)) + if match?({:&, _, _}, func) or match?({:fn, _, _}, func) do + Macro.prewalk(Macro.expand(func, env), fn + {:socket, _, nil} -> + warn_socket_access(op, env) other -> other @@ -47,20 +56,36 @@ defmodule Phoenix.LiveView.Async do :ok end + defp validate_function_env(func, op) do + {:env, variables} = Function.info(func, :env) + + if Enum.any?(variables, &match?(%Phoenix.LiveView.Socket{}, &1)) do + warn_socket_access(op, __ENV__) + end + end + def start_async(socket, key, func, opts, env) do validate_function_env(func, :start_async, env) quote do - Phoenix.LiveView.Async.run_async_task( + Phoenix.LiveView.Async.start_async( unquote(socket), unquote(key), unquote(func), - :start, unquote(opts) ) end end + def start_async(%Socket{} = socket, key, func, opts) when is_function(func, 0) do + # runtime check + if Phoenix.LiveView.connected?(socket) do + validate_function_env(func, :start_async) + end + + run_async_task(socket, key, func, :start, opts) + end + def assign_async(socket, key_or_keys, func, opts, env) do validate_function_env(func, :assign_async, env) @@ -77,6 +102,11 @@ defmodule Phoenix.LiveView.Async do def assign_async(%Socket{} = socket, key_or_keys, func, opts) when (is_atom(key_or_keys) or is_list(key_or_keys)) and is_function(func, 0) do + # runtime check + if Phoenix.LiveView.connected?(socket) do + validate_function_env(func, :assign_async) + end + keys = List.wrap(key_or_keys) # verifies result inside task diff --git a/test/phoenix_component/verify_test.exs b/test/phoenix_component/verify_test.exs index 741532e103..2f7fb18fad 100644 --- a/test/phoenix_component/verify_test.exs +++ b/test/phoenix_component/verify_test.exs @@ -544,7 +544,7 @@ defmodule Phoenix.ComponentVerifyTest do end end) - assert warnings == "" + refute warnings =~ "undefined attribute" end test "does warn for unknown attribute in slot without do-block when validate_attrs is true" do diff --git a/test/phoenix_live_view/async_test.exs b/test/phoenix_live_view/async_test.exs new file mode 100644 index 0000000000..36e6690748 --- /dev/null +++ b/test/phoenix_live_view/async_test.exs @@ -0,0 +1,87 @@ +defmodule Phoenix.LiveView.AsyncTest do + # run with async: false to prevent other messages from being captured + use ExUnit.Case, async: false + + import ExUnit.CaptureIO + + describe "async operations" do + for fun <- [:assign_async, :start_async] do + test "warns when passing socket to #{fun} function" do + warnings = + capture_io(:stderr, fn -> + Code.eval_string(""" + defmodule AssignAsyncSocket do + use Phoenix.LiveView + + def mount(_params, _session, socket) do + {:ok, #{unquote(fun)}(socket, :foo, fn -> + do_something(socket.assigns) + end)} + end + + defp do_something(_socket), do: :ok + end + """) + end) + + assert warnings =~ + "you are accessing the LiveView Socket inside a function given to #{unquote(fun)}" + end + + test "does not warn when accessing socket outside of function passed to #{fun}" do + warnings = + capture_io(:stderr, fn -> + Code.eval_string(""" + defmodule AssignAsyncSocket do + use Phoenix.LiveView + + def mount(_params, _session, socket) do + socket = assign(socket, :foo, :bar) + foo = socket.assigns.foo + + {:ok, #{unquote(fun)}(socket, :foo, fn -> + do_something(foo) + end)} + end + + defp do_something(assigns), do: :ok + end + """) + end) + + refute warnings =~ + "you are accessing the LiveView Socket inside a function given to #{unquote(fun)}" + end + + test "does not warn when argument is not a function (#{fun})" do + warnings = + capture_io(:stderr, fn -> + Code.eval_string(""" + defmodule AssignAsyncSocket do + use Phoenix.LiveView + + def mount(_params, _session, socket) do + socket = assign(socket, :foo, :bar) + + {:ok, #{unquote(fun)}(socket, :foo, function_that_returns_the_func(socket))} + end + + defp function_that_returns_the_func(socket) do + foo = socket.assigns.foo + + fn -> + do_something(foo) + end + end + + defp do_something(assigns), do: :ok + end + """) + end) + + refute warnings =~ + "you are accessing the LiveView Socket inside a function given to #{unquote(fun)}" + end + end + end +end diff --git a/test/phoenix_live_view/integrations/assign_async_test.exs b/test/phoenix_live_view/integrations/assign_async_test.exs index 0208b8b5ef..5c5c2ccb42 100644 --- a/test/phoenix_live_view/integrations/assign_async_test.exs +++ b/test/phoenix_live_view/integrations/assign_async_test.exs @@ -5,6 +5,8 @@ defmodule Phoenix.LiveView.AssignAsyncTest do import Phoenix.LiveViewTest alias Phoenix.LiveViewTest.Endpoint + import ExUnit.CaptureIO + @endpoint Endpoint setup do @@ -83,6 +85,30 @@ defmodule Phoenix.LiveView.AssignAsyncTest do assert render(lv) assert_receive {:exit, _pid, :boom}, 1000 end + + test "warns when accessing socket in function at runtime", %{conn: conn} do + warnings = + capture_io(:stderr, fn -> + {:ok, lv, _html} = live(conn, "/assign_async?test=socket_warning") + + render_async(lv) + end) + + assert warnings =~ + "you are accessing the LiveView Socket inside a function given to assign_async" + end + + test "warns when accessing socket in function (runtime only)", %{conn: conn} do + warnings = + capture_io(:stderr, fn -> + {:ok, lv, _html} = live(conn, "/assign_async?test=socket_warning_runtime_only") + + render_async(lv) + end) + + assert warnings =~ + "you are accessing the LiveView Socket inside a function given to assign_async" + end end describe "LiveComponent assign_async" do diff --git a/test/phoenix_live_view/integrations/start_async_test.exs b/test/phoenix_live_view/integrations/start_async_test.exs index 2d67611369..13987e30a9 100644 --- a/test/phoenix_live_view/integrations/start_async_test.exs +++ b/test/phoenix_live_view/integrations/start_async_test.exs @@ -5,6 +5,8 @@ defmodule Phoenix.LiveView.StartAsyncTest do import Phoenix.LiveViewTest alias Phoenix.LiveViewTest.Endpoint + import ExUnit.CaptureIO + @endpoint Endpoint setup do @@ -65,7 +67,7 @@ defmodule Phoenix.LiveView.StartAsyncTest do Process.register(self(), :start_async_trap_exit_test) {:ok, lv, _html} = live(conn, "/start_async?test=trap_exit") - assert render_async(lv, 200) =~ "result: :loading" + assert render_async(lv, 200) =~ "{:exit, :boom}" assert render(lv) assert_receive {:exit, _pid, :boom}, 1000 end @@ -79,19 +81,19 @@ defmodule Phoenix.LiveView.StartAsyncTest do test "navigate", %{conn: conn} do {:ok, lv, _html} = live(conn, "/start_async?test=navigate") - assert_redirect lv, "/start_async?test=ok" + assert_redirect(lv, "/start_async?test=ok") end test "patch", %{conn: conn} do {:ok, lv, _html} = live(conn, "/start_async?test=patch") - assert_patch lv, "/start_async?test=ok" + assert_patch(lv, "/start_async?test=ok") end test "redirect", %{conn: conn} do {:ok, lv, _html} = live(conn, "/start_async?test=redirect") - assert_redirect lv, "/not_found" + assert_redirect(lv, "/not_found") end test "put_flash", %{conn: conn} do @@ -99,6 +101,30 @@ defmodule Phoenix.LiveView.StartAsyncTest do assert render_async(lv) =~ "flash: hello" end + + test "warns when accessing socket in function at runtime", %{conn: conn} do + warnings = + capture_io(:stderr, fn -> + {:ok, lv, _html} = live(conn, "/start_async?test=socket_warning") + + render_async(lv) + end) + + assert warnings =~ + "you are accessing the LiveView Socket inside a function given to start_async" + end + + test "warns when accessing socket in function (runtime only)", %{conn: conn} do + warnings = + capture_io(:stderr, fn -> + {:ok, lv, _html} = live(conn, "/start_async?test=socket_warning_runtime_only") + + render_async(lv) + end) + + assert warnings =~ + "you are accessing the LiveView Socket inside a function given to start_async" + end end describe "LiveComponent start_async" do @@ -164,25 +190,25 @@ defmodule Phoenix.LiveView.StartAsyncTest do test "navigate", %{conn: conn} do {:ok, lv, _html} = live(conn, "/start_async?test=lc_navigate") - assert_redirect lv, "/start_async?test=ok" + assert_redirect(lv, "/start_async?test=ok") end test "patch", %{conn: conn} do {:ok, lv, _html} = live(conn, "/start_async?test=lc_patch") - assert_patch lv, "/start_async?test=ok" + assert_patch(lv, "/start_async?test=ok") end test "redirect", %{conn: conn} do {:ok, lv, _html} = live(conn, "/start_async?test=lc_redirect") - assert_redirect lv, "/not_found" + assert_redirect(lv, "/not_found") end test "navigate with flash", %{conn: conn} do {:ok, lv, _html} = live(conn, "/start_async?test=lc_navigate_flash") - flash = assert_redirect lv, "/start_async?test=ok" + flash = assert_redirect(lv, "/start_async?test=ok") assert %{"info" => "hello"} = flash end end diff --git a/test/phoenix_live_view_test.exs b/test/phoenix_live_view_test.exs index 2dd119170c..3719589a24 100644 --- a/test/phoenix_live_view_test.exs +++ b/test/phoenix_live_view_test.exs @@ -301,87 +301,4 @@ defmodule Phoenix.LiveViewUnitTest do end end end - - describe "async operations" do - import ExUnit.CaptureIO - - for fun <- [:assign_async, :start_async] do - test "warns when passing socket to #{fun} function" do - warnings = - capture_io(:stderr, fn -> - Code.eval_string(""" - defmodule AssignAsyncSocket do - use Phoenix.LiveView - - def mount(_params, _session, socket) do - {:ok, #{unquote(fun)}(socket, :foo, fn -> - do_something(socket.assigns) - end)} - end - - defp do_something(_socket), do: :ok - end - """) - end) - - assert warnings =~ - "you are accessing the LiveView Socket inside a function given to #{unquote(fun)}" - end - - test "does not warn when accessing socket outside of function passed to #{fun}" do - warnings = - capture_io(:stderr, fn -> - Code.eval_string(""" - defmodule AssignAsyncSocket do - use Phoenix.LiveView - - def mount(_params, _session, socket) do - socket = assign(socket, :foo, :bar) - foo = socket.assigns.foo - - {:ok, #{unquote(fun)}(socket, :foo, fn -> - do_something(foo) - end)} - end - - defp do_something(assigns), do: :ok - end - """) - end) - - refute warnings =~ - "you are accessing the LiveView Socket inside a function given to #{unquote(fun)}" - end - - test "does not warn when argument is not a function (#{fun})" do - warnings = - capture_io(:stderr, fn -> - Code.eval_string(""" - defmodule AssignAsyncSocket do - use Phoenix.LiveView - - def mount(_params, _session, socket) do - socket = assign(socket, :foo, :bar) - - {:ok, #{unquote(fun)}(socket, :foo, function_that_returns_the_func(socket))} - end - - defp function_that_returns_the_func(socket) do - foo = socket.assigns.foo - - fn -> - do_something(foo) - end - end - - defp do_something(assigns), do: :ok - end - """) - end) - - refute warnings =~ - "you are accessing the LiveView Socket inside a function given to #{unquote(fun)}" - end - end - end end diff --git a/test/support/live_views/general.ex b/test/support/live_views/general.ex index b84c03970e..e8913357d5 100644 --- a/test/support/live_views/general.ex +++ b/test/support/live_views/general.ex @@ -424,6 +424,26 @@ defmodule Phoenix.LiveViewTest.AssignAsyncLive do end)} end + def mount(%{"test" => "socket_warning"}, _session, socket) do + {:ok, + assign_async(socket, :data, fn -> + # simulate accessing socket in async task + Function.identity(socket) + {:ok, %{data: 0}} + end)} + end + + def mount(%{"test" => "socket_warning_runtime_only"}, _session, socket) do + {:ok, assign_async(socket, :data, function_that_returns_the_anonymous_function(socket))} + end + + defp function_that_returns_the_anonymous_function(socket) do + fn -> + Function.identity(socket) + {:ok, %{data: 0}} + end + end + def handle_info(:boom, _socket), do: exit(:boom) def handle_info(:cancel, socket) do @@ -586,7 +606,7 @@ defmodule Phoenix.LiveViewTest.StartAsyncLive do {:ok, socket |> assign(result: :loading) - |> assign_async(:result_task, fn -> + |> start_async(:result_task, fn -> spawn_link(fn -> exit(:boom) end) Process.sleep(100) :good @@ -628,6 +648,31 @@ defmodule Phoenix.LiveViewTest.StartAsyncLive do |> start_async(:flash, fn -> "hello" end)} end + def mount(%{"test" => "socket_warning"}, _session, socket) do + {:ok, + socket + |> assign(result: :loading) + |> start_async(:result_task, fn -> + # simulate accessing socket in async task + Function.identity(socket) + :ok + end)} + end + + def mount(%{"test" => "socket_warning_runtime_only"}, _session, socket) do + {:ok, + socket + |> assign(result: :loading) + |> start_async(:result_task, function_that_returns_the_anonymous_function(socket))} + end + + defp function_that_returns_the_anonymous_function(socket) do + fn -> + Function.identity(socket) + :ok + end + end + def handle_params(_unsigned_params, _uri, socket) do {:noreply, socket} end