raise custom error for duplicate ID when using keyed comprehensions (#3857)

* raise custom error for duplicate ID when using keyed comprehensions

* add test

Author: SteffenDE

lib/phoenix_component.ex

@@ -850,10 +850,13 @@ defmodule Phoenix.Component do
   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,
+    2. The `key` must be globally unique. LiveView prefixes the given value with the module,
+       line and file where the `:key` is defined, but there can still be situations where
+       conflicts occur.
+    3. 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
+    4. 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

lib/phoenix_live_view/diff.ex

@@ -645,8 +645,15 @@ defmodule Phoenix.LiveView.Diff do
           Enum.reduce(entries, {[], [], components, seen_ids}, fn
             {cid, id, new?, new_assigns}, {assigns_sockets, metadata, components, seen_ids} ->
               if Map.has_key?(seen_ids, [component | id]) do
-                raise "found duplicate ID #{inspect(id)} " <>
-                        "for component #{inspect(component)} when rendering template"
+                case id do
+                  {:keyed_comprehension, module, line, column, key} ->
+                    raise "found duplicate key #{inspect(key)} " <>
+                            "for keyed comprehension in module #{inspect(module)} at line #{line} column #{column}"
+
+                  _ ->
+                    raise "found duplicate ID #{inspect(id)} " <>
+                            "for component #{inspect(component)} when rendering template"
+                end
               end
 
               {socket, components, prints} =

lib/phoenix_live_view/tag_engine.ex

@@ -1204,8 +1204,8 @@ defmodule Phoenix.LiveView.TagEngine do
     ast =
       quote do
         Phoenix.LiveView.TagEngine.keyed_comprehension(
-          {unquote(state.caller.module), unquote(tag_meta.line), unquote(tag_meta.column),
-           unquote(key_expr)},
+          {:keyed_comprehension, unquote(state.caller.module), unquote(tag_meta.line),
+           unquote(tag_meta.column), unquote(key_expr)},
           %{unquote_splicing(Map.to_list(variables))},
           do: unquote(invoke_subengine(state, :handle_end, [[meta: [root: true]]]))
         )

test/phoenix_live_view/diff_test.exs

@@ -1783,7 +1783,8 @@ defmodule Phoenix.LiveView.DiffTest do
 
       assert {%{
                 1 =>
-                  {Phoenix.LiveView.KeyedComprehension, {Phoenix.LiveView.DiffTest, _, _, 1},
+                  {Phoenix.LiveView.KeyedComprehension,
+                   {:keyed_comprehension, Phoenix.LiveView.DiffTest, _, _, 1},
                    %{
                      id: 1,
                      name: "First",
@@ -1792,7 +1793,8 @@ defmodule Phoenix.LiveView.DiffTest do
                      myself: %Phoenix.LiveComponent.CID{cid: 1}
                    }, %{}, _},
                 2 =>
-                  {Phoenix.LiveView.KeyedComprehension, {Phoenix.LiveView.DiffTest, _, _, 2},
+                  {Phoenix.LiveView.KeyedComprehension,
+                   {:keyed_comprehension, Phoenix.LiveView.DiffTest, _, _, 2},
                    %{
                      id: 2,
                      name: "Second",
@@ -1911,5 +1913,18 @@ defmodule Phoenix.LiveView.DiffTest do
                :c => %{1 => %{1 => "Updated", 2 => "Updated"}}
              }
     end
+
+    test "raises on duplicate key" do
+      assigns = %{socket: %Socket{}}
+
+      rendered = ~H"""
+      <.keyed_comprehension_with_pattern items={[%{id: 1, name: "One"}]} />
+      <.keyed_comprehension_with_pattern items={[%{id: 1, name: "One"}]} />
+      """
+
+      assert_raise RuntimeError,
+                   ~r/found duplicate key 1 for keyed comprehension in module Phoenix.LiveView.DiffTest/,
+                   fn -> render(rendered) end
+    end
   end
 end