Keep change tracking if dynamic is empty (#3936)

Author: josevalim

lib/phoenix_live_view/engine.ex

@@ -776,49 +776,63 @@ defmodule Phoenix.LiveView.Engine do
     # Rewrite slots in static parts
     static = slots_to_rendered(static, vars, caller, Keyword.get(meta, :slots, []))
 
-    cond do
-      # We can optimize when there are static parts and no dynamic parts.
-      # We skip live components because they have their own tracking.
-      static_extra != [] and dynamic == %{} and
-          not match?({:&, _, [{:/, _, [{:live_component, _, _}, 1]}]}, fun) ->
-        keys =
-          for {key, value} <- static_extra,
-              # We pass empty assigns because if this code is rendered,
-              # it means that upstream assigns were change tracked.
-              {_, keys, _} = analyze_and_return_tainted_keys(value, vars, %{}, caller),
-              # If keys are empty, it is never changed.
-              keys != %{},
-              do: {key, to_component_keys(keys)}
-
-        # [id: [changed: [:id]], children: [vars_changed: [:children]]]
-        # -> [changed: [:id], vars_changed: [:children]]
-        vars_changed_keys =
-          Enum.flat_map(keys, fn
-            {_assign, :all} -> []
-            {_assign, keys} -> keys
-          end)
-
-        static_changed =
-          quote do
-            unquote(__MODULE__).to_component_static(
-              unquote(keys),
-              unquote(@assigns_var),
-              changed,
-              %{unquote_splicing(vars_changed_vars(vars_changed_keys))},
-              vars_changed
-            )
-          end
+    static =
+      cond do
+        # Live components have their own tracking, so we skip the logic below
+        match?({:&, _, [{:/, _, [{:live_component, _, _}, 1]}]}, fun) ->
+          static
+
+        # Compute change tracking for static parts
+        static_extra != [] and (dynamic == %{} or without_dependencies?(dynamic, vars, caller)) ->
+          keys =
+            for {key, value} <- static_extra,
+                # We pass empty assigns because if this code is rendered,
+                # it means that upstream assigns were change tracked.
+                {_, keys, _} = analyze_and_return_tainted_keys(value, vars, %{}, caller),
+                # If keys are empty, it is never changed.
+                keys != %{},
+                do: {key, to_component_keys(keys)}
+
+          # [id: [changed: [:id]], children: [vars_changed: [:children]]]
+          # -> [changed: [:id], vars_changed: [:children]]
+          vars_changed_keys =
+            Enum.flat_map(keys, fn
+              {_assign, :all} -> []
+              {_assign, keys} -> keys
+            end)
+
+          static_changed =
+            quote do
+              unquote(__MODULE__).to_component_static(
+                unquote(keys),
+                unquote(@assigns_var),
+                changed,
+                %{unquote_splicing(vars_changed_vars(vars_changed_keys))},
+                vars_changed
+              )
+            end
 
-        quote do: %{unquote_splicing([__changed__: static_changed] ++ static)}
+          [__changed__: static_changed] ++ static
 
-      dynamic == %{} ->
-        quote do: %{unquote_splicing(static)}
+        true ->
+          # We must disable change tracking when there is a non empty dynamic part
+          # (for example `<.my_component {assigns}>`) in case the parent assigns
+          # already contain a `__changed__` key
+          [__changed__: nil] ++ static
+      end
 
-      true ->
-        quote do: Map.merge(unquote(dynamic), %{unquote_splicing(static)})
+    if dynamic == %{} do
+      quote do: %{unquote_splicing(static)}
+    else
+      quote do: Map.merge(unquote(dynamic), %{unquote_splicing(static)})
     end
   end
 
+  defp without_dependencies?(ast, vars, caller) do
+    {_, keys, _} = analyze_and_return_tainted_keys(ast, vars, %{}, caller)
+    keys == %{}
+  end
+
   defp to_component_keys(:all), do: :all
   defp to_component_keys(map), do: Map.keys(map)
 

test/phoenix_component/rendering_test.exs

@@ -175,7 +175,10 @@ defmodule Phoenix.ComponentRenderingTest do
       assigns = %{foo: 1, bar: %{foo: 2}, __changed__: %{}}
       assert eval(~H"<.changed foo={@foo} {@bar} />") == [nil]
 
-      # we cannot perform any change tracking when dynamic assigns are involved
+      # We cannot perform any change tracking if dynamic assigns has dependencies
+      assigns = %{foo: 1, __changed__: %{foo: true}}
+      assert eval(~H"<.changed foo={@foo} {%{foo: 2}} />") == [["%{foo: true}"]]
+
       assigns = %{foo: 1, bar: %{foo: 2}, __changed__: %{bar: true}}
       assert eval(~H"<.changed foo={@foo} {@bar} />") == [["nil"]]
 
@@ -190,15 +193,18 @@ defmodule Phoenix.ComponentRenderingTest do
       assigns = %{foo: %{a: 1, b: 2}, __changed__: %{}}
       assert eval(~H"<.changed {@foo} />") == [nil]
 
-      # we cannot perform any change tracking when dynamic assigns are involved
+      # We cannot perform any change tracking if dynamic assigns has dependencies
       assigns = %{foo: %{a: 1, b: 2}, __changed__: %{foo: true}}
       assert eval(~H"<.changed {@foo} />") == [["nil"]]
 
       assigns = %{foo: %{a: 1, b: 2}, bar: 3, __changed__: %{bar: true}}
-      assert eval(~H"<.changed {@foo} bar={@bar} />") == [["nil"]]
+      assert eval(~H"<.changed {%{a: 1, b: 2}} bar={@bar} />") == [["%{bar: true}"]]
 
       assigns = %{foo: %{a: 1, b: 2}, bar: 3, __changed__: %{bar: true}}
-      assert eval(~H"<.changed {%{a: 1, b: 2}} bar={@bar} />") == [["nil"]]
+      assert eval(~H"<.changed {assigns} bar={@bar} />") == [["nil"]]
+
+      assigns = %{foo: %{a: 1, b: 2}, bar: 3, __changed__: %{bar: true}}
+      assert eval(~H"<.changed {@foo} bar={@bar} />") == [["nil"]]
 
       assigns = %{bar: 3, __changed__: %{bar: true}}