diff --git a/commonjs/duplex.js b/commonjs/duplex.js index 88975c6..826f6dd 100644 --- a/commonjs/duplex.js +++ b/commonjs/duplex.js @@ -121,6 +121,17 @@ function _generate(mirror, obj, patches, path, invertible) { if (typeof obj.toJSON === "function") { obj = obj.toJSON(); } + // If one operand is an array and the other is an object, the whole subtree + // has to be replaced. Handling it here (instead of only inside the key loop + // below) also covers the case where `mirror` is an empty container, in which + // the loop would never run and no patch would be emitted. + if (Array.isArray(mirror) !== Array.isArray(obj)) { + if (invertible) { + patches.push({ op: "test", path: path, value: helpers_js_1._deepClone(mirror) }); + } + patches.push({ op: "replace", path: path, value: helpers_js_1._deepClone(obj) }); + return; + } var newKeys = helpers_js_1._objectKeys(obj); var oldKeys = helpers_js_1._objectKeys(mirror); var changed = false; @@ -144,20 +155,15 @@ function _generate(mirror, obj, patches, path, invertible) { } } } - else if (Array.isArray(mirror) === Array.isArray(obj)) { + else { + // container types match here (handled above otherwise), so a missing key + // is always a removal if (invertible) { patches.push({ op: "test", path: path + "/" + helpers_js_1.escapePathComponent(key), value: helpers_js_1._deepClone(oldVal) }); } patches.push({ op: "remove", path: path + "/" + helpers_js_1.escapePathComponent(key) }); deleted = true; // property has been deleted } - else { - if (invertible) { - patches.push({ op: "test", path: path, value: mirror }); - } - patches.push({ op: "replace", path: path, value: obj }); - changed = true; - } } if (!deleted && newKeys.length == oldKeys.length) { return; diff --git a/module/duplex.mjs b/module/duplex.mjs index d27618e..cf5f82a 100644 --- a/module/duplex.mjs +++ b/module/duplex.mjs @@ -117,6 +117,17 @@ function _generate(mirror, obj, patches, path, invertible) { if (typeof obj.toJSON === "function") { obj = obj.toJSON(); } + // If one operand is an array and the other is an object, the whole subtree + // has to be replaced. Handling it here (instead of only inside the key loop + // below) also covers the case where `mirror` is an empty container, in which + // the loop would never run and no patch would be emitted. + if (Array.isArray(mirror) !== Array.isArray(obj)) { + if (invertible) { + patches.push({ op: "test", path: path, value: _deepClone(mirror) }); + } + patches.push({ op: "replace", path: path, value: _deepClone(obj) }); + return; + } var newKeys = _objectKeys(obj); var oldKeys = _objectKeys(mirror); var changed = false; @@ -140,20 +151,15 @@ function _generate(mirror, obj, patches, path, invertible) { } } } - else if (Array.isArray(mirror) === Array.isArray(obj)) { + else { + // container types match here (handled above otherwise), so a missing key + // is always a removal if (invertible) { patches.push({ op: "test", path: path + "/" + escapePathComponent(key), value: _deepClone(oldVal) }); } patches.push({ op: "remove", path: path + "/" + escapePathComponent(key) }); deleted = true; // property has been deleted } - else { - if (invertible) { - patches.push({ op: "test", path: path, value: mirror }); - } - patches.push({ op: "replace", path: path, value: obj }); - changed = true; - } } if (!deleted && newKeys.length == oldKeys.length) { return; diff --git a/src/duplex.ts b/src/duplex.ts index 078419c..5d77ecb 100644 --- a/src/duplex.ts +++ b/src/duplex.ts @@ -149,6 +149,18 @@ function _generate(mirror, obj, patches, path, invertible) { obj = obj.toJSON(); } + // If one operand is an array and the other is an object, the whole subtree + // has to be replaced. Handling it here (instead of only inside the key loop + // below) also covers the case where `mirror` is an empty container, in which + // the loop would never run and no patch would be emitted. + if (Array.isArray(mirror) !== Array.isArray(obj)) { + if (invertible) { + patches.push({ op: "test", path, value: _deepClone(mirror) }); + } + patches.push({ op: "replace", path, value: _deepClone(obj) }); + return; + } + var newKeys = _objectKeys(obj); var oldKeys = _objectKeys(mirror); var changed = false; @@ -176,18 +188,14 @@ function _generate(mirror, obj, patches, path, invertible) { } } } - else if(Array.isArray(mirror) === Array.isArray(obj)) { + else { + // container types match here (handled above otherwise), so a missing key + // is always a removal if (invertible) { patches.push({ op: "test", path: path + "/" + escapePathComponent(key), value: _deepClone(oldVal) }); } patches.push({ op: "remove", path: path + "/" + escapePathComponent(key) }); deleted = true; // property has been deleted - } else { - if (invertible) { - patches.push({ op: "test", path, value: mirror }); - } - patches.push({ op: "replace", path, value: obj }); - changed = true; } } diff --git a/test/spec/duplexSpec.mjs b/test/spec/duplexSpec.mjs index 130389b..3ed40fd 100644 --- a/test/spec/duplexSpec.mjs +++ b/test/spec/duplexSpec.mjs @@ -1704,6 +1704,54 @@ describe('duplex', function() { ]); } }); + variantIt('Replacing an empty array root with an object should be handled well', [ + ['invertible = FALSE', false], + ['invertible = TRUE', true] + ], function (testInvertible) { + return function () { + const objA = []; + const objB = { a: 1 }; + const patches = jsonpatch.compare(objA, objB, testInvertible); + expect(patches).toEqual([ + ...insertIf(testInvertible, { + op: 'test', + path: '', + value: objA + }), + { + op: 'replace', + path: '', + value: objB + } + ]); + // patch round-trips + expect(jsonpatch.applyPatch([], patches, true).newDocument).toEqual(objB); + } + }); + variantIt('Replacing an empty object root with an array should be handled well', [ + ['invertible = FALSE', false], + ['invertible = TRUE', true] + ], function (testInvertible) { + return function () { + const objA = {}; + const objB = [1, 2]; + const patches = jsonpatch.compare(objA, objB, testInvertible); + expect(patches).toEqual([ + ...insertIf(testInvertible, { + op: 'test', + path: '', + value: objA + }), + { + op: 'replace', + path: '', + value: objB + } + ]); + // patch round-trips + expect(jsonpatch.applyPatch({}, patches, true).newDocument).toEqual(objB); + } + }); variantIt('should return an add for a property that does not exist in the first obj, without a test operation', [ ['invertible = FALSE', false], ['invertible = TRUE', true]