Fix revertChangeset for index-based arrays

src/jsonDiff.ts

@@ -821,7 +821,7 @@ const revertLeafChange = (obj: any, change: any, embeddedKey = '$index', isPath?
     case Operation.UPDATE:
       return modifyKeyValue(obj, key, oldValue);
     case Operation.REMOVE:
-      return addKeyValue(obj, key, value);
+      return addKeyValue(obj, key, value, embeddedKey);
   }
 };
 
@@ -836,8 +836,20 @@ const revertLeafChange = (obj: any, change: any, embeddedKey = '$index', isPath?
  * consistency with other functions.
  */
 const revertArrayChange = (arr: any[], change: any) => {
-  for (const subchange of change.changes) {
-    if (subchange.value != null || subchange.type === Operation.REMOVE) {
+  let changes = change.changes;
+  if (change.embeddedKey === '$index') {
+    changes = [...changes].sort((a, b) => {
+      if (a.type === Operation.ADD && b.type === Operation.ADD) {
+        return Number(b.key) - Number(a.key);
+      }
+      if (a.type === Operation.ADD) return -1;
+      if (b.type === Operation.ADD) return 1;
+      return Number(a.key) - Number(b.key);
+    });
+  }
+
+  for (const subchange of changes) {
+    if (!subchange.changes) {
       revertLeafChange(arr, subchange, change.embeddedKey, change.embeddedKeyIsPath);
     } else {
       let element;

tests/__fixtures__/jsonDiff.fixture.ts

@@ -399,3 +399,26 @@ export const assortedDiffs: {
     expectedUpdate: []
   },
 ];
+
+export const indexArrayRevertCases: {
+  name: string;
+  oldVal: unknown[];
+  newVal: unknown[];
+}[] = [
+  { name: 'array grown at the end', oldVal: ['a'], newVal: ['a', 'b', 'c'] },
+  { name: 'array shrunk at the end', oldVal: ['a', 'b', 'c'], newVal: ['a'] },
+  { name: 'array filled from empty', oldVal: [], newVal: ['a', 'b', 'c'] },
+  { name: 'array emptied', oldVal: ['a', 'b', 'c'], newVal: [] },
+  { name: 'replace pair at a single index', oldVal: ['a', 2], newVal: [false, 2] },
+  { name: 'every element replaced by another type', oldVal: ['a', 1, 'b', 2], newVal: [1, 'a', 2, 'b'] },
+  { name: 'replace pair with trailing removes', oldVal: ['a', 'b', 'c'], newVal: [true] },
+  { name: 'replace pair with trailing adds', oldVal: [true], newVal: ['a', 'b', 'c'] },
+  { name: 'null values added', oldVal: [], newVal: [null, 'a', null] },
+  { name: 'null values removed', oldVal: [null, 'a', null], newVal: ['a'] },
+  { name: 'nested arrays changed on both sides', oldVal: [['a', 'b'], ['c']], newVal: [['a'], ['c', 'd']] },
+  {
+    name: 'objects replaced by primitives',
+    oldVal: [{ a: 1 }, { b: 2 }],
+    newVal: [{ a: 9 }, 'prim']
+  }
+];

tests/jsonDiff.test.ts

@@ -332,11 +332,83 @@ describe('jsonDiff#revertChangeset', () => {
     // Reverting a REMOVE operation should restore the object
     const obj = {};
     const changeset = [{ key: '$root', type: Operation.REMOVE, value: { x: 'y', z: 123 } }];
-    
+
     // Expected result is the original object that was removed
     const result = revertChangeset(obj, changeset);
     expect(result).toEqual({ x: 'y', z: 123 });
   });
+
+  describe('index-based arrays', () => {
+    it('should revert multiple ADDs without leaving later elements behind', () => {
+      // Reverting an ADD splices the element out, so the ADDs have to be undone
+      // from the highest index down. Undoing index 1 first shifts 'c' down into
+      // that slot, which used to leave the array as ['a', 'c'].
+      const oldArr = ['a'];
+      const newArr = ['a', 'b', 'c'];
+
+      const changes = diff(oldArr, newArr);
+      const result = revertChangeset(structuredClone(newArr), changes);
+      expect(result).toEqual(oldArr);
+    });
+
+    it('should revert multiple ADDs on an array nested in a property', () => {
+      const oldObj = { list: ['a'] };
+      const newObj = { list: ['a', 'b', 'c', 'd'] };
+
+      const changes = diff(oldObj, newObj);
+      const result = revertChangeset(structuredClone(newObj), changes);
+      expect(result).toEqual(oldObj);
+    });
+
+    it('should restore the original position when reverting a REMOVE/ADD replace pair', () => {
+      // A type change is encoded as REMOVE + ADD at the same index. Reverting
+      // the REMOVE has to splice the value back in at that index rather than
+      // append it, otherwise the element ends up at the end of the array.
+      const oldArr = ['a', 2];
+      const newArr = [false, 2];
+
+      const changes = diff(oldArr, newArr);
+      const result = revertChangeset(structuredClone(newArr), changes);
+      expect(result).toEqual(oldArr);
+    });
+
+    it('should revert ADDs carrying null values', () => {
+      // Null-valued subchanges must still be treated as leaf changes; routing
+      // them down the branch path silently dropped them.
+      const oldArr: unknown[] = [];
+      const newArr = [null, 'a', null];
+
+      const changes = diff(oldArr, newArr);
+      const result = revertChangeset(structuredClone(newArr), changes);
+      expect(result).toEqual(oldArr);
+    });
+
+    it('should revert mixed ADD and REMOVE operations in a single changeset', () => {
+      // Produces REMOVE@0, ADD@0 (the type change) plus ADD@2 and ADD@3.
+      const oldArr = ['a', 'b'];
+      const newArr = [1, 'b', 2, 'c'];
+
+      const changes = diff(oldArr, newArr);
+      const result = revertChangeset(structuredClone(newArr), changes);
+      expect(result).toEqual(oldArr);
+    });
+
+    it('should revert a nested branch change alongside a replace pair', () => {
+      // Branch subchange at index 0, replace pair at index 1, then trailing ADDs.
+      const oldObj = { list: [{ a: 1 }, { b: 2 }] };
+      const newObj = { list: [{ a: 9 }, 'prim', { e: 5 }, 'tail'] };
+
+      const changes = diff(oldObj, newObj);
+      const result = revertChangeset(structuredClone(newObj), changes);
+      expect(result).toEqual(oldObj);
+    });
+
+    it.each(fixtures.indexArrayRevertCases)('should round-trip a revert for $name', ({ oldVal, newVal }) => {
+      const changes = diff(oldVal, newVal);
+      const result = revertChangeset(structuredClone(newVal), changes);
+      expect(result).toEqual(oldVal);
+    });
+  });
 });
 
 describe('jsonDiff#flatten', () => {