Avoid cloning keyed comprehensions without moves

assets/js/phoenix_live_view/rendered.js

@@ -287,11 +287,17 @@ export default class Rendered {
 
   // keyed comprehensions
   mergeKeyed(target, source) {
-    // we need to clone the target since elements can move and otherwise
-    // it could happen that we modify an element that we'll need to refer to
-    // later
-    const clonedTarget = this.clone(target);
-    Object.entries(source[KEYED]).forEach(([i, entry]) => {
+    const entries = Object.entries(source[KEYED]);
+    const hasMoves = entries.some(
+      ([i, entry]) =>
+        i !== KEYED_COUNT &&
+        (Array.isArray(entry) || typeof entry === "number"),
+    );
+    // Moves read entries from their old positions. Clone before applying any
+    // entries so an earlier mutation cannot overwrite a position needed later.
+    // Stable-position updates never read from another position.
+    const clonedTarget = hasMoves && this.clone(target);
+    entries.forEach(([i, entry]) => {
       if (i === KEYED_COUNT) {
         return;
       }

assets/test/rendered_test.ts

@@ -9,20 +9,99 @@ import {
 
 describe("Rendered", () => {
   describe("mergeDiff", () => {
-    test("recursively merges two diffs", () => {
+    test("merges a diff into the rendered tree", () => {
       const simple = new Rendered("123", simpleDiff1);
       simple.mergeDiff(simpleDiff2);
       expect(simple.get()).toEqual({
         ...simpleDiffResult,
         [COMPONENTS]: {},
         newRender: true,
       });
+    });
 
+    test("merges stable-position keyed updates without cloning", () => {
       const deep = new Rendered("123", deepDiff1);
+      const clone = jest.spyOn(deep, "clone");
       deep.mergeDiff(deepDiff2);
+      expect(clone).not.toHaveBeenCalled();
       expect(deep.get()).toEqual({ ...deepDiffResult, [COMPONENTS]: {} });
     });
 
+    test("clones before moving entries so later moves use pre-merge positions", () => {
+      const rendered = new Rendered("123", {
+        [KEYED]: {
+          0: { 0: "first", 1: "unchanged" },
+          1: { 0: "second", 1: "old" },
+          [KEYED_COUNT]: 2,
+        },
+        [STATIC]: ["", "", ""],
+      });
+      const before = (rendered.get() as any)[KEYED];
+      const first = before[0];
+      const second = before[1];
+      const clone = jest.spyOn(rendered, "clone");
+
+      rendered.mergeDiff({
+        [KEYED]: {
+          0: [1, { 1: "updated" }],
+          1: 0,
+          [KEYED_COUNT]: 2,
+        },
+      });
+
+      expect(clone).toHaveBeenCalledTimes(1);
+      expect((rendered.get() as any)[KEYED]).toEqual({
+        0: { 0: "second", 1: "updated" },
+        1: { 0: "first", 1: "unchanged" },
+        [KEYED_COUNT]: 2,
+      });
+      expect(first).toEqual({ 0: "first", 1: "unchanged" });
+      expect(second).toEqual({ 0: "second", 1: "old" });
+    });
+
+    test("clones only a moved nested keyed subtree", () => {
+      const rendered = new Rendered("123", {
+        [KEYED]: {
+          0: {
+            0: "outer",
+            1: {
+              [KEYED]: {
+                0: { 0: "first" },
+                1: { 0: "second" },
+                [KEYED_COUNT]: 2,
+              },
+              [STATIC]: ["", ""],
+            },
+          },
+          [KEYED_COUNT]: 1,
+        },
+        [STATIC]: ["", "", ""],
+      });
+      const outer = (rendered.get() as any)[KEYED][0];
+      const nested = outer[1];
+      const clone = jest.spyOn(rendered, "clone");
+
+      rendered.mergeDiff({
+        [KEYED]: {
+          0: {
+            1: {
+              [KEYED]: { 0: 1, 1: 0, [KEYED_COUNT]: 2 },
+            },
+          },
+          [KEYED_COUNT]: 1,
+        },
+      });
+
+      expect(clone).toHaveBeenCalledTimes(1);
+      expect(clone.mock.calls[0][0]).toBe(nested);
+      expect((rendered.get() as any)[KEYED][0]).toBe(outer);
+      expect(nested[KEYED]).toEqual({
+        0: { 0: "second" },
+        1: { 0: "first" },
+        [KEYED_COUNT]: 2,
+      });
+    });
+
     test("merges the latter diff if it contains a `static` key", () => {
       const diff1 = { 0: ["a"], 1: ["b"] };
       const diff2 = { 0: ["c"], [STATIC]: ["c"] };