From 8003aec2318f4b30b3b9e46d581159e3b26a0248 Mon Sep 17 00:00:00 2001 From: Darren Wright Date: Wed, 29 May 2019 22:19:12 -0700 Subject: [PATCH] Fix callback details for array changes (#30) --- bench/bench.js | 161 ++++++++++++++++++++++--------------------------- index.js | 75 +++++++++++++++++++---- test.js | 74 ++++++++++++++++++++++- 3 files changed, 206 insertions(+), 104 deletions(-) diff --git a/bench/bench.js b/bench/bench.js index 70bc3bf..0e95c72 100644 --- a/bench/bench.js +++ b/bench/bench.js @@ -4,22 +4,11 @@ const onChange = require('..'); const save = () => {}; -suite('on-change', () => { - set('mintime', 5000); +const commonBench = function () { + set('mintime', 500); let val = 0; - before(() => { - this.object = onChange({ - a: 0, - b: 0, - c: 0, - d: 0, - subObj: {a: 0} - }, save); - this.array = onChange([0, 0, 0, 0], save); - }); - bench('object read', () => { this.object.a === val++; // eslint-disable-line no-unused-expressions }); @@ -34,109 +23,103 @@ suite('on-change', () => { bench('object write', () => { this.object.a = val++; - this.object.b = val++; - this.object.c = val++; - this.object.d = val++; }); bench('array write', () => { this.array[0] = val++; - this.array[1] = val++; - this.array[2] = val++; - this.array[3] = val++; }); -}); - -suite('on-change shallow', () => { - set('mintime', 5000); - - let val = 0; - before(() => { - this.object = onChange({ - a: 0, - b: 0, - c: 0, - d: 0, - subObj: {a: 0} - }, save, true); - this.array = onChange([0, 0, 0, 0], save, true); + bench('array write in apply', () => { + this.array.some((value, index) => { + this.array[index] = val++; + return true; + }); }); - bench('object read', () => { - this.object.a === val++; // eslint-disable-line no-unused-expressions + bench('array push + pop', () => { + this.array.push(val++); + this.array.pop(); }); - bench('nested read', () => { - this.object.subObj.a === val++; // eslint-disable-line no-unused-expressions + bench('array unshift + shift', () => { + this.array.unshift(val++); + this.array.shift(); }); +}; - bench('array read', () => { - this.array[0] === val++; // eslint-disable-line no-unused-expressions - }); +const buildArray = length => { + const array = []; + array.length = length; + return array.fill(0); +}; - bench('object write', () => { - this.object.a = val++; - this.object.b = val++; - this.object.c = val++; - this.object.d = val++; - }); +const buildObject = length => { + let prop; + const object = { + subObj: {a: 0} + }; - bench('array write', () => { - this.array[0] = val++; - this.array[1] = val++; - this.array[2] = val++; - this.array[3] = val++; - }); -}); + for (let index = 0; index < length; index++) { + prop = String.fromCharCode((index % 26) + 97); + object[prop.repeat(Math.ceil((index + 1) / 26))] = 0; + } -suite('native', () => { - set('mintime', 5000); + return object; +}; - let val = 0; +const SMALL = 10; +const LARGE = 100000; +suite('on-change', () => { before(() => { - this.object = { - a: 0, - b: 0, - c: 0, - d: 0, - subObj: {a: 0} - }; - this.array = [0, 0, 0, 0]; + this.object = onChange(buildObject(SMALL), save); + this.array = onChange(buildArray(SMALL), save); }); - bench('object read', () => { - this.object.a === val++; // eslint-disable-line no-unused-expressions + commonBench.call(this); +}); + +suite('on-change, large objects', () => { + before(() => { + this.object = onChange(buildObject(LARGE), save); + this.array = onChange(buildArray(LARGE), save); }); - bench('nested read', () => { - this.object.subObj.a === val++; // eslint-disable-line no-unused-expressions + commonBench.call(this); +}); + +suite('on-change, isShallow', () => { + before(() => { + this.object = onChange(buildObject(SMALL), save, {isShallow: true}); + this.array = onChange(buildArray(SMALL), save, {isShallow: true}); }); - bench('array read', () => { - this.array[0] === val++; // eslint-disable-line no-unused-expressions + commonBench.call(this); +}); + +suite('on-change, isShallow, large objects', () => { + before(() => { + this.object = onChange(buildObject(LARGE), save, {isShallow: true}); + this.array = onChange(buildArray(LARGE), save, {isShallow: true}); }); - bench('object write', () => { - this.object.a = val++; - save(); - this.object.b = val++; - save(); - this.object.c = val++; - save(); - this.object.d = val++; - save(); + commonBench.call(this); +}); + +suite('native', () => { + before(() => { + this.object = buildObject(SMALL); + this.array = buildArray(SMALL); }); - bench('array write', () => { - this.array[0] = val++; - save(); - this.array[1] = val++; - save(); - this.array[2] = val++; - save(); - this.array[3] = val++; - save(); + commonBench.call(this); +}); + +suite('native, large objects', () => { + before(() => { + this.object = buildObject(LARGE); + this.array = buildArray(LARGE); }); + + commonBench.call(this); }); diff --git a/index.js b/index.js index 4183efa..3097238 100644 --- a/index.js +++ b/index.js @@ -1,5 +1,7 @@ 'use strict'; +const PATH_SEPARATOR = '.'; + const isPrimitive = value => value === null || (typeof value !== 'object' && typeof value !== 'function'); const isBuiltinWithoutMutableMethods = value => value instanceof RegExp || value instanceof Number; @@ -9,7 +11,7 @@ const isBuiltinWithMutableMethods = value => value instanceof Date; const concatPath = (path, property) => { if (property && property.toString) { if (path) { - path += '.'; + path += PATH_SEPARATOR; } path += property.toString(); @@ -18,21 +20,63 @@ const concatPath = (path, property) => { return path; }; +const walkPath = (path, callback) => { + let index; + + while (path) { + index = path.indexOf(PATH_SEPARATOR); + + if (index === -1) { + index = path.length; + } + + callback(path.slice(0, index)); + + path = path.slice(index + 1); + } +}; + +const shallowClone = value => { + if (Array.isArray(value)) { + return value.slice(); + } + + return Object.assign({}, value); +}; + const proxyTarget = Symbol('ProxyTarget'); const onChange = (object, onChange, options = {}) => { let inApply = false; let changed = false; + let applyPath; + let applyPrevious; const propCache = new WeakMap(); const pathCache = new WeakMap(); const proxyCache = new WeakMap(); const handleChange = (path, property, previous, value) => { if (!inApply) { - onChange.call(proxy, concatPath(path, property), value, previous); - } else if (!changed) { - changed = true; + onChange(concatPath(path, property), value, previous); + return; } + + if (inApply && previous !== undefined && value !== undefined && property !== 'length') { + let item = applyPrevious; + + if (path !== applyPath) { + path = path.replace(applyPath, '').slice(1); + + walkPath(path, key => { + item[key] = shallowClone(item[key]); + item = item[key]; + }); + } + + item[property] = previous; + } + + changed = true; }; const getOwnPropertyDescriptor = (target, property) => { @@ -106,7 +150,7 @@ const onChange = (object, onChange, options = {}) => { } const previous = Reflect.get(target, property, receiver); - const result = Reflect.set(target, property, value); + const result = Reflect.set(target[proxyTarget] || target, property, value); if (previous !== value) { handleChange(pathCache.get(target), property, previous, value); @@ -136,7 +180,6 @@ const onChange = (object, onChange, options = {}) => { apply(target, thisArg, argumentsList) { const compare = isBuiltinWithMutableMethods(thisArg); - let previous; if (compare) { thisArg = thisArg[proxyTarget]; @@ -146,17 +189,24 @@ const onChange = (object, onChange, options = {}) => { inApply = true; if (compare) { - previous = thisArg.valueOf(); + applyPrevious = thisArg.valueOf(); } - const result = Reflect.apply(target, thisArg, argumentsList); - - if (changed || (compare && previous !== thisArg.valueOf())) { - onChange(); + if (Array.isArray(thisArg)) { + applyPrevious = shallowClone(thisArg[proxyTarget]); } + applyPath = pathCache.get(target); + applyPath = applyPath.slice(0, applyPath.lastIndexOf(PATH_SEPARATOR)); + const result = Reflect.apply(target, thisArg, argumentsList); + inApply = false; - changed = false; + + if (changed || (compare && applyPrevious !== thisArg.valueOf())) { + handleChange(applyPath, '', applyPrevious, thisArg); + applyPrevious = null; + changed = false; + } return result; } @@ -167,6 +217,7 @@ const onChange = (object, onChange, options = {}) => { pathCache.set(object, ''); const proxy = new Proxy(object, handler); + onChange = onChange.bind(proxy); return proxy; }; diff --git a/test.js b/test.js index ba06e4e..1001049 100644 --- a/test.js +++ b/test.js @@ -214,6 +214,7 @@ test('the callback should provide the original proxied object, the path to the c } }; + let callCount = 0; let returnedObject; let returnedPath; let returnedPrevious; @@ -224,25 +225,92 @@ test('the callback should provide the original proxied object, the path to the c returnedPath = path; returnedValue = value; returnedPrevious = previous; + callCount++; }); proxy.x.y[0].z = 1; t.is(returnedObject, proxy); t.is(returnedPath, 'x.y.0.z'); - t.is(returnedValue, 1); t.is(returnedPrevious, 0); + t.is(returnedValue, 1); + t.is(callCount, 1); proxy.x.y[0].new = 1; t.is(returnedObject, proxy); t.is(returnedPath, 'x.y.0.new'); - t.is(returnedValue, 1); t.is(returnedPrevious, undefined); + t.is(returnedValue, 1); + t.is(callCount, 2); delete proxy.x.y[0].new; t.is(returnedObject, proxy); t.is(returnedPath, 'x.y.0.new'); - t.is(returnedValue, undefined); t.is(returnedPrevious, 1); + t.is(returnedValue, undefined); + t.is(callCount, 3); + + proxy.x.y.push('pushed'); + t.is(returnedObject, proxy); + t.is(returnedPath, 'x.y'); + t.deepEqual(returnedPrevious, [{z: 1}]); + t.deepEqual(returnedValue, [{z: 1}, 'pushed']); + t.is(callCount, 4); + + proxy.x.y.pop(); + t.is(returnedObject, proxy); + t.is(returnedPath, 'x.y'); + t.deepEqual(returnedPrevious, [{z: 1}, 'pushed']); + t.deepEqual(returnedValue, [{z: 1}]); + t.is(callCount, 5); + + proxy.x.y.unshift('unshifted'); + t.is(returnedObject, proxy); + t.is(returnedPath, 'x.y'); + t.deepEqual(returnedPrevious, [{z: 1}]); + t.deepEqual(returnedValue, ['unshifted', {z: 1}]); + t.is(callCount, 6); + + proxy.x.y.shift(); + t.is(returnedObject, proxy); + t.is(returnedPath, 'x.y'); + t.deepEqual(returnedPrevious, ['unshifted', {z: 1}]); + t.deepEqual(returnedValue, [{z: 1}]); + t.is(callCount, 7); + + proxy.x.y = proxy.x.y.concat([{z: 3}, {z: 2}]); + t.is(returnedObject, proxy); + t.is(returnedPath, 'x.y'); + t.deepEqual(returnedPrevious, [{z: 1}]); + t.deepEqual(returnedValue, [{z: 1}, {z: 3}, {z: 2}]); + t.is(callCount, 8); + + proxy.x.y.sort((a, b) => a.z - b.z); + t.is(returnedObject, proxy); + t.is(returnedPath, 'x.y'); + t.deepEqual(returnedPrevious, [{z: 1}, {z: 3}, {z: 2}]); + t.deepEqual(returnedValue, [{z: 1}, {z: 2}, {z: 3}]); + t.is(callCount, 9); + + proxy.x.y.reverse(); + t.is(returnedObject, proxy); + t.is(returnedPath, 'x.y'); + t.deepEqual(returnedPrevious, [{z: 1}, {z: 2}, {z: 3}]); + t.deepEqual(returnedValue, [{z: 3}, {z: 2}, {z: 1}]); + t.is(callCount, 10); + + proxy.x.y.forEach(item => item.z++); + t.is(returnedObject, proxy); + t.is(returnedPath, 'x.y'); + t.deepEqual(returnedPrevious, [{z: 3}, {z: 2}, {z: 1}]); + t.deepEqual(returnedValue, [{z: 4}, {z: 3}, {z: 2}]); + t.is(callCount, 11); + + proxy.x.y.splice(1, 2); + t.is(returnedObject, proxy); + t.is(returnedPath, 'x.y'); + t.deepEqual(returnedPrevious, [{z: 4}, {z: 3}, {z: 2}]); + t.deepEqual(returnedValue, [{z: 4}]); + t.is(callCount, 12); }); test('should not call the callback for nested items if isShallow is true', t => {