fix: more efficient patch operation for the undo action of moving an …

…item inside an object

src/lib/logic/operations.test.ts

@@ -252,6 +252,34 @@ describe('operations', () => {
       assert.deepStrictEqual(Object.keys(revertedJson), ['a', 'b', 'c', 'nested'])
     })
 
+    test('should restore key order when sorting all keys of an object ', () => {
+      const json = {
+        b: 2,
+        a: 1,
+        c: 3
+      }
+      assert.deepStrictEqual(Object.keys(json), ['b', 'a', 'c'])
+
+      const operations: JSONPatchOperation[] = [
+        { op: 'move', from: '/a', path: '/a' },
+        { op: 'move', from: '/b', path: '/b' },
+        { op: 'move', from: '/c', path: '/c' }
+      ]
+      const updatedJson = immutableJSONPatch(json, operations)
+      assert.deepStrictEqual(Object.keys(updatedJson as Record<string, number>), ['a', 'b', 'c'])
+
+      const revertOperations = revertJSONPatchWithMoveOperations(json, operations)
+      assert.deepStrictEqual(revertOperations, [
+        { op: 'move', from: '/b', path: '/b' },
+        { op: 'move', from: '/a', path: '/a' },
+        { op: 'move', from: '/c', path: '/c' }
+      ])
+
+      const revertedJson = immutableJSONPatch(updatedJson, revertOperations)
+      assert.deepStrictEqual(revertedJson, json)
+      assert.deepStrictEqual(Object.keys(revertedJson), ['b', 'a', 'c'])
+    })
+
     test('should restore correctly revert multiple remove operations in an array', () => {
       const json = [0, 1, 2, 3, 4]
 

src/lib/logic/operations.ts

@@ -39,6 +39,7 @@ import {
 } from './selection.js'
 import type { ClipboardValues, DragInsideAction, JSONParser, JSONSelection } from '$lib/types'
 import { int } from '../utils/numberUtils.js'
+import { dedupeKeepLast } from '$lib/utils/arrayUtils'
 
 /**
  * Create a JSONPatch for an insert operation.
@@ -721,25 +722,30 @@ export function revertJSONPatchWithMoveOperations(
   json: unknown,
   operations: JSONPatchDocument
 ): JSONPatchDocument {
-  return revertJSONPatch(json, operations, {
-    before: (json, operation, revertOperations) => {
-      if (isJSONPatchRemove(operation)) {
-        const path = parseJSONPointer(operation.path)
-        return {
-          revertOperations: [...revertOperations, ...createRevertMoveOperations(json, path)]
+  return dedupeKeepLast(
+    revertJSONPatch(json, operations, {
+      before: (json, operation, revertOperations) => {
+        if (isJSONPatchRemove(operation)) {
+          const path = parseJSONPointer(operation.path)
+          return {
+            revertOperations: [...revertOperations, ...createRevertMoveOperations(json, path)]
+          }
         }
-      }
 
-      if (isJSONPatchMove(operation)) {
-        const from = parseJSONPointer(operation.from)
-        return {
-          revertOperations: [...revertOperations, ...createRevertMoveOperations(json, from)]
+        if (isJSONPatchMove(operation)) {
+          const from = parseJSONPointer(operation.from)
+          return {
+            revertOperations:
+              operation.from === operation.path
+                ? [operation, ...createRevertMoveOperations(json, from)] // move in-place (just for re-ordering object keys)
+                : [...revertOperations, ...createRevertMoveOperations(json, from)]
+          }
         }
-      }
 
-      return { document: json }
-    }
-  })
+        return { document: json }
+      }
+    })
+  )
 }
 
 function createRevertMoveOperations(json: unknown, path: JSONPath): JSONPatchOperation[] {

src/lib/utils/arrayUtils.test.ts

@@ -4,12 +4,12 @@ import {
   arrayStartsWith,
   arrayToObject,
   compareArrays,
+  dedupeKeepLast,
   forEachSample,
   getNestedPaths,
   moveItems,
   objectToArray
 } from './arrayUtils.js'
-import type { JSONArray } from 'immutable-json-patch'
 
 describe('arrayUtils', () => {
   test('compareArrays', () => {
@@ -28,7 +28,7 @@ describe('arrayUtils', () => {
 
   describe('getNestedPaths', () => {
     test('should extract all nested paths of an array containing objects', () => {
-      const json: JSONArray = [
+      const json = [
         { name: 'A', location: { latitude: 1, longitude: 2 } },
         { name: 'B', location: { latitude: 1, longitude: 2 } },
         { name: 'C', timestamp: 0 }
@@ -49,7 +49,7 @@ describe('arrayUtils', () => {
     })
 
     test('should extract all nested paths of an array containing objects, including objects', () => {
-      const json: JSONArray = [
+      const json = [
         { name: 'A', location: { latitude: 1, longitude: 2 } },
         { name: 'B', location: { latitude: 1, longitude: 2 } },
         { name: 'C', timestamp: 0 }
@@ -158,4 +158,43 @@ describe('arrayUtils', () => {
       expect(sample([], 4)).toEqual([])
     })
   })
+
+  describe('dedupeKeepLast', () => {
+    test('should keep the last item in case of a duplicate', () => {
+      expect(dedupeKeepLast([3, 1, 3])).toEqual([1, 3])
+      expect(dedupeKeepLast([3, 1, 3, 3])).toEqual([1, 3])
+
+      expect(
+        dedupeKeepLast([
+          { id: 1, name: 'Joe' },
+          { id: 3, name: 'Sarah' },
+          { id: 1, name: 'Joe' }
+        ])
+      ).toEqual([
+        { id: 3, name: 'Sarah' },
+        { id: 1, name: 'Joe' }
+      ])
+
+      expect(
+        dedupeKeepLast([
+          { id: 1, name: 'Joe' },
+          { id: 3, name: 'Sarah' },
+          { id: 1, name: 'Joey' }
+        ])
+      ).toEqual([
+        { id: 1, name: 'Joe' },
+        { id: 3, name: 'Sarah' },
+        { id: 1, name: 'Joey' }
+      ])
+    })
+
+    test('should pass a custom comparator', () => {
+      const comparator = (a: Record<string, unknown>, b: Record<string, unknown>) => a.id === b.id
+
+      const input = [{ id: 1, name: 'Joe' }, { id: 3 }, { id: 1, name: 'Joey' }]
+      const expected = [{ id: 3 }, { id: 1, name: 'Joey' }]
+
+      expect(dedupeKeepLast(input, comparator)).toEqual(expected)
+    })
+  })
 })

src/lib/utils/arrayUtils.ts

@@ -189,3 +189,19 @@ export function forEachSample<T>(
 export function insertItemsAt<T>(array: T[], index: number, items: T[]): T[] {
   return array.slice(0, index).concat(items).concat(array.slice(index))
 }
+
+/**
+ * Remove duplicate entries from an array, keeping the last item in case of a duplicate.
+ * This is similar to the `uniqWith` function of Lodash, but that function keeps the *first* item in case of a duplicate.
+ */
+export function dedupeKeepLast<T>(array: T[], comparator: (a: T, b: T) => boolean = isEqual): T[] {
+  return array.filter((item, index) => {
+    for (let i = index + 1; i < array.length; i++) {
+      if (comparator(item, array[i])) {
+        return false
+      }
+    }
+
+    return true
+  })
+}