Starcounter-Jack / JSON-Patch

Lean and mean Javascript implementation of the JSON-Patch standard (RFC 6902). Update JSON documents using delta patches.
MIT License
1.83k stars 216 forks source link

compare() is not returning any diff in case of diffrent object type #239

Open chotuchaudhary opened 5 years ago

chotuchaudhary commented 5 years ago

I am using JSON-Patch for comparing two json object. I am getting empty diff in following cases:
Case 1: jsonA = [] , jsonB = {} , result = [] jsonpatch.compare(jsonA, jsonB) returns [](empty array) Case 2: jsonA = {a:[]} , jsonB = {a:{}} , result = [] jsonpatch.compare(jsonA, jsonB) returns [](empty array)

I don't know whether it's intended behaviour or not. Can anyone clarifies?

Surg420 commented 4 years ago

Looks like a bug and it is very critical for me. my case:

compare([{}, []], [[], {}]);
// actual result: 
// []

// expected result:
// [{op: "replace", path: "/1", value: {}}, {op: "replace", path: "/0", value: []}]
Kasea commented 4 years ago

Had the same issue pop up today.

Given the following data you would expect there to be a difference between the two variables, but it's not, presumably due to the 0 key attribute in the object, which aligns with the arrays 0th entry.

const oldData = {
  value: {
    0: 123
  }
};

const newData = {
  value: [123]
};

const diff = compare(oldData, newData); // []
Starcounter-Jack commented 4 years ago

I'm so sorry for the unreasonable delay. It should now be fixed. If you can, could you please verify that the issue is now fixed?

grumpygary commented 4 years ago

Having similar issues. If I had an empty array, and then change it to an object with properties, the patch will be generated, but fail when I verify it.

original
{ prop: [] }
update
{ prop: { a: 1, b: { c: 2 } } }

Will you be releasing a new version once the fix has been verified?

joejordan commented 4 years ago

Also experiencing the same issue as @grumpygary when attempting to convert an empty array into an object.

JvJefke commented 3 years ago

I had the same issue as described by chotuchaudhary. Using the latest version on master fixed the issue, so I can verify it works.

@Starcounter-Jack When will you be able to release this fix in a new version to NPM? I noticed you already prepared the version some months ago but it isn't available on NPM yet.