From 4315ed5b827ff953fc108775329d4c3c18771759 Mon Sep 17 00:00:00 2001 From: Rico Kahler Date: Tue, 3 Jun 2025 01:23:41 -0500 Subject: [PATCH] refactor: simplify array unset logic by removing revision lock check Remove the isRevisionLocked() condition from array diffing logic when deciding between key-based vs index-based unset operations. The revision lock check was redundant because: - Key-based unsets are inherently safer in collaborative environments - Revision locking doesn't change the safety characteristics of the operation - If array items have unique keys, key-based operations should always be preferred regardless of whether we're targeting a specific revision - Index-based operations are still used as fallback when items lack unique keys This simplifies the logic to: if items have unique keys, always use key-based unsets; otherwise fall back to index-based unsets. --- src/diffPatch.ts | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/diffPatch.ts b/src/diffPatch.ts index 22d5448..696d695 100644 --- a/src/diffPatch.ts +++ b/src/diffPatch.ts @@ -267,21 +267,19 @@ function diffArray( const isSingle = itemA.length - itemB.length === 1 const unsetItems = itemA.slice(itemB.length) - // If we're revision locked, we can safely unset ranges (eg 5:). - // Also, if we don't have unique array keys, we can't use any better approach - // than array indexes. If we _do_ have unique array keys, we'll want to unset - // by key, as this is safer in a realtime, collaborative setting - if (isRevisionLocked(options) || !isUniquelyKeyed(unsetItems)) { - patches.push({ - op: 'unset', - path: path.concat([isSingle ? itemB.length : [itemB.length, '']]), - }) - } else { + // If we have unique array keys, we'll want to unset by key, as this is + // safer in a realtime, collaborative setting + if (isUniquelyKeyed(unsetItems)) { patches.push( ...unsetItems.map( (item): UnsetPatch => ({op: 'unset', path: path.concat({_key: item._key})}), ), ) + } else { + patches.push({ + op: 'unset', + path: path.concat([isSingle ? itemB.length : [itemB.length, '']]), + }) } } @@ -598,10 +596,6 @@ function nullifyUndefined(item: unknown, path: Path, index: number, options: Pat return null } -function isRevisionLocked(options: PatchOptions): boolean { - return Boolean(options.ifRevisionID) -} - function yes(_: unknown) { return true }