mattphillips / deep-object-diff

Deep diffs two objects, including nested structures of arrays and objects, and returns the difference. ❄️
https://www.npmjs.com/package/deep-object-diff
MIT License
1.05k stars 89 forks source link

Add failing test for deep ordered object comparison. #16

Closed paulirish closed 6 years ago

paulirish commented 7 years ago

I wasn't expecting the result I get from d-o-d for this comparison. Hopefully a test case is more useful to you than a straight bug report. :)

Do the expectations below seem worthwhile?

Note that L70 currently passes (woo!), however L94 and L95 both fail (boo!)

mattphillips commented 7 years ago

Hey @paulirish thanks for the failing tests! 😄

The issue you're seeing with the failing tests is because of arrays and their indexes.

If we look at what is happening when you insert into the start of an array (or any index that isn't the tail). The difference isn't that something was added at that index but rather the pre-existing value is updated and everything afterwards is updated until the last item which will now be considered as a new entry even though it was previously in the array.

I find it useful to visualise this with an object (which is all an array actually is).

const original = {
  '0': 'hello',
  '1': 'world',
};

const updated = {
  '0': 'paul',
  '1': 'hello',
  '2': 'world',
};

// a detailed diff would show:
console.log(detailedDiff(original, updated))
/*
{
  added: { '2': 'world' },
  deleted: {},
  updated: { '0': 'paul', '1': 'hello' }
}
*/

As you can see all existing properties of the data structure are being updated when we insert at the start.

This is a common problem I see when performing diffs on arrays (when building d-o-d I held the same assumption). Perhaps there is a way to determine a difference in array in this context. I think the challenge of this is when multiple changes occur (additions/updates/deletions) and potentially at nested paths in the tree.

Do you have any ideas of how this context could be derived?

mattphillips commented 6 years ago

Closing for now as this is the expected behaviour. Feel free to reopen.