Skip to content

Commit

Permalink
async: compile and runtime checks
Browse files Browse the repository at this point in the history
  • Loading branch information
SteffenDE committed Mar 6, 2024
1 parent 34fb931 commit 2996d63
Show file tree
Hide file tree
Showing 7 changed files with 252 additions and 121 deletions.
86 changes: 58 additions & 28 deletions lib/phoenix_live_view/async.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/phoenix_component/verify_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
87 changes: 87 additions & 0 deletions test/phoenix_live_view/async_test.exs
Original file line number Diff line number Diff line change
@@ -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
26 changes: 26 additions & 0 deletions test/phoenix_live_view/integrations/assign_async_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ defmodule Phoenix.LiveView.AssignAsyncTest do
import Phoenix.LiveViewTest
alias Phoenix.LiveViewTest.Endpoint

import ExUnit.CaptureIO

@endpoint Endpoint

setup do
Expand Down Expand Up @@ -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
Expand Down
42 changes: 34 additions & 8 deletions test/phoenix_live_view/integrations/start_async_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ defmodule Phoenix.LiveView.StartAsyncTest do
import Phoenix.LiveViewTest
alias Phoenix.LiveViewTest.Endpoint

import ExUnit.CaptureIO

@endpoint Endpoint

setup do
Expand Down Expand Up @@ -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
Expand All @@ -79,26 +81,50 @@ 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
{:ok, lv, _html} = live(conn, "/start_async?test=put_flash")

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
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 2996d63

Please sign in to comment.