introduce :key-ed :for comprehensions (#3827)

* introduce `:key`-ed `:for` comprehensions

A keyed for comprehension works by transforming the comprehension's
content into a LiveComponent, which will perform change-tracking and
optimizes the payload over the wire:

```heex
<ul>
  <li :for={%{id: @id, name: @name} <- @Items} :key={@id}>
    Count: <span>{@count}</span>,
    item: {@name}
  </li>
</ul>
```

To support change tracking, keyed comprehensions require you to define
the left-hand side of the `:for` comprehension, as well as the `:key`,
using assign-syntax: `{@item <- @Items}` instead of `{item <- @Items}`.

Because they use live components under the hood, keyed comprehensions
come with the limitation of only being supported on HTML tags, so while
you can use a regular `:for` comprehension on `<.function_components>`,
and `<:slots>`, this is not supported keyed comprehensions and will raise
an exception at compile time.

* add diff test

* add documentation

* introduce :key-ed :for comprehensions without special syntax (#3837)

* with inner_block

* without inner_block

* update docs

* format

* include module and line in key

* store change tracked vars in vars

* Update lib/phoenix_live_view/engine.ex

* Update guides/server/assigns-eex.md

Co-authored-by: José Valim <[email protected]>

* simplify

* include column in id

* this is nice

* don't classify tag again

* no need for skip_require any more

* ensure meta is a keyword list

* don't duplicate if handling

* remove moved comment

* we don't need render_with

* test for keyed comprehension handling

* unit test

* mark as root in meta

* fix nested change tracking and add test

---------

Co-authored-by: José Valim <[email protected]>

Author: SteffenDE

guides/server/assigns-eex.md

@@ -267,8 +267,15 @@ in `@posts` changes, all posts are sent again.
 
 There are two common solutions to this problem.
 
-The first one is to use `Phoenix.LiveComponent` for each item in the
-comprehension:
+The first one is to also provide a `:key` expression:
+
+```heex
+<section :for={post <- @posts} :key={post.id}>
+  <h1>{expand_title(post.title)}</h1>
+</section>
+```
+
+This is functionally equivalent to doing:
 
 ```heex
 <section :for={post <- @posts}>
@@ -280,8 +287,9 @@ Since LiveComponents have their own assigns, LiveComponents would allow
 you to perform change tracking for each item. If the `@posts` variable
 changes, the client will simply send a list of component IDs (which are
 integers) and only the data for the posts that actually changed.
+You can read more about `:key` in the [documentation for `sigil_H/2`](Phoenix.Component.html#sigil_H/2-special-attributes).
 
-Another solution is to use `Phoenix.LiveView.stream/4`, which gives you
+The second solution is to use `Phoenix.LiveView.stream/4`, which gives you
 precise control over how elements are added, removed, and updated. Streams
 are particularly useful when you don't need to keep the collection in memory,
 allowing you to reduce the data sent over the wire and the server memory

lib/phoenix_component.ex

@@ -831,6 +831,35 @@ defmodule Phoenix.Component do
   Note that unlike Elixir's regular `for`, HEEx' `:for` does not support multiple
   generators in one expression. In such cases, you must use `EEx`'s blocks.
 
+  #### `:key`ed comprehensions
+
+  When using `:for`, you can optionally provide a `:key` expression to also perform
+  change-tracking inside the comprehension:
+
+  ```heex
+  <ul>
+    <li :for={%{id: id, name: name} <- @items} :key={id}>
+      Count: <span>{@count}</span>,
+      item: {name}
+    </li>
+  </ul>
+  ```
+
+  Internally, this works by turning the tag where the comprehension is defined on
+  into the template of a `Phoenix.LiveComponent`. Because of this, the following limitations
+  apply to comprehensions defined with `:key`:
+
+    1. A `:key` can only be defined on regular HTML tags, not on components or slots.
+    2. The diff over the wire is optimized to only send changes for each item,
+       but it will always include a list of component IDs (integers) specifying
+       the overall order of items.
+    3. Removing an entry involves separate round-trips with the client to confirm
+       the component removal.
+
+  We recommend to use `:key`ed comprehensions only if you already determined that you need
+  to optimize the diff over the write and [streams](`Phoenix.LiveView.stream/4`)
+  are not an option.
+
   ### Function components
 
   Function components are stateless components implemented as pure functions

lib/phoenix_live_view/engine.ex

@@ -297,7 +297,8 @@ defmodule Phoenix.LiveView.Engine do
   mind the collection itself is not "diffed" across renders.
   If one entry in the comprehension changes, the whole collection
   is sent again. Consider using `Phoenix.LiveComponent` and
-  `Phoenix.LiveView.stream/4` to optimize those cases.
+  `Phoenix.LiveView.stream/4` to optimize those cases, or see the
+  [`:key` attribute when using :for`](Phoenix.Component.html#sigil_H/2-special-attributes).
 
   The list of dynamics is always a list of iodatas or components,
   as we don't perform change tracking inside the comprehensions
@@ -422,6 +423,8 @@ defmodule Phoenix.LiveView.Engine do
           end
         end
 
+      root = Keyword.get(opts, :root, meta[:root])
+
       {:ok,
        quote do
          dynamic = fn track_changes? ->
@@ -434,7 +437,7 @@ defmodule Phoenix.LiveView.Engine do
            static: unquote(static),
            dynamic: dynamic,
            fingerprint: unquote(fingerprint),
-           root: unquote(opts[:root])
+           root: unquote(root)
          }
        end}
     else
@@ -618,20 +621,34 @@ defmodule Phoenix.LiveView.Engine do
 
   defp changed_assigns(assigns) do
     checks =
-      for {key, _} <- assigns, not nested_and_parent_is_checked?(key, assigns) do
+      for {{changed_var, key}, _} <- assigns, not nested_and_parent_is_checked?(key, assigns) do
+        changed = Macro.var(changed_var, __MODULE__)
+
         case key do
           [assign] ->
             quote do
-              unquote(__MODULE__).changed_assign?(changed, unquote(assign))
+              unquote(__MODULE__).changed_assign?(unquote(changed), unquote(assign))
             end
 
           [assign | tail] ->
+            assigns_var =
+              case changed_var do
+                :changed ->
+                  @assigns_var
+
+                :vars_changed ->
+                  # we pass a map %{var: var} for nested change tracking
+                  quote do
+                    %{unquote(assign) => unquote(Macro.var(assign, nil))}
+                  end
+              end
+
             quote do
               unquote(__MODULE__).nested_changed_assign?(
                 unquote(tail),
                 unquote(assign),
-                unquote(@assigns_var),
-                changed
+                unquote(assigns_var),
+                unquote(changed)
               )
             end
         end
@@ -700,8 +717,25 @@ defmodule Phoenix.LiveView.Engine do
               keys != %{},
               do: {key, to_component_keys(keys)}
 
+        has_vars_changed? =
+          Enum.any?(keys, fn {_name, entries} ->
+            is_list(entries) and Enum.any?(entries, &match?({:vars_changed, _}, &1))
+          end)
+
+        vars_changed =
+          if has_vars_changed? do
+            Macro.var(:vars_changed, __MODULE__)
+          else
+            []
+          end
+
         quote do
-          unquote(__MODULE__).to_component_static(unquote(keys), unquote(@assigns_var), changed)
+          unquote(__MODULE__).to_component_static(
+            unquote(keys),
+            unquote(@assigns_var),
+            changed,
+            unquote(vars_changed)
+          )
         end
       else
         Macro.escape(%{})
@@ -733,14 +767,24 @@ defmodule Phoenix.LiveView.Engine do
       true ->
         {_, keys, _} = analyze_and_return_tainted_keys(dynamic, vars, %{}, caller)
 
+        has_vars_changed? = keys != :all and Enum.any?(keys, &match?({:vars_changed, _}, &1))
+
+        vars_changed =
+          if has_vars_changed? do
+            Macro.var(:vars_changed, __MODULE__)
+          else
+            []
+          end
+
         quote do
           unquote(__MODULE__).to_component_dynamic(
             %{unquote_splicing(static)},
             unquote(dynamic),
             unquote(static_changed),
             unquote(to_component_keys(keys)),
             unquote(@assigns_var),
-            changed
+            changed,
+            unquote(vars_changed)
           )
         end
     end
@@ -750,25 +794,25 @@ defmodule Phoenix.LiveView.Engine do
   defp to_component_keys(map), do: Map.keys(map)
 
   @doc false
-  def to_component_static(_keys, _assigns, nil) do
+  def to_component_static(_keys, _assigns, nil, []) do
     nil
   end
 
-  def to_component_static(keys, assigns, changed) do
+  def to_component_static(keys, assigns, changed, vars_changed) do
     for {assign, entries} <- keys,
-        changed = component_changed(entries, assigns, changed),
+        changed = component_changed(entries, assigns, changed, vars_changed),
         into: %{},
         do: {assign, changed}
   end
 
   @doc false
-  def to_component_dynamic(static, dynamic, _static_changed, _keys, _assigns, nil) do
+  def to_component_dynamic(static, dynamic, _static_changed, _keys, _assigns, nil, []) do
     merge_dynamic_static_changed(dynamic, static, nil)
   end
 
-  def to_component_dynamic(static, dynamic, static_changed, keys, assigns, changed) do
+  def to_component_dynamic(static, dynamic, static_changed, keys, assigns, changed, vars_changed) do
     component_changed =
-      if component_changed(keys, assigns, changed) do
+      if component_changed(keys, assigns, changed, vars_changed) do
         Enum.reduce(dynamic, static_changed, fn {k, _}, acc -> Map.put(acc, k, true) end)
       else
         static_changed
@@ -781,19 +825,23 @@ defmodule Phoenix.LiveView.Engine do
     dynamic |> Map.merge(static) |> Map.put(:__changed__, changed)
   end
 
-  defp component_changed(:all, _assigns, _changed), do: true
+  defp component_changed(:all, _assigns, _changed, _vars_changed), do: true
 
-  defp component_changed([path], assigns, changed) do
+  defp component_changed([path], assigns, changed, vars_changed) do
     case path do
-      [key] -> changed_assign(changed, key)
-      [key | tail] -> nested_changed_assign(tail, key, assigns, changed)
+      {:changed, [key]} -> changed_assign(changed, key)
+      {:changed, [key | tail]} -> nested_changed_assign(tail, key, assigns, changed)
+      {:vars_changed, [key]} -> changed_assign(vars_changed, key)
+      {:vars_changed, [key | tail]} -> nested_changed_assign(tail, key, assigns, vars_changed)
     end
   end
 
-  defp component_changed(entries, assigns, changed) do
+  defp component_changed(entries, assigns, changed, vars_changed) do
     Enum.any?(entries, fn
-      [key] -> changed_assign?(changed, key)
-      [key | tail] -> nested_changed_assign?(tail, key, assigns, changed)
+      {:changed, [key]} -> changed_assign?(changed, key)
+      {:changed, [key | tail]} -> nested_changed_assign?(tail, key, assigns, changed)
+      {:vars_changed, [key]} -> changed_assign?(vars_changed, key)
+      {:vars_changed, [key | tail]} -> nested_changed_assign?(tail, key, assigns, vars_changed)
     end)
   end
 
@@ -887,11 +935,29 @@ defmodule Phoenix.LiveView.Engine do
     {ast, keys, vars}
   end
 
+  # if we find a variable (or something more complex handled by the other clauses)
+  # like foo[:bar][:baz] and foo is marked as :change_track in vars, we consider it
+  # as an assign, but look into vars_changed instead of changed
+  defp analyze_assign(
+         {name, _, context} = expr,
+         {type, map} = vars,
+         assigns,
+         _caller,
+         nest
+       )
+       when is_atom(name) and is_atom(context) and is_map_key(map, name) and type != :tainted do
+    if map[name] == :change_track do
+      {expr, vars, Map.put(assigns, {:vars_changed, [name | nest]}, true)}
+    else
+      {expr, vars, assigns}
+    end
+  end
+
   # @name
   defp analyze_assign({:@, meta, [{name, _, context}]}, vars, assigns, _caller, nest)
        when is_atom(name) and is_atom(context) do
     expr = {{:., meta, [@assigns_var, name]}, [no_parens: true] ++ meta, []}
-    {expr, vars, Map.put(assigns, [name | nest], true)}
+    {expr, vars, Map.put(assigns, {:changed, [name | nest]}, true)}
   end
 
   # assigns.name
@@ -903,7 +969,7 @@ defmodule Phoenix.LiveView.Engine do
          nest
        )
        when is_atom(name) and args in [[], nil] do
-    {expr, vars, Map.put(assigns, [name | nest], true)}
+    {expr, vars, Map.put(assigns, {:changed, [name | nest]}, true)}
   end
 
   # assigns[:name]
@@ -915,7 +981,7 @@ defmodule Phoenix.LiveView.Engine do
          nest
        )
        when is_atom(name) and is_access(access) do
-    {expr, vars, Map.put(assigns, [name | nest], true)}
+    {expr, vars, Map.put(assigns, {:changed, [name | nest]}, true)}
   end
 
   # Maybe: assigns.foo[:bar]
@@ -984,20 +1050,39 @@ defmodule Phoenix.LiveView.Engine do
     {expr, vars, assigns}
   end
 
-  # Vars always taint unless we are in restricted mode.
-  defp analyze({name, meta, nil} = expr, {:restricted, map}, assigns, caller)
+  # Vars always taint unless we are in restricted mode
+  # or the variable is marked as `:change_track` for vars_changed.
+  defp analyze({name, meta, nil} = expr, {:restricted, map} = vars, assigns, caller)
        when is_atom(name) do
-    if Map.has_key?(map, name) do
-      maybe_warn_taint(name, meta, caller)
-      {expr, {:tainted, map}, assigns}
-    else
-      {expr, {:restricted, map}, assigns}
+    case map do
+      %{^name => :tainted} ->
+        maybe_warn_taint(name, meta, caller)
+        {expr, {:tainted, map}, assigns}
+
+      %{^name => :change_track} ->
+        {expr, vars, Map.put(assigns, {:vars_changed, [name]}, true)}
+
+      _ ->
+        {expr, {:restricted, map}, assigns}
     end
   end
 
-  defp analyze({name, meta, nil} = expr, {_, map}, assigns, caller) when is_atom(name) do
-    maybe_warn_taint(name, meta, caller)
-    {expr, {:tainted, Map.put(map, name, true)}, assigns}
+  defp analyze({name, meta, nil} = expr, {type, map}, assigns, caller)
+       when is_atom(name) do
+    cond do
+      Map.get(map, name) == :change_track ->
+        {expr, {type, map}, Map.put(assigns, {:vars_changed, [name]}, true)}
+
+      Keyword.get(meta, :change_track) ->
+        # this is a variable inside the left-hand side of a keyed for expression;
+        # we mark it as change_track in the vars map so that we treat it as change-tracked
+        # when we see it used again later (see the previous analyze clause above)
+        {expr, {type, Map.put(map, name, :change_track)}, assigns}
+
+      true ->
+        maybe_warn_taint(name, meta, caller)
+        {expr, {:tainted, Map.put(map, name, :tainted)}, assigns}
+    end
   end
 
   # Quoted vars are ignored as they come from engine code.
@@ -1334,8 +1419,11 @@ defmodule Phoenix.LiveView.Engine do
   defp classify_taint(:with, [_ | _]), do: :live
   defp classify_taint(:for, [_ | _]), do: :live
 
-  # Constructs from Phoenix and TagEngine
+  # Constructs from TagEngine
   defp classify_taint(:inner_block, [_, [do: _]]), do: :live
+  defp classify_taint(:keyed_comprehension, [_, _, [do: _]]), do: :live
+
+  # Constructs from Phoenix.View
   defp classify_taint(:render_layout, [_, _, _, [do: _]]), do: :live
 
   # Special forms are forbidden and raise.

lib/phoenix_live_view/keyed_comprehension.ex

@@ -0,0 +1,26 @@
+defmodule Phoenix.LiveView.KeyedComprehension do
+  @moduledoc false
+
+  use Phoenix.LiveComponent
+
+  @impl true
+  def update(assigns, socket) do
+    # we assign all entries from vars_changed to change-track them inside
+    # the LiveComponent
+    socket =
+      Enum.reduce(assigns.vars_changed, socket, fn {key, value}, socket ->
+        assign(socket, key, value)
+      end)
+
+    socket
+    |> assign(:render, assigns.render)
+    |> assign(:keys, Map.keys(assigns.vars_changed))
+    |> then(&{:ok, &1})
+  end
+
+  @impl true
+  def render(assigns) do
+    vars_changed = Map.take(assigns.__changed__, assigns.keys)
+    assigns.render.(vars_changed)
+  end
+end