From 070ab1a017a0b9dd6f0076e144b3e19410aa3b6d Mon Sep 17 00:00:00 2001
From: Yossi Vainshtein <jvainshtein@paloaltonetworks.com>
Date: Mon, 8 Dec 2025 17:38:44 +0200
Subject: [PATCH] fix: If type has identifier attribute, use map for efficient
 array reconiliation

---
 __tests__/core/array.test.ts     | 35 ++++++++++++++++++++-
 src/types/complex-types/array.ts | 52 ++++++++++++++++++++++++++++----
 2 files changed, 80 insertions(+), 7 deletions(-)

diff --git a/__tests__/core/array.test.ts b/__tests__/core/array.test.ts
index 09438f08b..f308bde4a 100644
--- a/__tests__/core/array.test.ts
+++ b/__tests__/core/array.test.ts
@@ -12,7 +12,8 @@ import {
     IJsonPatch,
     setLivelinessChecking,
     detach,
-    cast
+    cast,
+    SnapshotIn
 } from "../../src"
 import { observable, autorun, configure } from "mobx"
 import { expect, test } from "bun:test"
@@ -288,6 +289,38 @@ test("it should reconciliate keyed instances correctly", () => {
     expect(store.todos[1] === coffee).toBe(true)
     expect(store.todos[2] === biscuit).toBe(false)
 })
+test("it should reconciliate large array instances efficiently", () => {
+    const Todo = types.model("Task", {
+        id: types.identifier,
+        task: ""
+    })
+    const Store = types
+        .model("Store", {
+            todos: types.array(types.maybeNull(Todo))
+        })
+        .actions(self => ({
+            setTodos(todos: SnapshotIn<typeof self.todos>) {
+                self.todos = cast(todos)
+            }
+        }))
+    const todos = Array.from({ length: 5000 }, (_, i) => ({ id: `todo-${i}`, task: `task-${i}` }))
+    const reversedTodos = [...todos].reverse()
+    const store = Store.create({
+        todos: todos
+    })
+
+    const todosBeforeSet = store.todos.slice()
+
+    const now = Date.now()
+    store.setTodos(reversedTodos)
+    const elapsed = Date.now() - now
+    expect(elapsed).toBeLessThan(1000)
+
+    for (let i = 0; i < todos.length; i++) {
+        // Check reconciliation, the instance in store should be the same instance as before the set, just in reversed order
+        expect(store.todos[i]).toBe(todosBeforeSet[todos.length - i - 1])
+    }
+})
 test("it correctly reconciliate when swapping", () => {
     const Task = types.model("Task", {})
     const Store = types.model({
diff --git a/src/types/complex-types/array.ts b/src/types/complex-types/array.ts
index 4718a0322..6695a270e 100644
--- a/src/types/complex-types/array.ts
+++ b/src/types/complex-types/array.ts
@@ -350,6 +350,30 @@ export function array<IT extends IAnyType>(subtype: IT): IArrayType<IT> {
     return new ArrayType<IT>(`${subtype.name}[]`, subtype)
 }
 
+function buildObjectByIdMap(nodes: AnyNode[]): [Set<string>, Map<string, Array<AnyNode>> | null] {
+    // Creates a map of node by identifier value.
+    //  In theory, several nodes can have the same identifier, if the array contains different types, so every identifier is mapped to an array of nodes with the same values.
+    //  In practice this in probably a rare case, so we can live with the performance hit.
+    //
+    // If not all nodes have identifier, we can't use the map for lookups, so we return null.
+    const identifierAttributes = new Set<string>()
+    const keyToObjectMap = new Map<string, Array<AnyObjectNode>>()
+
+    for (const node of nodes) {
+        if (node instanceof ObjectNode && node.identifierAttribute && node.identifier !== null) {
+            identifierAttributes.add(node.identifierAttribute)
+            const key = node.identifier
+            if (!keyToObjectMap.has(key)) {
+                keyToObjectMap.set(key, [])
+            }
+            keyToObjectMap.get(key)!.push(node)
+        } else {
+            return [identifierAttributes, null] // Not all nodes have identifier, so we can't use the map.
+        }
+    }
+    return [identifierAttributes, keyToObjectMap]
+}
+
 function reconcileArrayChildren<TT>(
     parent: AnyObjectNode,
     childType: IType<any, any, TT>,
@@ -359,6 +383,8 @@ function reconcileArrayChildren<TT>(
 ): AnyNode[] | null {
     let nothingChanged = true
 
+    const [identifierAttributes, oldNodeMap] = buildObjectByIdMap(oldNodes)
+
     for (let i = 0; ; i++) {
         const hasNewNode = i <= newValues.length - 1
         const oldNode = oldNodes[i]
@@ -404,14 +430,28 @@ function reconcileArrayChildren<TT>(
             // nothing to do, try to reorder
             let oldMatch = undefined
 
-            // find a possible candidate to reuse
-            for (let j = i; j < oldNodes.length; j++) {
-                if (areSame(oldNodes[j], newValue)) {
-                    oldMatch = oldNodes.splice(j, 1)[0]
-                    break
+            // Try to find match by identifier attributes
+            if (oldNodeMap && typeof newValue === "object" && newValue !== null) {
+                for (const identifierAttribute of identifierAttributes) {
+                    if (!(identifierAttribute in newValue)) {
+                        continue
+                    }
+                    const identifierValue = (newValue as any)[identifierAttribute]
+                    const matchingNodes = oldNodeMap.get(identifierValue) || []
+
+                    oldMatch = matchingNodes.find(node => areSame(node, newValue))
+                    if (oldMatch) {
+                        break
+                    }
+                }
+            } else {
+                for (let j = i; j < oldNodes.length; j++) {
+                    if (areSame(oldNodes[j], newValue)) {
+                        oldMatch = oldNodes.splice(j, 1)[0]
+                        break
+                    }
                 }
             }
-
             nothingChanged = false
             const newNode = valueAsNode(childType, parent, newPath, newValue, oldMatch)
             oldNodes.splice(i, 0, newNode)