diff --git a/src/jsonDiff.ts b/src/jsonDiff.ts index 20458b1..7b422c1 100644 --- a/src/jsonDiff.ts +++ b/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; diff --git a/tests/__fixtures__/jsonDiff.fixture.ts b/tests/__fixtures__/jsonDiff.fixture.ts index 9d96f26..50c8a85 100644 --- a/tests/__fixtures__/jsonDiff.fixture.ts +++ b/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'] + } +]; diff --git a/tests/jsonDiff.test.ts b/tests/jsonDiff.test.ts index f3ccfe5..d52f188 100644 --- a/tests/jsonDiff.test.ts +++ b/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', () => {