diff --git a/src/diffPatch.ts b/src/diffPatch.ts index ee3e150..2e9b668 100644 --- a/src/diffPatch.ts +++ b/src/diffPatch.ts @@ -1,6 +1,6 @@ import {makePatches, stringifyPatches} from '@sanity/diff-match-patch' import {DiffError} from './diffError.js' -import {type Path, pathToString} from './paths.js' +import {isKeyedObject, type KeyedSanityObject, type Path, pathToString} from './paths.js' import {validateProperty} from './validate.js' import { type Patch, @@ -13,6 +13,7 @@ import { type SanityInsertPatchOperation, type SanityDiffMatchPatchOperation, } from './patches.js' +import {difference, intersection} from './setOperations.js' /** * Document keys that are ignored during diff operations. @@ -41,16 +42,6 @@ const DMP_MIN_SIZE_FOR_RATIO_CHECK = 10_000 type PrimitiveValue = string | number | boolean | null | undefined -/** - * An object (record) that has a `_key` property - * - * @internal - */ -export interface KeyedSanityObject { - [key: string]: unknown - _key: string -} - /** * An object (record) that _may_ have a `_key` property * @@ -235,11 +226,20 @@ function diffObject(itemA: SanityObject, itemB: SanityObject, path: Path, patche } function diffArray(itemA: unknown[], itemB: unknown[], path: Path, patches: Patch[]) { + if (isUniquelyKeyed(itemA) && isUniquelyKeyed(itemB)) { + return diffArrayByKey(itemA, itemB, path, patches) + } + + return diffArrayByIndex(itemA, itemB, path, patches) +} + +function diffArrayByIndex(itemA: unknown[], itemB: unknown[], path: Path, patches: Patch[]) { // Check for new items if (itemB.length > itemA.length) { patches.push({ op: 'insert', - after: path.concat([-1]), + position: 'after', + path: path.concat([-1]), items: itemB.slice(itemA.length).map(nullifyUndefined), }) } @@ -276,39 +276,125 @@ function diffArray(itemA: unknown[], itemB: unknown[], path: Path, patches: Patc const segmentA = itemA.slice(0, overlapping) const segmentB = itemB.slice(0, overlapping) - return isUniquelyKeyed(segmentA) && isUniquelyKeyed(segmentB) - ? diffArrayByKey(segmentA, segmentB, path, patches) - : diffArrayByIndex(segmentA, segmentB, path, patches) -} - -function diffArrayByIndex(itemA: unknown[], itemB: unknown[], path: Path, patches: Patch[]) { - for (let i = 0; i < itemA.length; i++) { - diffItem(itemA[i], nullifyUndefined(itemB[i]), path.concat(i), patches) + for (let i = 0; i < segmentA.length; i++) { + diffItem(segmentA[i], nullifyUndefined(segmentB[i]), path.concat(i), patches) } return patches } +/** + * Diffs two arrays of keyed objects by their `_key` properties. + * + * This approach is preferred over index-based diffing for collaborative editing scenarios + * because it generates patches that are more resilient to concurrent modifications. + * When multiple users edit the same array simultaneously, key-based patches have better + * conflict resolution characteristics than index-based patches. + * + * The function handles three main operations: + * 1. **Reordering**: When existing items change positions within the array + * 2. **Content changes**: When the content of existing items is modified + * 3. **Insertions/Deletions**: When items are added or removed from the array + * + * @param source - The original array with keyed objects + * @param target - The target array with keyed objects + * @param path - The path to this array within the document + * @param patches - Array to accumulate generated patches + * @returns The patches array with new patches appended + */ function diffArrayByKey( - itemA: KeyedSanityObject[], - itemB: KeyedSanityObject[], + source: KeyedSanityObject[], + target: KeyedSanityObject[], path: Path, patches: Patch[], ) { - const keyedA = indexByKey(itemA) - const keyedB = indexByKey(itemB) + // Create lookup maps for efficient key-based access to array items + const sourceItemsByKey = new Map(source.map((item) => [item._key, item])) + const targetItemsByKey = new Map(target.map((item) => [item._key, item])) + + // Categorize keys by their presence in source vs target arrays + const sourceKeys = new Set(sourceItemsByKey.keys()) + const targetKeys = new Set(targetItemsByKey.keys()) + const keysRemovedFromSource = difference(sourceKeys, targetKeys) + const keysAddedToTarget = difference(targetKeys, sourceKeys) + const keysInBothArrays = intersection(sourceKeys, targetKeys) + + // Handle reordering of existing items within the array. + // We detect reordering by comparing the relative positions of keys that exist in both arrays, + // excluding keys that were added or removed (since they don't participate in reordering). + const sourceKeysStillPresent = Array.from(difference(sourceKeys, keysRemovedFromSource)) + const targetKeysAlreadyPresent = Array.from(difference(targetKeys, keysAddedToTarget)) + + // Track which keys need to be reordered by comparing their relative positions + const keyReorderOperations: {sourceKey: string; targetKey: string}[] = [] + + for (let i = 0; i < keysInBothArrays.size; i++) { + const keyAtPositionInSource = sourceKeysStillPresent[i] + const keyAtPositionInTarget = targetKeysAlreadyPresent[i] + + // If different keys occupy the same relative position, a reorder is needed + if (keyAtPositionInSource !== keyAtPositionInTarget) { + keyReorderOperations.push({ + sourceKey: keyAtPositionInSource, + targetKey: keyAtPositionInTarget, + }) + } + } + + // Generate reorder patch if any items changed positions + if (keyReorderOperations.length) { + patches.push({ + op: 'reorder', + path, + snapshot: source, + reorders: keyReorderOperations, + }) + } - // There's a bunch of hard/semi-hard problems related to using keys - // Unless we have the exact same order, just use indexes for now - if (!arrayIsEqual(keyedA.keys, keyedB.keys)) { - return diffArrayByIndex(itemA, itemB, path, patches) + // Process content changes for items that exist in both arrays + for (const key of keysInBothArrays) { + diffItem(sourceItemsByKey.get(key), targetItemsByKey.get(key), [...path, {_key: key}], patches) } - for (let i = 0; i < keyedB.keys.length; i++) { - const key = keyedB.keys[i] - const valueA = keyedA.index[key] - const valueB = nullifyUndefined(keyedB.index[key]) - diffItem(valueA, valueB, path.concat({_key: key}), patches) + // Remove items that no longer exist in the target array + for (const keyToRemove of keysRemovedFromSource) { + patches.push({op: 'unset', path: [...path, {_key: keyToRemove}]}) + } + + // Insert new items that were added to the target array + // We batch consecutive insertions for efficiency and insert them at the correct positions + if (keysAddedToTarget.size) { + let insertionAnchorKey: string // The key after which we'll insert pending items + let itemsPendingInsertion: unknown[] = [] + + const flushPendingInsertions = () => { + if (itemsPendingInsertion.length) { + patches.push({ + op: 'insert', + // Insert after the anchor key if we have one, otherwise insert at the beginning + ...(insertionAnchorKey + ? {position: 'after', path: [...path, {_key: insertionAnchorKey}]} + : {position: 'before', path: [...path, 0]}), + items: itemsPendingInsertion, + }) + } + } + + // Walk through the target array to determine where new items should be inserted + for (const key of targetKeys) { + if (keysAddedToTarget.has(key)) { + // This is a new item - add it to the pending insertion batch + itemsPendingInsertion.push(targetItemsByKey.get(key)!) + } else if (keysInBothArrays.has(key)) { + // This is an existing item - flush any pending insertions before it + flushPendingInsertions() + insertionAnchorKey = key + itemsPendingInsertion = [] + } + } + + // Flush any remaining insertions at the end + flushPendingInsertions() } return patches @@ -494,10 +580,59 @@ function serializePatches(patches: Patch[], curr?: SanityPatchOperation): Sanity if (curr) return [curr, ...serializePatches(patches)] return [ - {insert: {after: pathToString(patch.after), items: patch.items}}, + { + insert: { + [patch.position]: pathToString(patch.path), + items: patch.items, + }, + } as SanityInsertPatchOperation, ...serializePatches(rest), ] } + case 'reorder': { + if (curr) return [curr, ...serializePatches(patches)] + + // REORDER STRATEGY: Two-phase approach to avoid key collisions + // + // Problem: Direct key swaps can cause collisions. For example, swapping A↔B: + // - Set A's content to B: ✓ + // - Set B's content to A: ✗ (A's content was already overwritten) + // + // Solution: Use temporary keys as an intermediate step + // Phase 1: Move all items to temporary keys with their final content + // Phase 2: Update just the _key property to restore the final keys + + // Phase 1: Move items to collision-safe temporary keys + const tempKeyOperations: SanityPatchOperations = {} + tempKeyOperations.set = {} + + for (const {sourceKey, targetKey} of patch.reorders) { + const temporaryKey = `__temp_reorder_${sourceKey}__` + const finalContentForThisPosition = + patch.snapshot[getIndexForKey(patch.snapshot, targetKey)] + + Object.assign(tempKeyOperations.set, { + [pathToString([...patch.path, {_key: sourceKey}])]: { + ...finalContentForThisPosition, + _key: temporaryKey, + }, + }) + } + + // Phase 2: Update _key properties to restore the intended final keys + const finalKeyOperations: SanityPatchOperations = {} + finalKeyOperations.set = {} + + for (const {sourceKey, targetKey} of patch.reorders) { + const temporaryKey = `__temp_reorder_${sourceKey}__` + + Object.assign(finalKeyOperations.set, { + [pathToString([...patch.path, {_key: temporaryKey}, '_key'])]: targetKey, + }) + } + + return [tempKeyOperations, finalKeyOperations, ...serializePatches(rest)] + } default: { return [] } @@ -505,37 +640,40 @@ function serializePatches(patches: Patch[], curr?: SanityPatchOperation): Sanity } function isUniquelyKeyed(arr: unknown[]): arr is KeyedSanityObject[] { - const keys = [] + const seenKeys = new Set() - for (let i = 0; i < arr.length; i++) { - const key = getKey(arr[i]) - if (!key || keys.indexOf(key) !== -1) { - return false - } + for (const item of arr) { + // Each item must be a keyed object with a _key property + if (!isKeyedObject(item)) return false - keys.push(key) + // Each _key must be unique within the array + if (seenKeys.has(item._key)) return false + + seenKeys.add(item._key) } return true } -function getKey(obj: unknown) { - return typeof obj === 'object' && obj !== null && (obj as KeyedSanityObject)._key -} +// Cache to avoid recomputing key-to-index mappings for the same array +const keyToIndexCache = new WeakMap>() + +function getIndexForKey(keyedArray: KeyedSanityObject[], targetKey: string) { + const cachedMapping = keyToIndexCache.get(keyedArray) + if (cachedMapping) return cachedMapping[targetKey] -function indexByKey(arr: KeyedSanityObject[]) { - return arr.reduce( - (acc, item) => { - acc.keys.push(item._key) - acc.index[item._key] = item - return acc + // Build a mapping from _key to array index + const keyToIndexMapping = keyedArray.reduce>( + (mapping, {_key}, arrayIndex) => { + mapping[_key] = arrayIndex + return mapping }, - {keys: [] as string[], index: {} as {[key: string]: KeyedSanityObject}}, + {}, ) -} -function arrayIsEqual(itemA: unknown[], itemB: unknown[]) { - return itemA.length === itemB.length && itemA.every((item, i) => itemB[i] === item) + keyToIndexCache.set(keyedArray, keyToIndexMapping) + + return keyToIndexMapping[targetKey] } /** diff --git a/src/patches.ts b/src/patches.ts index ccd0ddd..647abca 100644 --- a/src/patches.ts +++ b/src/patches.ts @@ -1,4 +1,4 @@ -import type {Path} from './paths.js' +import type {KeyedSanityObject, Path} from './paths.js' /** * A `set` operation @@ -29,9 +29,10 @@ export interface UnsetPatch { * * @internal */ -export interface InsertAfterPatch { +export interface InsertPatch { op: 'insert' - after: Path + position: 'before' | 'after' | 'replace' + path: Path items: any[] } @@ -47,12 +48,26 @@ export interface DiffMatchPatch { value: string } +/** + * A `reorder` operation used to ... + * + * Note: NOT a serializable mutation. + * + * @public + */ +export interface ReorderPatch { + op: 'reorder' + path: Path + snapshot: KeyedSanityObject[] + reorders: {sourceKey: string; targetKey: string}[] +} + /** * Internal patch representation used during diff generation * * @internal */ -export type Patch = SetPatch | UnsetPatch | InsertAfterPatch | DiffMatchPatch +export type Patch = SetPatch | UnsetPatch | InsertPatch | DiffMatchPatch | ReorderPatch /** * A Sanity `set` patch mutation operation diff --git a/src/paths.ts b/src/paths.ts index e4c7cc6..495205a 100644 --- a/src/paths.ts +++ b/src/paths.ts @@ -1,5 +1,3 @@ -import type {KeyedSanityObject} from './diffPatch.js' - const IS_DOTTABLE_RE = /^[A-Za-z_][A-Za-z0-9_]*$/ /** @@ -54,6 +52,16 @@ export function pathToString(path: Path): string { }, '') } -function isKeyedObject(obj: any): obj is KeyedSanityObject { - return typeof obj === 'object' && typeof obj._key === 'string' +/** + * An object (record) that has a `_key` property + * + * @internal + */ +export interface KeyedSanityObject { + [key: string]: unknown + _key: string +} + +export function isKeyedObject(obj: unknown): obj is KeyedSanityObject { + return typeof obj === 'object' && !!obj && '_key' in obj && typeof obj._key === 'string' } diff --git a/src/setOperations.ts b/src/setOperations.ts new file mode 100644 index 0000000..84c77db --- /dev/null +++ b/src/setOperations.ts @@ -0,0 +1,29 @@ +/// + +export function difference(source: Set, target: Set): Set { + if ('difference' in Set.prototype) { + return source.difference(target) + } + + const result = new Set() + for (const item of source) { + if (!target.has(item)) { + result.add(item) + } + } + return result +} + +export function intersection(source: Set, target: Set): Set { + if ('intersection' in Set.prototype) { + return source.intersection(target) + } + + const result = new Set() + for (const item of source) { + if (target.has(item)) { + result.add(item) + } + } + return result +} diff --git a/test/__snapshots__/object-arrays.test.ts.snap b/test/__snapshots__/object-arrays.test.ts.snap index 596d62c..9690ec5 100644 --- a/test/__snapshots__/object-arrays.test.ts.snap +++ b/test/__snapshots__/object-arrays.test.ts.snap @@ -6,7 +6,7 @@ exports[`object arrays > add to end (multiple) 1`] = ` "patch": { "id": "die-hard-iii", "insert": { - "after": "characters[-1]", + "after": "characters[_key=="john"]", "items": [ { "_key": "simon", @@ -29,7 +29,7 @@ exports[`object arrays > add to end (single) 1`] = ` "patch": { "id": "die-hard-iii", "insert": { - "after": "characters[-1]", + "after": "characters[_key=="john"]", "items": [ { "_key": "simon", @@ -93,25 +93,108 @@ exports[`object arrays > remove from middle (single) 1`] = ` "patch": { "id": "die-hard-iii", "unset": [ - "characters[_key=="zeus"]", + "characters[_key=="simon"]", ], }, }, +] +`; + +exports[`object arrays > reorder (complete reverse) 1`] = ` +[ + { + "patch": { + "id": "die-hard-iii", + "set": { + "characters[_key=="john"]": { + "_key": "__temp_reorder_john__", + "name": "Zeus Carver", + }, + "characters[_key=="zeus"]": { + "_key": "__temp_reorder_zeus__", + "name": "John McClane", + }, + }, + }, + }, + { + "patch": { + "id": "die-hard-iii", + "set": { + "characters[_key=="__temp_reorder_john__"]._key": "zeus", + "characters[_key=="__temp_reorder_zeus__"]._key": "john", + }, + }, + }, +] +`; + +exports[`object arrays > reorder (simple swap) 1`] = ` +[ { "patch": { "id": "die-hard-iii", "set": { - "characters[1]._key": "zeus", + "characters[_key=="john"]": { + "_key": "__temp_reorder_john__", + "name": "Simon Gruber", + }, + "characters[_key=="simon"]": { + "_key": "__temp_reorder_simon__", + "name": "John McClane", + }, + }, + }, + }, + { + "patch": { + "id": "die-hard-iii", + "set": { + "characters[_key=="__temp_reorder_john__"]._key": "simon", + "characters[_key=="__temp_reorder_simon__"]._key": "john", + }, + }, + }, +] +`; + +exports[`object arrays > reorder with content change 1`] = ` +[ + { + "patch": { + "id": "die-hard-iii", + "set": { + "characters[_key=="john"]": { + "_key": "__temp_reorder_john__", + "name": "Zeus Carver", + }, + "characters[_key=="simon"]": { + "_key": "__temp_reorder_simon__", + "name": "John McClane", + }, + "characters[_key=="zeus"]": { + "_key": "__temp_reorder_zeus__", + "name": "Simon Gruber", + }, + }, + }, + }, + { + "patch": { + "id": "die-hard-iii", + "set": { + "characters[_key=="__temp_reorder_john__"]._key": "zeus", + "characters[_key=="__temp_reorder_simon__"]._key": "john", + "characters[_key=="__temp_reorder_zeus__"]._key": "simon", }, }, }, { "patch": { "diffMatchPatch": { - "characters[1].name": "@@ -1,12 +1,11 @@ --Simon Grub -+Zeus Carv - er + "characters[_key=="zeus"].name": "@@ -4,8 +4,12 @@ + s Carver ++ Jr. ", }, "id": "die-hard-iii", @@ -119,3 +202,109 @@ exports[`object arrays > remove from middle (single) 1`] = ` }, ] `; + +exports[`object arrays > reorder with insertion and deletion 1`] = ` +[ + { + "patch": { + "id": "die-hard-iii", + "set": { + "characters[_key=="john"]": { + "_key": "__temp_reorder_john__", + "name": "Zeus Carver", + }, + "characters[_key=="zeus"]": { + "_key": "__temp_reorder_zeus__", + "name": "John McClane", + }, + }, + }, + }, + { + "patch": { + "id": "die-hard-iii", + "set": { + "characters[_key=="__temp_reorder_john__"]._key": "zeus", + "characters[_key=="__temp_reorder_zeus__"]._key": "john", + }, + }, + }, + { + "patch": { + "id": "die-hard-iii", + "unset": [ + "characters[_key=="simon"]", + ], + }, + }, + { + "patch": { + "id": "die-hard-iii", + "insert": { + "after": "characters[_key=="zeus"]", + "items": [ + { + "_key": "hans", + "name": "Hans Gruber", + }, + ], + }, + }, + }, +] +`; + +exports[`object arrays > reorder with size change (multiple insertions) 1`] = ` +[ + { + "patch": { + "id": "die-hard-iii", + "set": { + "characters[_key=="john"]": { + "_key": "__temp_reorder_john__", + "name": "Zeus Carver", + }, + "characters[_key=="zeus"]": { + "_key": "__temp_reorder_zeus__", + "name": "John McClane", + }, + }, + }, + }, + { + "patch": { + "id": "die-hard-iii", + "set": { + "characters[_key=="__temp_reorder_john__"]._key": "zeus", + "characters[_key=="__temp_reorder_zeus__"]._key": "john", + }, + }, + }, + { + "patch": { + "id": "die-hard-iii", + "unset": [ + "characters[_key=="simon"]", + ], + }, + }, + { + "patch": { + "id": "die-hard-iii", + "insert": { + "after": "characters[_key=="john"]", + "items": [ + { + "_key": "hans", + "name": "Hans Gruber", + }, + { + "_key": "karl", + "name": "Karl Vreski", + }, + ], + }, + }, + }, +] +`; diff --git a/test/__snapshots__/pt.test.ts.snap b/test/__snapshots__/pt.test.ts.snap index fdcee02..def5834 100644 --- a/test/__snapshots__/pt.test.ts.snap +++ b/test/__snapshots__/pt.test.ts.snap @@ -2,15 +2,6 @@ exports[`portable text > undo bold change 1`] = ` [ - { - "patch": { - "id": "die-hard-iii", - "unset": [ - "portableText[_key=="920ebbba9ada"].children[_key=="1bda8032e34c"]", - "portableText[_key=="920ebbba9ada"].children[_key=="cb0568a8e746"]", - ], - }, - }, { "patch": { "diffMatchPatch": { @@ -22,5 +13,14 @@ exports[`portable text > undo bold change 1`] = ` "id": "die-hard-iii", }, }, + { + "patch": { + "id": "die-hard-iii", + "unset": [ + "portableText[_key=="920ebbba9ada"].children[_key=="1bda8032e34c"]", + "portableText[_key=="920ebbba9ada"].children[_key=="cb0568a8e746"]", + ], + }, + }, ] `; diff --git a/test/fixtures/object-array-reorder.ts b/test/fixtures/object-array-reorder.ts new file mode 100644 index 0000000..9d4fc22 --- /dev/null +++ b/test/fixtures/object-array-reorder.ts @@ -0,0 +1,72 @@ +export const a = { + _id: 'die-hard-iii', + title: 'Die Hard with a Vengeance', + characters: [ + {_key: 'john', name: 'John McClane'}, + {_key: 'simon', name: 'Simon Gruber'}, + {_key: 'zeus', name: 'Zeus Carver'}, + ], +} + +// Simple reorder: swap first two items +export const b = { + _id: 'die-hard-iii', + title: 'Die Hard with a Vengeance', + characters: [ + {_key: 'simon', name: 'Simon Gruber'}, + {_key: 'john', name: 'John McClane'}, + {_key: 'zeus', name: 'Zeus Carver'}, + ], +} + +// Complex reorder: completely reverse the array +export const c = { + _id: 'die-hard-iii', + title: 'Die Hard with a Vengeance', + characters: [ + {_key: 'zeus', name: 'Zeus Carver'}, + {_key: 'simon', name: 'Simon Gruber'}, + {_key: 'john', name: 'John McClane'}, + ], +} + +// Reorder with content change: move zeus to front and change his name +export const d = { + _id: 'die-hard-iii', + title: 'Die Hard with a Vengeance', + characters: [ + {_key: 'zeus', name: 'Zeus Carver Jr.'}, + {_key: 'john', name: 'John McClane'}, + {_key: 'simon', name: 'Simon Gruber'}, + ], +} + +// Complex scenario: reorder + insertion + deletion +// - Remove simon (deletion) +// - Add hans as new villain (insertion) +// - Reorder zeus to front, john to end (reordering) +export const e = { + _id: 'die-hard-iii', + title: 'Die Hard with a Vengeance', + characters: [ + {_key: 'zeus', name: 'Zeus Carver'}, + {_key: 'hans', name: 'Hans Gruber'}, + {_key: 'john', name: 'John McClane'}, + ], +} + +// Complex scenario with size change: reorder + multiple insertions + deletion +// - Remove simon (deletion) +// - Add hans and karl as new villains (2 insertions) +// - Reorder zeus to front, john to middle (reordering) +// Result: array grows from 3 to 4 items +export const f = { + _id: 'die-hard-iii', + title: 'Die Hard with a Vengeance', + characters: [ + {_key: 'zeus', name: 'Zeus Carver'}, + {_key: 'john', name: 'John McClane'}, + {_key: 'hans', name: 'Hans Gruber'}, + {_key: 'karl', name: 'Karl Vreski'}, + ], +} diff --git a/test/object-arrays.test.ts b/test/object-arrays.test.ts index 49998ee..18e5cda 100644 --- a/test/object-arrays.test.ts +++ b/test/object-arrays.test.ts @@ -3,6 +3,7 @@ import {diffPatch} from '../src' import * as objectArrayAdd from './fixtures/object-array-add' import * as objectArrayRemove from './fixtures/object-array-remove' import * as objectArrayChange from './fixtures/object-array-change' +import * as objectArrayReorder from './fixtures/object-array-reorder' describe('object arrays', () => { test('change item', () => { @@ -28,4 +29,24 @@ describe('object arrays', () => { test('remove from middle (single)', () => { expect(diffPatch(objectArrayRemove.a, objectArrayRemove.d)).toMatchSnapshot() }) + + test('reorder (simple swap)', () => { + expect(diffPatch(objectArrayReorder.a, objectArrayReorder.b)).toMatchSnapshot() + }) + + test('reorder (complete reverse)', () => { + expect(diffPatch(objectArrayReorder.a, objectArrayReorder.c)).toMatchSnapshot() + }) + + test('reorder with content change', () => { + expect(diffPatch(objectArrayReorder.a, objectArrayReorder.d)).toMatchSnapshot() + }) + + test('reorder with insertion and deletion', () => { + expect(diffPatch(objectArrayReorder.a, objectArrayReorder.e)).toMatchSnapshot() + }) + + test('reorder with size change (multiple insertions)', () => { + expect(diffPatch(objectArrayReorder.a, objectArrayReorder.f)).toMatchSnapshot() + }) })