deep-object-diff icon indicating copy to clipboard operation
deep-object-diff copied to clipboard

Fix the preserve array function with array of objects

Open popojargo opened this issue 5 years ago • 5 comments

Your examples with works fine execpt when the values compared in the array are objects.

For example. this will work fine:

// added (end of array)
const left = { alphabet: ['a', 'b', 'c'] };
const right = { alphabet: ['a', 'b', 'c', 'd'] };
diff(left, right);
/*
{
  alphabet: [empty, empty, empty, 'd']
}
*/

But, if you use a similar representation with objects, it fails:

const left = { alphabet: [{ a: 'a' }, { b: 'b' }, { c: 'c' }] };
const right = { alphabet: [{ a: 'a' }, { b: 'b' }, { c: 'c' }, { d: 'd' }] };

popojargo avatar Apr 20 '19 03:04 popojargo

Coverage Status

Coverage remained the same at 100.0% when pulling 0a408004c1db579ac0b702f9ded4e953db1694d9 on popojargo:fix/preserve-array into 6296889cfc0e54f08b0077460c9bf75f2febfa9f on mattphillips:master.

coveralls avatar Apr 20 '19 03:04 coveralls

@mattphillips Can you take a look a this?

popojargo avatar Apr 27 '19 18:04 popojargo

Hi, user here. Drivebying with a lightning review.

Things that made me think:

  • Could you describe in English what exactly is wrong that you're attempting to fix?

  • What exactly does this failure you mention look like? You've posted code to replicate it, but I don't know what I'm looking at, or what the output is, or what you expected the output to be.

  • Your changes only modify existing tests. Perhaps there should be a test for this case specifically?

Hopefully this is helpful :running_man:

anko avatar Apr 29 '19 14:04 anko

@anko

  1. I'm using the "preserve-array" functions that is available in "src/preserve-array.js". I'm not sure if it's officially suppported but that's what the function that I need to diff my objects.

Basically, I'm using CouchDB has a database. When updating a document, I need to do a partial update in order to update only the fields that have changed. When manipulating objects with diff, it works fine. Although, the array handling by default is not very good.

For this reason, I use preserve-array.js which keeps the array and produce a real "array difference".

So let's say I had originally this document:

{
  "children": [{"name":1}]
}

and I end up with this document:

{
  "children": [{"name":1},{"name":2}]
}

I would like the following diff to apply to the latest document:

{
  "children": [empty,{"name":2}]
}
  1. Using the "preserve-array" works fine with array of primitives (number, string). When I'm using arrays of objects, the code just crash since the lhs or rhs can be null at some point.

  2. Yes I did change the tests but the changes I made are testing the object diff. Since a lot of "test cases" were covered in a single test case, I simply added the new things to cover. I can make it more explicit if required.

popojargo avatar Apr 29 '19 15:04 popojargo

Speaking of which -- why is preserve-array in the repo when it appears unused? And it's also undocumented.

bdombro avatar Feb 09 '23 15:02 bdombro