Skip to content

Conversation

@ricokahler
Copy link
Contributor

@ricokahler ricokahler commented Jun 4, 2025

This PR introduces a significant enhancement to how keyed object arrays are diffed and patched: support for key-based reordering.

Previously, diffArrayByKey would fall back to index-based diffing if the order of keys changed between the source and target arrays. This could lead to less precise or less conflict-resistant patches when items were simply moved around.

With this change, diffArrayByKey now:

  1. Detects reordering, insertions, and deletions independently.
  2. Generates a new internal ReorderPatch type when items change position.
  3. Emits insert and unset patches for items added or removed.
  4. Continues to diff the content of items that exist in both arrays (even if reordered).

The serializePatches function has been updated to handle the new ReorderPatch. It employs a two-phase reordering strategy to avoid key collisions during the patch application:

  • Phase 1: Moves all reordered items to temporary keys (__temp_reorder_<originalKey>__) while applying their final content.
  • Phase 2: Updates only the _key property of these temporary items to restore their intended final keys.

This two-phase approach ensures that direct key swaps (e.g., A ↔ B) are handled correctly without data loss.

Key Changes:

  • diffArrayByKey Overhaul:
    • No longer falls back to diffArrayByIndex if key order changes.
    • Uses Map for efficient key-based lookups.
    • Utilizes new difference and intersection set operations (from setOperations.ts) to categorize keys (removed, added, common).
    • Detects reordering by comparing relative positions of keys present in both source and target.
    • If reordering is detected, a new ReorderPatch is pushed to the patches array, containing the original array snapshot and the specific reorders (sourceKey → targetKey).
    • Handles insertions of new items, batching consecutive insertions and using position: 'before' or position: 'after' based on the surrounding existing keys.
    • Handles deletions of items no longer present.
    • Item content changes are still diffed using diffItem.
  • New ReorderPatch Type (Internal):
    • Added ReorderPatch to the internal Patch union in patches.ts.
    • ReorderPatch includes op: 'reorder', path, snapshot (the original array), and reorders (an array of {sourceKey, targetKey} pairs).
  • serializePatches Handling of ReorderPatch:
    • When a ReorderPatch is encountered:
      • It emits two separate set patch operations.
      • Phase 1 Set: Sets the content of items at their original keyed paths (_key: sourceKey) to be the final content of the item that will occupy that position, but with a temporary _key (e.g., _key: "__temp_reorder_sourceKey__").
      • Phase 2 Set: Sets the _key property of these temporary items (e.g., path: 'array[_key=="__temp_reorder_sourceKey__"]._key') to their final targetKey.
  • New setOperations.ts:
    • Added difference and intersection utility functions for Set<T>. These use native Set.prototype.difference/intersection if available (ESNext) or provide fallbacks for older JS environments.
  • New KeyedSanityObject and isKeyedObject in paths.ts:
    • Moved KeyedSanityObject type definition from diffPatch.ts to paths.ts.
    • Improved isKeyedObject type guard for robustness.
  • New getIndexForKey Utility:
    • Added a cached utility in diffPatch.ts to efficiently get the index of an item in a keyed array.
  • diffArray Logic Update:
    • Now checks if both source and target arrays are uniquely keyed before calling diffArrayByKey. Otherwise, it falls back to diffArrayByIndex.
    • Insertions for non-keyed arrays now use position: 'after' and path: path.concat([-1]).
  • Updated InsertPatch Type:
    • The internal InsertPatch (previously InsertAfterPatch) now includes a position: 'before' | 'after' | 'replace' property and uses path instead of just after.
    • serializePatches updated to use patch.position when creating the SanityInsertPatchOperation.
  • New Test Fixtures and Tests:
    • Added test/fixtures/object-array-reorder.ts with various reordering scenarios (simple swap, complete reverse, reorder with content change, reorder with insertions/deletions).
    • Added corresponding tests in test/object-arrays.test.ts to verify these scenarios.
    • Updated snapshots across several test files to reflect the new, more precise patching for reorders and keyed arrays.

Benefits:

  • More Collaborative-Editing-Friendly Patches: Key-based reordering generates patches that are significantly more resilient to concurrent modifications in arrays of keyed objects.
  • Preserves User Intent: Better captures the user's intent when they are merely reordering items, rather than treating it as a series of deletions and insertions at different indices.
  • Handles Complex Scenarios: The new logic correctly handles combinations of reordering, insertions, deletions, and content modifications within the same array.

This is a foundational improvement for generating robust patches for keyed arrays, crucial for collaborative environments.

@ricokahler ricokahler force-pushed the v6_local-patch-application-for-tests branch from fc6ce17 to 20fd215 Compare June 4, 2025 03:32
@ricokahler ricokahler force-pushed the v6_key-based-reordering branch from 2ad2ec4 to 2777c18 Compare June 4, 2025 03:32
@ricokahler ricokahler force-pushed the v6_local-patch-application-for-tests branch from 20fd215 to 1654acc Compare June 4, 2025 13:36
@ricokahler ricokahler force-pushed the v6_key-based-reordering branch from 2777c18 to a40879b Compare June 4, 2025 13:36
@ricokahler ricokahler force-pushed the v6_local-patch-application-for-tests branch from 1654acc to 8f32025 Compare June 4, 2025 15:32
@ricokahler ricokahler force-pushed the v6_key-based-reordering branch from a40879b to f7bcff7 Compare June 4, 2025 15:32
@ricokahler ricokahler marked this pull request as ready for review June 4, 2025 22:09
@ricokahler ricokahler requested a review from rexxars June 4, 2025 22:24
@ricokahler ricokahler force-pushed the v6_local-patch-application-for-tests branch from 8f32025 to c5e2f7f Compare June 6, 2025 21:57
@ricokahler ricokahler force-pushed the v6_key-based-reordering branch from f7bcff7 to 131e5bd Compare June 6, 2025 21:57
@ricokahler ricokahler force-pushed the v6_local-patch-application-for-tests branch from c5e2f7f to e35e767 Compare June 6, 2025 22:00
@ricokahler ricokahler force-pushed the v6_key-based-reordering branch from 131e5bd to 8a9f9ed Compare June 6, 2025 22:00
@ricokahler ricokahler changed the base branch from v6_local-patch-application-for-tests to graphite-base/41 June 6, 2025 22:16
@ricokahler ricokahler force-pushed the v6_key-based-reordering branch from 8a9f9ed to efa4722 Compare June 6, 2025 22:16
@ricokahler ricokahler changed the base branch from graphite-base/41 to v6_sequential-patch-serialization June 6, 2025 22:16
Comment on lines +229 to +233
if (isUniquelyKeyed(itemA) && isUniquelyKeyed(itemB)) {
return diffArrayByKey(itemA, itemB, path, patches)
}

return diffArrayByIndex(itemA, itemB, path, patches)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (isUniquelyKeyed(itemA) && isUniquelyKeyed(itemB)) {
return diffArrayByKey(itemA, itemB, path, patches)
}
return diffArrayByIndex(itemA, itemB, path, patches)
return isUniquelyKeyed(itemA) && isUniquelyKeyed(itemB)
? diffArrayByKey(itemA, itemB, path, patches)
: diffArrayByIndex(itemA, itemB, path, patches)

Copy link
Contributor Author

ricokahler commented Jun 13, 2025

Merge activity

  • Jun 13, 7:24 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jun 13, 7:37 PM UTC: Graphite rebased this pull request as part of a merge.
  • Jun 13, 7:39 PM UTC: @ricokahler merged this pull request with Graphite.

@ricokahler ricokahler changed the base branch from v6_sequential-patch-serialization to graphite-base/41 June 13, 2025 19:34
@ricokahler ricokahler changed the base branch from graphite-base/41 to main June 13, 2025 19:36
- Implement ReorderPatch type for handling array reordering operations
- Add sophisticated diffArrayByKey algorithm that detects reordering, insertions, and deletions
- Use two-phase reordering strategy with temporary keys to avoid key collisions
- Update InsertPatch to support position-based insertion (before/after/replace)
- Add set operations utilities with fallbacks for older JS environments
- Enhance serialization to handle complex reorder scenarios safely
- Add comprehensive test coverage for various reordering scenarios

This enables more collaborative-editing-friendly patches for arrays of keyed objects,
generating patches that are more resilient to concurrent modifications.
@ricokahler ricokahler force-pushed the v6_key-based-reordering branch from efa4722 to b9d0b98 Compare June 13, 2025 19:37
@ricokahler ricokahler merged commit 27dcdc2 into main Jun 13, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants