From 2e41ca826db0d342463b6e70c53d9e72dd6b4f0c Mon Sep 17 00:00:00 2001 From: Marcin Warpechowski Date: Thu, 6 Jun 2019 11:57:38 +0200 Subject: [PATCH 01/11] DRY generation of deep object fixture --- test/spec/proxySpec.js | 209 ++++++----------------------------------- 1 file changed, 30 insertions(+), 179 deletions(-) diff --git a/test/spec/proxySpec.js b/test/spec/proxySpec.js index a6b877f..25b4c95 100644 --- a/test/spec/proxySpec.js +++ b/test/spec/proxySpec.js @@ -24,6 +24,21 @@ function getPatchesUsingCompare(objFactory, objChanger) { return jsonpatch.compare(mirror, JSON.parse(JSON.stringify(obj))); } +function generateDeepObjectFixture() { + return { + firstName: 'Albert', + lastName: 'Einstein', + phoneNumbers: [ + { + number: '12345' + }, + { + number: '45353' + } + ] + } +} + var customMatchers = { /** * This matcher is only needed in Chrome 28 (Chrome 28 cannot successfully compare observed objects immediately after they have been changed. Chrome 30 is unaffected) @@ -88,18 +103,7 @@ describe('proxy', function() { describe('generate', function() { it('should generate replace', function() { - var obj = { - firstName: 'Albert', - lastName: 'Einstein', - phoneNumbers: [ - { - number: '12345' - }, - { - number: '45353' - } - ] - }; + const obj = generateDeepObjectFixture(); var jsonPatcherProxy = new JSONPatcherProxy(obj); var observedObj = jsonPatcherProxy.observe(true); @@ -111,18 +115,7 @@ describe('proxy', function() { var patches = jsonPatcherProxy.generate(); - var obj2 = { - firstName: 'Albert', - lastName: 'Einstein', - phoneNumbers: [ - { - number: '12345' - }, - { - number: '45353' - } - ] - }; + const obj2 = generateDeepObjectFixture(); jsonpatch.applyPatch(obj2, patches); /* iOS and Android */ @@ -212,19 +205,7 @@ describe('proxy', function() { }); it('should generate replace (double change, shallow object)', function() { - var obj = { - firstName: 'Albert', - lastName: 'Einstein', - phoneNumbers: [ - { - number: '12345' - }, - { - number: '45353' - } - ] - }; - + const obj = generateDeepObjectFixture(); var jsonPatcherProxy = new JSONPatcherProxy(obj); var observedObj = jsonPatcherProxy.observe(true); @@ -268,19 +249,7 @@ describe('proxy', function() { }); it('should generate replace (double change, deep object)', function() { - var obj = { - firstName: 'Albert', - lastName: 'Einstein', - phoneNumbers: [ - { - number: '12345' - }, - { - number: '45353' - } - ] - }; - + const obj = generateDeepObjectFixture(); var jsonPatcherProxy = new JSONPatcherProxy(obj); var observedObj = jsonPatcherProxy.observe(true); @@ -419,14 +388,7 @@ describe('proxy', function() { ]); }); it('should generate add', function() { - var obj = { - lastName: 'Einstein', - phoneNumbers: [ - { - number: '12345' - } - ] - }; + const obj = generateDeepObjectFixture(); var jsonPatcherProxy = new JSONPatcherProxy(obj); var observedObj = jsonPatcherProxy.observe(true); @@ -438,31 +400,12 @@ describe('proxy', function() { }); var patches = jsonPatcherProxy.generate(); - var obj2 = { - lastName: 'Einstein', - phoneNumbers: [ - { - number: '12345' - } - ] - }; - + var obj2 = generateDeepObjectFixture(); jsonpatch.applyPatch(obj2, patches); expect(obj2).toEqualInJson(observedObj); }); it('should generate remove', function() { - var obj = { - lastName: 'Einstein', - firstName: 'Albert', - phoneNumbers: [ - { - number: '12345' - }, - { - number: '4234' - } - ] - }; + const obj = generateDeepObjectFixture(); var jsonPatcherProxy = new JSONPatcherProxy(obj); var observedObj = jsonPatcherProxy.observe(true); @@ -489,18 +432,7 @@ describe('proxy', function() { expect(obj2).toEqualInJson(observedObj); }); it('should generate remove and disable all traps', function() { - var obj = { - lastName: 'Einstein', - firstName: 'Albert', - phoneNumbers: [ - { - number: '12345' - }, - { - number: '4234' - } - ] - }; + const obj = generateDeepObjectFixture(); var jsonPatcherProxy = new JSONPatcherProxy(obj); var observedObj = jsonPatcherProxy.observe(true); @@ -937,18 +869,7 @@ describe('proxy', function() { describe('callback', function() { it('should generate replace', function() { var obj2; - var obj = { - firstName: 'Albert', - lastName: 'Einstein', - phoneNumbers: [ - { - number: '12345' - }, - { - number: '45353' - } - ] - }; + const obj = generateDeepObjectFixture(); var jsonPatcherProxy = new JSONPatcherProxy(obj); var observedObj = jsonPatcherProxy.observe(true); @@ -959,18 +880,7 @@ describe('proxy', function() { observedObj.firstName = 'Joachim'; function objChanged(operation) { - obj2 = { - firstName: 'Albert', - lastName: 'Einstein', - phoneNumbers: [ - { - number: '12345' - }, - { - number: '45353' - } - ] - }; + obj2 = generateDeepObjectFixture(); jsonpatch.applyOperation(obj2, operation); /* iOS and Android */ @@ -984,18 +894,7 @@ describe('proxy', function() { var lastPatches, called = 0; - var obj = { - firstName: 'Albert', - lastName: 'Einstein', - phoneNumbers: [ - { - number: '12345' - }, - { - number: '45353' - } - ] - }; + const obj = generateDeepObjectFixture(); var jsonPatcherProxy = new JSONPatcherProxy(obj); var observedObj = jsonPatcherProxy.observe(true, function(patches) { @@ -1051,18 +950,7 @@ describe('proxy', function() { var lastPatches, called = 0; - var obj = { - firstName: 'Albert', - lastName: 'Einstein', - phoneNumbers: [ - { - number: '12345' - }, - { - number: '45353' - } - ] - }; + const obj = generateDeepObjectFixture(); var jsonPatcherProxy = new JSONPatcherProxy(obj); var observedObj = jsonPatcherProxy.observe(true); @@ -1200,18 +1088,7 @@ describe('proxy', function() { var lastPatches, called = 0, res; - var obj = { - firstName: 'Albert', - lastName: 'Einstein', - phoneNumbers: [ - { - number: '12345' - }, - { - number: '45353' - } - ] - }; + const obj = generateDeepObjectFixture(); var jsonPatcherProxy = new JSONPatcherProxy(obj); var observedObj = jsonPatcherProxy.observe(true, function(patches) { called++; @@ -1427,20 +1304,7 @@ describe('proxy', function() { describe('pausing and resuming', function() { it("shouldn't emit patches when paused", function() { var called = 0; - - var obj = { - firstName: 'Albert', - lastName: 'Einstein', - phoneNumbers: [ - { - number: '12345' - }, - { - number: '45353' - } - ] - }; - + const obj = generateDeepObjectFixture(); var jsonPatcherProxy = new JSONPatcherProxy(obj); var observedObj = jsonPatcherProxy.observe(true, function(patches) { called++; @@ -1460,20 +1324,7 @@ describe('proxy', function() { it('Should re-start emitting patches when paused then resumed', function() { var called = 0; - - var obj = { - firstName: 'Albert', - lastName: 'Einstein', - phoneNumbers: [ - { - number: '12345' - }, - { - number: '45353' - } - ] - }; - + const obj = generateDeepObjectFixture(); var jsonPatcherProxy = new JSONPatcherProxy(obj); var observedObj = jsonPatcherProxy.observe(true, function(patches) { called++; From fb30a53df223d38ddc9bef4f9bfdb15dcef69c8d Mon Sep 17 00:00:00 2001 From: Marcin Warpechowski Date: Thu, 6 Jun 2019 12:03:23 +0200 Subject: [PATCH 02/11] more DRYness in tests use a one liner to clone the object where possible --- test/spec/proxySpec.js | 61 ++++++++---------------------------------- 1 file changed, 11 insertions(+), 50 deletions(-) diff --git a/test/spec/proxySpec.js b/test/spec/proxySpec.js index 25b4c95..e199d44 100644 --- a/test/spec/proxySpec.js +++ b/test/spec/proxySpec.js @@ -124,7 +124,7 @@ describe('proxy', function() { expect(obj2).toReallyEqual(observedObj); }); it('should generate replace (escaped chars)', function() { - var obj = { + const obj = { '/name/first': 'Albert', '/name/last': 'Einstein', '~phone~/numbers': [ @@ -136,6 +136,7 @@ describe('proxy', function() { } ] }; + const obj2 = JSON.parse(JSON.stringify(obj)); var jsonPatcherProxy = new JSONPatcherProxy(obj); var observedObj = jsonPatcherProxy.observe(true); @@ -145,19 +146,6 @@ describe('proxy', function() { observedObj['~phone~/numbers'][1].number = '456'; var patches = jsonPatcherProxy.generate(); - var obj2 = { - '/name/first': 'Albert', - '/name/last': 'Einstein', - '~phone~/numbers': [ - { - number: '12345' - }, - { - number: '45353' - } - ] - }; - jsonpatch.applyPatch(obj2, patches); /* iOS and Android */ @@ -278,18 +266,10 @@ describe('proxy', function() { /* iOS and Android */ observedObj = JSONPatcherProxy.deepClone(observedObj); - expect(observedObj).toReallyEqual({ - firstName: 'Albert', - lastName: 'Einstein', - phoneNumbers: [ - { - number: '123' - }, - { - number: '456' - } - ] - }); //objects should be still the same + const obj2 = generateDeepObjectFixture(); + obj2.phoneNumbers[0].number = '123'; + obj2.phoneNumbers[1].number = '456'; + expect(observedObj).toReallyEqual(obj2); //objects should be still the same }); it('should generate replace (changes in new array cell, primitive values)', function() { @@ -416,18 +396,7 @@ describe('proxy', function() { var patches = jsonPatcherProxy.generate(); - var obj2 = { - lastName: 'Einstein', - firstName: 'Albert', - phoneNumbers: [ - { - number: '12345' - }, - { - number: '4234' - } - ] - }; + const obj2 = generateDeepObjectFixture(); jsonpatch.applyPatch(obj2, patches); expect(obj2).toEqualInJson(observedObj); }); @@ -981,18 +950,10 @@ describe('proxy', function() { } ]); //first patch should NOT be reported again here - expect(observedObj).toReallyEqual({ - firstName: 'Albert', - lastName: 'Einstein', - phoneNumbers: [ - { - number: '123' - }, - { - number: '456' - } - ] - }); + const obj2 = generateDeepObjectFixture(); + obj2.phoneNumbers[0].number = '123'; + obj2.phoneNumbers[1].number = '456'; + expect(observedObj).toReallyEqual(obj2); }); describe( From edb513a72a75d6f28d9a1f5240e9134155163af9 Mon Sep 17 00:00:00 2001 From: Marcin Warpechowski Date: Wed, 5 Jun 2019 22:54:20 +0200 Subject: [PATCH 03/11] refactor tree metadata access - changed _treeMetadataMap to use the unproxified version of the tree, instead of proxified - merged _parenthoodMap into _treeMetadataMap - added a new symbol _metadataSymbol used to access the metadata from the proxified object, but only if you have a reference to the symbol --- src/jsonpatcherproxy.js | 145 ++++++++++++++++++++++------------------ 1 file changed, 81 insertions(+), 64 deletions(-) diff --git a/src/jsonpatcherproxy.js b/src/jsonpatcherproxy.js index a3d9b5c..8cd7227 100644 --- a/src/jsonpatcherproxy.js +++ b/src/jsonpatcherproxy.js @@ -12,6 +12,11 @@ /** Class representing a JS Object observer */ const JSONPatcherProxy = (function() { + function isObject(value) { + const type = typeof value + return value != null && (type == 'object' || type == 'function') + } + /** * Deep clones your object and returns a new object. */ @@ -34,17 +39,17 @@ const JSONPatcherProxy = (function() { JSONPatcherProxy.escapePathComponent = escapePathComponent; /** - * Walk up the parenthood tree to get the path + * Walk up the tree metadata to get the path * @param {JSONPatcherProxy} instance * @param {Object} tree the object you need to find its path */ function getPathToTree(instance, tree) { const pathComponents = []; - let parenthood = instance._parenthoodMap.get(tree); - while (parenthood && parenthood.key) { + let treeMetadata = instance._treeMetadataMap.get(tree); + while (treeMetadata && treeMetadata.keyInParent) { // because we're walking up-tree, we need to use the array as a stack - pathComponents.unshift(parenthood.key); - parenthood = instance._parenthoodMap.get(parenthood.parent); + pathComponents.unshift(treeMetadata.keyInParent); + treeMetadata = instance._treeMetadataMap.get(treeMetadata.parent); } if (pathComponents.length) { const path = pathComponents.join('/'); @@ -52,9 +57,23 @@ const JSONPatcherProxy = (function() { } return ''; } + /** + * A callback to be used as the proxy get trap callback. + * It allows to check get the tree metadata from within the tree. This allows to check if the object is proxified by this + * instance of JSONPatcherProxy, even when all you have is a reference that is someone else's proxy + * @param {JSONPatcherProxy} instance JSONPatcherProxy instance + * @param {Object} tree the affected object + * @param {String} key the effect property's name + */ + function trapForGet(instance, tree, key) { + if (key === instance._metadataSymbol) { + return instance._treeMetadataMap.get(tree); + } + return Reflect.get(tree, key); + } /** * A callback to be used as the proxy set trap callback. - * It updates parenthood map if needed, proxifies nested newly-added objects, calls default callback with the changes occurred. + * It updates tree metadata map if needed, proxifies nested newly-added objects, calls default callback with the changes occurred. * @param {JSONPatcherProxy} instance JSONPatcherProxy instance * @param {Object} tree the affected object * @param {String} key the effect property's name @@ -62,45 +81,43 @@ const JSONPatcherProxy = (function() { */ function trapForSet(instance, tree, key, newValue) { const pathToKey = getPathToTree(instance, tree) + '/' + escapePathComponent(key); - const subtreeMetadata = instance._treeMetadataMap.get(newValue); - - if (instance._treeMetadataMap.has(newValue)) { - instance._parenthoodMap.set(subtreeMetadata.originalObject, { parent: tree, key }); - } - /* - mark already proxified values as inherited. - rationale: proxy.arr.shift() - will emit - {op: replace, path: '/arr/1', value: arr_2} - {op: remove, path: '/arr/2'} - - by default, the second operation would revoke the proxy, and this renders arr revoked. - That's why we need to remember the proxies that are inherited. - */ - /* - Why do we need to check instance._isProxifyingTreeNow? - - We need to make sure we mark revocables as inherited ONLY when we're observing, - because throughout the first proxification, a sub-object is proxified and then assigned to - its parent object. This assignment of a pre-proxified object can fool us into thinking - that it's a proxified object moved around, while in fact it's the first assignment ever. - - Checking _isProxifyingTreeNow ensures this is not happening in the first proxification, - but in fact is is a proxified object moved around the tree - */ - if (subtreeMetadata && !instance._isProxifyingTreeNow) { - subtreeMetadata.inherited = true; + + if (isObject(newValue)) { + const subtreeMetadata = newValue[instance._metadataSymbol]; + if (subtreeMetadata) { + subtreeMetadata.parent = tree; + subtreeMetadata.keyInParent = key; + /* + mark already proxified values as inherited. + rationale: proxy.arr.shift() + will emit + {op: replace, path: '/arr/1', value: arr_2} + {op: remove, path: '/arr/2'} + + by default, the second operation would revoke the proxy, and this renders arr revoked. + That's why we need to remember the proxies that are inherited. + */ + /* + Why do we need to check instance._isProxifyingTreeNow? + + We need to make sure we mark revocables as inherited ONLY when we're observing, + because throughout the first proxification, a sub-object is proxified and then assigned to + its parent object. This assignment of a pre-proxified object can fool us into thinking + that it's a proxified object moved around, while in fact it's the first assignment ever. + + Checking _isProxifyingTreeNow ensures this is not happening in the first proxification, + but in fact is is a proxified object moved around the tree + */ + if (!instance._isProxifyingTreeNow) { + subtreeMetadata.inherited = true; + } + } + else { + // make sure to watch it + newValue = instance._proxifyTreeRecursively(tree, newValue, key); + } } - // if the new value is an object, make sure to watch it - if ( - newValue && - typeof newValue == 'object' && - !instance._treeMetadataMap.has(newValue) - ) { - instance._parenthoodMap.set(newValue, { parent: tree, key }); - newValue = instance._proxifyTreeRecursively(tree, newValue, key); - } // let's start with this operation, and may or may not update it later const operation = { op: 'remove', @@ -118,12 +135,10 @@ const JSONPatcherProxy = (function() { // undefined array elements are JSON.stringified to `null` (operation.op = 'replace'), (operation.value = null); } - const oldSubtreeMetadata = instance._treeMetadataMap.get(tree[key]); + const oldSubtreeMetadata = tree[key][instance._metadataSymbol]; if (oldSubtreeMetadata) { - //TODO there is no test for this! - instance._parenthoodMap.delete(tree[key]); instance._disableTrapsForTreeMetadata(oldSubtreeMetadata); - instance._treeMetadataMap.delete(oldSubtreeMetadata); + instance._treeMetadataMap.delete(oldSubtreeMetadata.originalObject); //TODO there is no test for this } } } else { @@ -148,7 +163,7 @@ const JSONPatcherProxy = (function() { } /** * A callback to be used as the proxy delete trap callback. - * It updates parenthood map if needed, calls default callbacks with the changes occurred. + * It updates tree metadata map if needed, calls default callbacks with the changes occurred. * @param {JSONPatcherProxy} instance JSONPatcherProxy instance * @param {Object} tree the effected object * @param {String} key the effected property's name @@ -156,7 +171,7 @@ const JSONPatcherProxy = (function() { function trapForDeleteProperty(instance, tree, key) { if (typeof tree[key] !== 'undefined') { const pathToKey = getPathToTree(instance, tree) + '/' + escapePathComponent(key); - const subtreeMetadata = instance._treeMetadataMap.get(tree[key]); + const subtreeMetadata = tree[key][instance._metadataSymbol]; if (subtreeMetadata) { if (subtreeMetadata.inherited) { @@ -170,9 +185,8 @@ const JSONPatcherProxy = (function() { */ subtreeMetadata.inherited = false; } else { - instance._parenthoodMap.delete(subtreeMetadata.originalObject); instance._disableTrapsForTreeMetadata(subtreeMetadata); - instance._treeMetadataMap.delete(tree[key]); + instance._treeMetadataMap.delete(subtreeMetadata.originalObject); //TODO this is not tested } } const reflectionResult = Reflect.deleteProperty(tree, key); @@ -193,10 +207,13 @@ const JSONPatcherProxy = (function() { * @constructor */ function JSONPatcherProxy(root, showDetachedWarning) { + /** + * Use tree[this._metadataSymbol] to access the tree metadata when all you have is a reference to a proxified version of the tree. + */ + this._metadataSymbol = Symbol("Symbol for getting the tree metadata from external access."); this._isProxifyingTreeNow = false; this._isObserving = false; this._treeMetadataMap = new Map(); - this._parenthoodMap = new Map(); // default to true if (typeof showDetachedWarning !== 'boolean') { showDetachedWarning = true; @@ -216,26 +233,26 @@ const JSONPatcherProxy = (function() { return tree; } const handler = { + get: (...args) => trapForGet(this, ...args), set: (...args) => trapForSet(this, ...args), deleteProperty: (...args) => trapForDeleteProperty(this, ...args) }; const treeMetadata = Proxy.revocable(tree, handler); // cache the object that contains traps to disable them later. - treeMetadata.handler = handler; treeMetadata.originalObject = tree; - - /* keeping track of the object's parent and the key within the parent */ - this._parenthoodMap.set(tree, { parent, key }); + treeMetadata.handler = handler; + treeMetadata.parent = parent; + treeMetadata.keyInParent = key; /* keeping track of all the proxies to be able to revoke them later */ - this._treeMetadataMap.set(treeMetadata.proxy, treeMetadata); + this._treeMetadataMap.set(tree, treeMetadata); //the key is an UNPROXIFIED tree return treeMetadata.proxy; }; // grab tree's leaves one by one, encapsulate them into a proxy and return JSONPatcherProxy.prototype._proxifyTreeRecursively = function(parent, tree, key) { - for (let key in tree) { + for (let key in tree) { //TODO this creates a new local "key" that is different to "key" argument of the function. The name of the function argument should be changed to avoid confusion if (tree.hasOwnProperty(key)) { - if (tree[key] instanceof Object) { + if (isObject(tree[key])) { tree[key] = this._proxifyTreeRecursively( tree, tree[key], @@ -256,7 +273,7 @@ const JSONPatcherProxy = (function() { initial process; */ this.pause(); - this._isProxifyingTreeNow = true; + this._isProxifyingTreeNow = true; //TODO probably not needed, when I comment this out, all tests pass const proxifiedRoot = this._proxifyTreeRecursively( undefined, root, @@ -276,13 +293,12 @@ const JSONPatcherProxy = (function() { const message = "You're accessing an object that is detached from the observedObject tree, see https://github.com/Palindrom/JSONPatcherProxy#detached-objects"; - treeMetadata.handler.set = ( + treeMetadata.handler.get = ( parent, - key, - newValue + key ) => { console.warn(message); - return Reflect.set(parent, key, newValue); + return Reflect.get(parent, key); }; treeMetadata.handler.set = ( parent, @@ -348,6 +364,7 @@ const JSONPatcherProxy = (function() { JSONPatcherProxy.prototype.disableTraps = function() { this._treeMetadataMap.forEach(this._disableTrapsForTreeMetadata, this); }; + /** * Restores callback back to the original one provided to `observe`. */ From c461b492a4fa2e2869ba99cbe0329db3846e93cd Mon Sep 17 00:00:00 2001 From: Marcin Warpechowski Date: Thu, 6 Jun 2019 12:25:08 +0200 Subject: [PATCH 04/11] handle external proxification (#53) skip generation of a replace operation, if an object within the tree is replaced with a proxified version of itself by external code --- src/jsonpatcherproxy.js | 4 ++++ test/spec/proxySpec.js | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/jsonpatcherproxy.js b/src/jsonpatcherproxy.js index 8cd7227..a1a8539 100644 --- a/src/jsonpatcherproxy.js +++ b/src/jsonpatcherproxy.js @@ -85,6 +85,10 @@ const JSONPatcherProxy = (function() { if (isObject(newValue)) { const subtreeMetadata = newValue[instance._metadataSymbol]; if (subtreeMetadata) { + if(subtreeMetadata.parent === tree && subtreeMetadata.keyInParent === key) { + //this is the same object that we already proxified, proxified now by someone else. In this case, remain silent + return Reflect.set(tree, key, newValue); + } subtreeMetadata.parent = tree; subtreeMetadata.keyInParent = key; /* diff --git a/test/spec/proxySpec.js b/test/spec/proxySpec.js index e199d44..5263ccb 100644 --- a/test/spec/proxySpec.js +++ b/test/spec/proxySpec.js @@ -271,6 +271,30 @@ describe('proxy', function() { obj2.phoneNumbers[1].number = '456'; expect(observedObj).toReallyEqual(obj2); //objects should be still the same }); + + it('should generate replace (deep object, proxified)', function() { + const obj = generateDeepObjectFixture(); + const jsonPatcherProxy = new JSONPatcherProxy(obj); + let observedObj = jsonPatcherProxy.observe(true); + + //begin external proxification + observedObj = new Proxy(observedObj, {}); + observedObj.phoneNumbers = new Proxy(observedObj.phoneNumbers, {}); + observedObj.phoneNumbers[0] = new Proxy(observedObj.phoneNumbers[0], {}); + observedObj.phoneNumbers[1] = new Proxy(observedObj.phoneNumbers[1], {}); + //end external proxification + + observedObj.phoneNumbers[0].number = '123'; + + const patches = jsonPatcherProxy.generate(); + expect(patches).toReallyEqual([ + { + op: 'replace', + path: '/phoneNumbers/0/number', + value: '123' + } + ]); + }); it('should generate replace (changes in new array cell, primitive values)', function() { var arr = [1]; From 3678f4719d89b2e1972ed65887dfde670cdcc77a Mon Sep 17 00:00:00 2001 From: Marcin Warpechowski Date: Thu, 6 Jun 2019 16:17:06 +0200 Subject: [PATCH 05/11] rewrite the comment that explains how the fix for #35 works --- src/jsonpatcherproxy.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/jsonpatcherproxy.js b/src/jsonpatcherproxy.js index a1a8539..447c04a 100644 --- a/src/jsonpatcherproxy.js +++ b/src/jsonpatcherproxy.js @@ -86,7 +86,11 @@ const JSONPatcherProxy = (function() { const subtreeMetadata = newValue[instance._metadataSymbol]; if (subtreeMetadata) { if(subtreeMetadata.parent === tree && subtreeMetadata.keyInParent === key) { - //this is the same object that we already proxified, proxified now by someone else. In this case, remain silent + /* + This is an object that is already proxified by this instance of JSONPatcherProxy, + which is now being proxified by some external code or simply reassigned at the same place. + In this case, remain silent. + */ return Reflect.set(tree, key, newValue); } subtreeMetadata.parent = tree; From bcde61c41f9ebe5487fe4a5c712bb2d911a064e9 Mon Sep 17 00:00:00 2001 From: Marcin Warpechowski Date: Thu, 6 Jun 2019 16:46:11 +0200 Subject: [PATCH 06/11] breaking change in benchmark: DRY fixtures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Warning, this commit changes the performance of the benchmark. Generation of tested object is now 2x faster. This is good for the benchmark, because cheaper object generation allows the benchmark to be more sensitive in catching the differences in the performance of JSONPatcherProxy. Just remember that the results from before and after this commit are not comparable. Benchmark results before this commit ============================= jsonpatcherproxy generate operation Ops/sec: 132404 ±2.55% Ran 8276 times in 0.063 seconds. jsonpatch generate operation Ops/sec: 106585 ±6.73% Ran 6434 times in 0.060 seconds. jsonpatcherproxy mutation - huge object Ops/sec: 906 ±2.45% Ran 51 times in 0.056 seconds. jsonpatch mutation - huge object Ops/sec: 862 ±1.52% Ran 47 times in 0.055 seconds. jsonpatcherproxy generate operation - huge object Ops/sec: 811 ±1.99% Ran 46 times in 0.057 seconds. jsonpatch generate operation - huge object Ops/sec: 847 ±1.20% Ran 47 times in 0.055 seconds. PROXIFY big object Ops/sec: 1813 ±2.00% Ran 102 times in 0.056 seconds. DIRTY-OBSERVE big object Ops/sec: 1625 ±1.48% Ran 91 times in 0.056 seconds. Benchmark results after this commit ============================= jsonpatcherproxy generate operation Ops/sec: 136915 ±2.46% Ran 8566 times in 0.063 seconds. jsonpatch generate operation Ops/sec: 104113 ±7.19% Ran 6330 times in 0.061 seconds. jsonpatcherproxy mutation - huge object Ops/sec: 1960 ±3.53% Ran 130 times in 0.066 seconds. jsonpatch mutation - huge object Ops/sec: 1607 ±1.84% Ran 88 times in 0.055 seconds. jsonpatcherproxy generate operation - huge object Ops/sec: 1562 ±3.40% Ran 101 times in 0.065 seconds. jsonpatch generate operation - huge object Ops/sec: 1620 ±2.04% Ran 90 times in 0.056 seconds. PROXIFY big object Ops/sec: 3968 ±4.07% Ran 294 times in 0.074 seconds. DIRTY-OBSERVE big object Ops/sec: 3285 ±1.83% Ran 180 times in 0.055 seconds. --- test/spec/proxyBenchmark.js | 202 +++++++++--------------------------- 1 file changed, 51 insertions(+), 151 deletions(-) diff --git a/test/spec/proxyBenchmark.js b/test/spec/proxyBenchmark.js index 361034b..6b9a0cc 100644 --- a/test/spec/proxyBenchmark.js +++ b/test/spec/proxyBenchmark.js @@ -1,9 +1,7 @@ -var obj, obj2; - if (typeof window === 'undefined') { const jsdom = require("jsdom"); const { JSDOM } = jsdom; - var dom = new JSDOM(); + const dom = new JSDOM(); global.window = dom.window; global.document = dom.window.document; } @@ -22,10 +20,10 @@ if (typeof Benchmark === 'undefined') { .benchmarkResultsToConsole; } -var suite = new Benchmark.Suite(); +const suite = new Benchmark.Suite(); -suite.add('jsonpatcherproxy generate operation', function() { - var obj = { +function generateDeepObjectFixture() { + return { firstName: 'Albert', lastName: 'Einstein', phoneNumbers: [ @@ -36,187 +34,89 @@ suite.add('jsonpatcherproxy generate operation', function() { number: '45353' } ] - }; - - var jsonPatcherProxy = new JSONPatcherProxy(obj); - var observedObj = jsonPatcherProxy.observe(true); + } +} - var patches = jsonPatcherProxy.generate(); +function generateSmallObjectFixture() { + return { name: 'Tesla', speed: 100 }; +} - observedObj.firstName = 'Joachim'; - observedObj.lastName = 'Wester'; - observedObj.phoneNumbers[0].number = '123'; - observedObj.phoneNumbers[1].number = '456'; -}); -suite.add('jsonpatch generate operation', function() { - var observedObj = { +function generateBigObjectFixture(carsSize) { + const obj = { firstName: 'Albert', lastName: 'Einstein', - phoneNumbers: [ - { - number: '12345' - }, - { - number: '45353' - } - ] + cars: [] }; - var observer = jsonpatch.observe(observedObj); + for (let i = 0; i < carsSize; i++) { + let deep = generateSmallObjectFixture(); + obj.cars.push(deep); + for (let j = 0; j < 5; j++) { + deep.temp = generateSmallObjectFixture(); + deep = deep.temp; + } + } + return obj; +} +suite.add('jsonpatcherproxy generate operation', function() { + const obj = generateDeepObjectFixture(); + const jsonPatcherProxy = new JSONPatcherProxy(obj); + const observedObj = jsonPatcherProxy.observe(true); + const patches = jsonPatcherProxy.generate(); observedObj.firstName = 'Joachim'; observedObj.lastName = 'Wester'; observedObj.phoneNumbers[0].number = '123'; observedObj.phoneNumbers[1].number = '456'; +}); +suite.add('jsonpatch generate operation', function() { + const obj = generateDeepObjectFixture(); + const observer = jsonpatch.observe(obj); + obj.firstName = 'Joachim'; + obj.lastName = 'Wester'; + obj.phoneNumbers[0].number = '123'; + obj.phoneNumbers[1].number = '456'; jsonpatch.generate(observer); }); - - - - suite.add('jsonpatcherproxy mutation - huge object', function() { - var singleCar = { name: 'Tesla', speed: 100 }; - - var obj = { - firstName: 'Albert', - lastName: 'Einstein', - cars: [] - }; - - for (var i = 0; i < 100; i++) { - var copy = JSONPatcherProxy.deepClone(singleCar); - var temp = copy; - for (var j = 0; j < 5; j++) { - temp.temp = JSONPatcherProxy.deepClone(singleCar); - temp = temp.temp; - } - obj.cars.push(copy); - } - var jsonPatcherProxy = new JSONPatcherProxy(obj); - var observedObj = jsonPatcherProxy.observe(true); - + const obj = generateBigObjectFixture(100); + const jsonPatcherProxy = new JSONPatcherProxy(obj); + const observedObj = jsonPatcherProxy.observe(true); observedObj.cars[50].name = 'Toyota' }); suite.add('jsonpatch mutation - huge object', function() { - var singleCar = { name: 'Tesla', speed: 100 }; - - var obj = { - firstName: 'Albert', - lastName: 'Einstein', - cars: [] - }; - - for (var i = 0; i < 100; i++) { - var copy = JSONPatcherProxy.deepClone(singleCar); - var temp = copy; - for (var j = 0; j < 5; j++) { - temp.temp = JSONPatcherProxy.deepClone(singleCar); - temp = temp.temp; - } - obj.cars.push(copy); - } - - var observer = jsonpatch.observe(obj); - + const obj = generateBigObjectFixture(100); + const observer = jsonpatch.observe(obj); obj.cars[50].name = 'Toyota'; - jsonpatch.generate(observer); }); suite.add('jsonpatcherproxy generate operation - huge object', function() { - var singleCar = { name: 'Tesla', speed: 100 }; - - var obj = { - firstName: 'Albert', - lastName: 'Einstein', - cars: [] - }; - - for (var i = 0; i < 100; i++) { - var copy = JSONPatcherProxy.deepClone(singleCar); - var temp = copy; - for (var j = 0; j < 5; j++) { - temp.temp = JSONPatcherProxy.deepClone(singleCar); - temp = temp.temp; - } - obj.cars.push(copy); - } - var jsonPatcherProxy = new JSONPatcherProxy(obj); - var observedObj = jsonPatcherProxy.observe(true); - + const obj = generateBigObjectFixture(100); + const jsonPatcherProxy = new JSONPatcherProxy(obj); + const observedObj = jsonPatcherProxy.observe(true); observedObj.cars.shift(); }); suite.add('jsonpatch generate operation - huge object', function() { - var singleCar = { name: 'Tesla', speed: 100 }; - - var obj = { - firstName: 'Albert', - lastName: 'Einstein', - cars: [] - }; - - for (var i = 0; i < 100; i++) { - var copy = JSONPatcherProxy.deepClone(singleCar); - var temp = copy; - for (var j = 0; j < 5; j++) { - temp.temp = JSONPatcherProxy.deepClone(singleCar); - temp = temp.temp; - } - obj.cars.push(copy); - } - - var observer = jsonpatch.observe(obj); - + const obj = generateBigObjectFixture(100); + const observer = jsonpatch.observe(obj); obj.cars.shift(); - jsonpatch.generate(observer); }); suite.add('PROXIFY big object', function() { - var singleCar = { name: 'Tesla', speed: 100 }; - - var obj = { - firstName: 'Albert', - lastName: 'Einstein', - cars: [] - }; - for (var i = 0; i < 50; i++) { - var copy = JSONPatcherProxy.deepClone(singleCar); - var temp = copy; - for (var j = 0; j < 5; j++) { - temp.temp = JSONPatcherProxy.deepClone(singleCar); - temp = temp.temp; - } - obj.cars.push(copy); - } - - var jsonPatcherProxy = new JSONPatcherProxy(obj); - - var observedObj = jsonPatcherProxy.observe(true); + const obj = generateBigObjectFixture(50); + const jsonPatcherProxy = new JSONPatcherProxy(obj); + const observedObj = jsonPatcherProxy.observe(true); observedObj.a = 1; }); suite.add('DIRTY-OBSERVE big object', function() { - var singleCar = { name: 'Tesla', speed: 100 }; - - var obj = { - firstName: 'Albert', - lastName: 'Einstein', - cars: [] - }; - for (var i = 0; i < 50; i++) { - var copy = JSONPatcherProxy.deepClone(singleCar); - var temp = copy; - for (var j = 0; j < 5; j++) { - temp.temp = JSONPatcherProxy.deepClone(singleCar); - temp = temp.temp; - } - obj.cars.push(copy); - } - var observer = jsonpatch.observe(obj); + const obj = generateBigObjectFixture(50); + const observer = jsonpatch.observe(obj); obj.a = 1; jsonpatch.generate(observer); @@ -230,4 +130,4 @@ if (typeof benchmarkReporter !== 'undefined') { benchmarkResultsToConsole(suite); }); suite.run(); -} +} \ No newline at end of file From 67cc6f570e275fcb4e1c9a886eb9977e3421bf83 Mon Sep 17 00:00:00 2001 From: Marcin Warpechowski Date: Thu, 6 Jun 2019 17:48:49 +0200 Subject: [PATCH 07/11] rewrite the benchmark the benchmark methods are almost exactly the same, but I have made the following changes: - group benchmarks by the topic - give more meaningful titles - wherever the preparation code is not meaningful for the benchmark, move it outside the benchmark - use code blocks to give scope to preparation variables this is a change that makes the benchmark results incomparable with the previous versions. however, it will work if you copy this file onto an older version and run it. --- test/spec/proxyBenchmark.js | 221 +++++++++++++++++++++++------------- 1 file changed, 145 insertions(+), 76 deletions(-) diff --git a/test/spec/proxyBenchmark.js b/test/spec/proxyBenchmark.js index 6b9a0cc..d635180 100644 --- a/test/spec/proxyBenchmark.js +++ b/test/spec/proxyBenchmark.js @@ -22,21 +22,6 @@ if (typeof Benchmark === 'undefined') { const suite = new Benchmark.Suite(); -function generateDeepObjectFixture() { - return { - firstName: 'Albert', - lastName: 'Einstein', - phoneNumbers: [ - { - number: '12345' - }, - { - number: '45353' - } - ] - } -} - function generateSmallObjectFixture() { return { name: 'Tesla', speed: 100 }; } @@ -58,69 +43,153 @@ function generateBigObjectFixture(carsSize) { return obj; } -suite.add('jsonpatcherproxy generate operation', function() { - const obj = generateDeepObjectFixture(); - const jsonPatcherProxy = new JSONPatcherProxy(obj); - const observedObj = jsonPatcherProxy.observe(true); - const patches = jsonPatcherProxy.generate(); - observedObj.firstName = 'Joachim'; - observedObj.lastName = 'Wester'; - observedObj.phoneNumbers[0].number = '123'; - observedObj.phoneNumbers[1].number = '456'; -}); - -suite.add('jsonpatch generate operation', function() { - const obj = generateDeepObjectFixture(); - const observer = jsonpatch.observe(obj); +function modifyObj(obj) { obj.firstName = 'Joachim'; obj.lastName = 'Wester'; - obj.phoneNumbers[0].number = '123'; - obj.phoneNumbers[1].number = '456'; - jsonpatch.generate(observer); -}); - -suite.add('jsonpatcherproxy mutation - huge object', function() { - const obj = generateBigObjectFixture(100); - const jsonPatcherProxy = new JSONPatcherProxy(obj); - const observedObj = jsonPatcherProxy.observe(true); - observedObj.cars[50].name = 'Toyota' -}); - -suite.add('jsonpatch mutation - huge object', function() { - const obj = generateBigObjectFixture(100); - const observer = jsonpatch.observe(obj); - obj.cars[50].name = 'Toyota'; - jsonpatch.generate(observer); -}); - -suite.add('jsonpatcherproxy generate operation - huge object', function() { - const obj = generateBigObjectFixture(100); - const jsonPatcherProxy = new JSONPatcherProxy(obj); - const observedObj = jsonPatcherProxy.observe(true); - observedObj.cars.shift(); -}); - -suite.add('jsonpatch generate operation - huge object', function() { - const obj = generateBigObjectFixture(100); - const observer = jsonpatch.observe(obj); - obj.cars.shift(); - jsonpatch.generate(observer); -}); - -suite.add('PROXIFY big object', function() { - const obj = generateBigObjectFixture(50); - const jsonPatcherProxy = new JSONPatcherProxy(obj); - const observedObj = jsonPatcherProxy.observe(true); - observedObj.a = 1; -}); - -suite.add('DIRTY-OBSERVE big object', function() { - const obj = generateBigObjectFixture(50); - const observer = jsonpatch.observe(obj); - obj.a = 1; - jsonpatch.generate(observer); - -}); + obj.cars[0].speed = 123; + obj.cars[0].temp.speed = 456; +} + +function reverseString(str) { + return str.split("").reverse().join(""); +} + +/* ============================= */ + +{ + const suiteName = 'Observe and generate, small object'; + + { + suite.add(`${suiteName} (noop)`, function() { + const obj = generateBigObjectFixture(1); + modifyObj(obj); + }); + } + + { + suite.add(`${suiteName} (JSONPatcherProxy)`, function() { + const obj = generateBigObjectFixture(1); + const jsonPatcherProxy = new JSONPatcherProxy(obj); + const observedObj = jsonPatcherProxy.observe(true); + const patches = jsonPatcherProxy.generate(); + modifyObj(observedObj); + }); + } + + { + suite.add(`${suiteName} (fast-json-patch)`, function() { + const obj = generateBigObjectFixture(1); + const observer = jsonpatch.observe(obj); + modifyObj(obj); + jsonpatch.generate(observer); + }); + } +} + +/* ============================= */ + +{ + const suiteName = 'Observe and generate'; + + { + suite.add(`${suiteName} (noop)`, function() { + const obj = generateBigObjectFixture(100); + modifyObj(obj); + }); + } + + { + suite.add(`${suiteName} (JSONPatcherProxy)`, function() { + const obj = generateBigObjectFixture(100); + const jsonPatcherProxy = new JSONPatcherProxy(obj); + const observedObj = jsonPatcherProxy.observe(true); + const patches = jsonPatcherProxy.generate(); + modifyObj(observedObj); + }); + } + + { + suite.add(`${suiteName} (fast-json-patch)`, function() { + const obj = generateBigObjectFixture(100); + const observer = jsonpatch.observe(obj); + modifyObj(obj); + jsonpatch.generate(observer); + }); + } +} + +/* ============================= */ + +{ + const suiteName = 'Primitive mutation'; + + { + const obj = generateBigObjectFixture(100); + + suite.add(`${suiteName} (noop)`, function() { + obj.cars[50].name = reverseString(obj.cars[50].name); + }); + } + + { + const obj = generateBigObjectFixture(100); + const jsonPatcherProxy = new JSONPatcherProxy(obj); + const observedObj = jsonPatcherProxy.observe(true); + + suite.add(`${suiteName} (JSONPatcherProxy)`, function() { + observedObj.cars[50].name = reverseString(observedObj.cars[50].name); + }); + } + + { + const obj = generateBigObjectFixture(100); + const observer = jsonpatch.observe(obj); + + suite.add(`${suiteName} (fast-json-patch)`, function() { + obj.cars[50].name = reverseString(obj.cars[50].name); + jsonpatch.generate(observer); + }); + } +} + +/* ============================= */ + +{ + const suiteName = 'Complex mutation'; + + { + const obj = generateBigObjectFixture(100); + + suite.add(`${suiteName} (noop)`, function() { + const item = obj.cars.shift(); + obj.cars.push(item); + }); + } + + { + const obj = generateBigObjectFixture(100); + const jsonPatcherProxy = new JSONPatcherProxy(obj); + const observedObj = jsonPatcherProxy.observe(true); + + suite.add(`${suiteName} (JSONPatcherProxy)`, function() { + const item = observedObj.cars.shift(); + observedObj.cars.push(item); + }); + } + + { + const obj = generateBigObjectFixture(100); + const observer = jsonpatch.observe(obj); + + suite.add(`${suiteName} (fast-json-patch)`, function() { + const item = obj.cars.shift(); + obj.cars.push(item); + jsonpatch.generate(observer); + }); + } +} + +/* ============================= */ // if we are in the browser with benchmark < 2.1.2 if (typeof benchmarkReporter !== 'undefined') { From 6fd48b60f2416831f32ada1fbd813c892e844509 Mon Sep 17 00:00:00 2001 From: Marcin Warpechowski Date: Thu, 6 Jun 2019 17:53:50 +0200 Subject: [PATCH 08/11] add benchmark for getting data from the observed object as requested in https://github.com/Palindrom/JSONPatcherProxy/pull/39#discussion_r291165035 the benchmark simply serializes the whole object, which triggers the "get" trap on every subtree --- test/spec/proxyBenchmark.js | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/test/spec/proxyBenchmark.js b/test/spec/proxyBenchmark.js index d635180..83634be 100644 --- a/test/spec/proxyBenchmark.js +++ b/test/spec/proxyBenchmark.js @@ -191,6 +191,39 @@ function reverseString(str) { /* ============================= */ +{ + const suiteName = 'Serialization'; + + { + const obj = generateBigObjectFixture(100); + + suite.add(`${suiteName} (noop)`, function() { + JSON.stringify(obj); + }); + } + + { + const obj = generateBigObjectFixture(100); + const jsonPatcherProxy = new JSONPatcherProxy(obj); + const observedObj = jsonPatcherProxy.observe(true); + + suite.add(`${suiteName} (JSONPatcherProxy)`, function() { + JSON.stringify(observedObj); + }); + } + + { + const obj = generateBigObjectFixture(100); + const observer = jsonpatch.observe(obj); + + suite.add(`${suiteName} (fast-json-patch)`, function() { + JSON.stringify(obj); + }); + } +} + +/* ============================= */ + // if we are in the browser with benchmark < 2.1.2 if (typeof benchmarkReporter !== 'undefined') { benchmarkReporter(suite); From 8abd5b6031e15358081a630ac76778b98703f61e Mon Sep 17 00:00:00 2001 From: Marcin Warpechowski Date: Thu, 6 Jun 2019 18:20:15 +0200 Subject: [PATCH 09/11] add CLI argument to include comparison with no-op and fast-json-patch by default such comparison will be excluded. Benefits of this: - faster execution - shorter report to copy to PRs --- package.json | 1 + test/spec/proxyBenchmark.js | 34 ++++++++++++++++++++++++---------- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index 694df42..44dbf8e 100644 --- a/package.json +++ b/package.json @@ -8,6 +8,7 @@ "build": "webpack", "build-watch": "webpack --watch", "bench": "node test/spec/proxyBenchmark.js", + "bench-compare": "node test/spec/proxyBenchmark.js --compare", "test": "jasmine DUPLEX=proxy JASMINE_CONFIG_PATH=test/jasmine.json", "test-debug": "node --inspect-brk node_modules/jasmine/bin/jasmine.js DUPLEX=proxy JASMINE_CONFIG_PATH=test/jasmine.json", "version": "webpack && git add -A" diff --git a/test/spec/proxyBenchmark.js b/test/spec/proxyBenchmark.js index 83634be..5bc1417 100644 --- a/test/spec/proxyBenchmark.js +++ b/test/spec/proxyBenchmark.js @@ -1,9 +1,23 @@ +/* + To run with comparisons: + $ npm run bench-compare + + To run without comparisons: + $ npm run bench + */ + +let includeComparisons = true; + if (typeof window === 'undefined') { const jsdom = require("jsdom"); const { JSDOM } = jsdom; const dom = new JSDOM(); global.window = dom.window; global.document = dom.window.document; + + if (!process.argv.includes("--compare")) { + includeComparisons = false; + } } if (typeof jsonpatch === 'undefined') { @@ -59,7 +73,7 @@ function reverseString(str) { { const suiteName = 'Observe and generate, small object'; - { + if (includeComparisons) { suite.add(`${suiteName} (noop)`, function() { const obj = generateBigObjectFixture(1); modifyObj(obj); @@ -76,7 +90,7 @@ function reverseString(str) { }); } - { + if (includeComparisons) { suite.add(`${suiteName} (fast-json-patch)`, function() { const obj = generateBigObjectFixture(1); const observer = jsonpatch.observe(obj); @@ -91,7 +105,7 @@ function reverseString(str) { { const suiteName = 'Observe and generate'; - { + if (includeComparisons) { suite.add(`${suiteName} (noop)`, function() { const obj = generateBigObjectFixture(100); modifyObj(obj); @@ -108,7 +122,7 @@ function reverseString(str) { }); } - { + if (includeComparisons) { suite.add(`${suiteName} (fast-json-patch)`, function() { const obj = generateBigObjectFixture(100); const observer = jsonpatch.observe(obj); @@ -123,7 +137,7 @@ function reverseString(str) { { const suiteName = 'Primitive mutation'; - { + if (includeComparisons) { const obj = generateBigObjectFixture(100); suite.add(`${suiteName} (noop)`, function() { @@ -141,7 +155,7 @@ function reverseString(str) { }); } - { + if (includeComparisons) { const obj = generateBigObjectFixture(100); const observer = jsonpatch.observe(obj); @@ -157,7 +171,7 @@ function reverseString(str) { { const suiteName = 'Complex mutation'; - { + if (includeComparisons) { const obj = generateBigObjectFixture(100); suite.add(`${suiteName} (noop)`, function() { @@ -177,7 +191,7 @@ function reverseString(str) { }); } - { + if (includeComparisons) { const obj = generateBigObjectFixture(100); const observer = jsonpatch.observe(obj); @@ -194,7 +208,7 @@ function reverseString(str) { { const suiteName = 'Serialization'; - { + if (includeComparisons) { const obj = generateBigObjectFixture(100); suite.add(`${suiteName} (noop)`, function() { @@ -212,7 +226,7 @@ function reverseString(str) { }); } - { + if (includeComparisons) { const obj = generateBigObjectFixture(100); const observer = jsonpatch.observe(obj); From 44d43f2b2be8fb634ccd5c2cbf0d24b85d585c8a Mon Sep 17 00:00:00 2001 From: Marcin Warpechowski Date: Fri, 7 Jun 2019 13:57:32 +0200 Subject: [PATCH 10/11] add "generate" as the last step in the benchmarks to make functionality equal to fast-json-patch'es --- test/spec/proxyBenchmark.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/spec/proxyBenchmark.js b/test/spec/proxyBenchmark.js index 5bc1417..486ee58 100644 --- a/test/spec/proxyBenchmark.js +++ b/test/spec/proxyBenchmark.js @@ -85,8 +85,8 @@ function reverseString(str) { const obj = generateBigObjectFixture(1); const jsonPatcherProxy = new JSONPatcherProxy(obj); const observedObj = jsonPatcherProxy.observe(true); - const patches = jsonPatcherProxy.generate(); modifyObj(observedObj); + jsonPatcherProxy.generate(); }); } @@ -117,8 +117,8 @@ function reverseString(str) { const obj = generateBigObjectFixture(100); const jsonPatcherProxy = new JSONPatcherProxy(obj); const observedObj = jsonPatcherProxy.observe(true); - const patches = jsonPatcherProxy.generate(); modifyObj(observedObj); + jsonPatcherProxy.generate(); }); } @@ -152,6 +152,7 @@ function reverseString(str) { suite.add(`${suiteName} (JSONPatcherProxy)`, function() { observedObj.cars[50].name = reverseString(observedObj.cars[50].name); + jsonPatcherProxy.generate(); }); } @@ -188,6 +189,7 @@ function reverseString(str) { suite.add(`${suiteName} (JSONPatcherProxy)`, function() { const item = observedObj.cars.shift(); observedObj.cars.push(item); + jsonPatcherProxy.generate(); }); } From f9d65674f733766398cfb6c7ca08e19942af9b2c Mon Sep 17 00:00:00 2001 From: Marcin Warpechowski Date: Fri, 7 Jun 2019 14:38:06 +0200 Subject: [PATCH 11/11] add test for double proxification by JSONPatcherProxy --- test/spec/proxySpec.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/spec/proxySpec.js b/test/spec/proxySpec.js index 5263ccb..960ce5c 100644 --- a/test/spec/proxySpec.js +++ b/test/spec/proxySpec.js @@ -295,6 +295,27 @@ describe('proxy', function() { } ]); }); + + it('should generate replace (deep object, proxified twice by JSONPatcherProxy)', function() { + const obj = generateDeepObjectFixture(); + const jsonPatcherProxy = new JSONPatcherProxy(obj); + let observedObj = jsonPatcherProxy.observe(true); + const jsonPatcherProxy2 = new JSONPatcherProxy(observedObj); + let observedObj2 = jsonPatcherProxy2.observe(true); + + observedObj2.phoneNumbers[0].number = '123'; + + const patches = jsonPatcherProxy.generate(); + const patches2 = jsonPatcherProxy2.generate(); + expect(patches).toReallyEqual([ + { + op: 'replace', + path: '/phoneNumbers/0/number', + value: '123' + } + ]); + expect(patches).toReallyEqual(patches2); + }); it('should generate replace (changes in new array cell, primitive values)', function() { var arr = [1];