From 62e7174fbe9c0a2b4cb37fdf06147384bae446a7 Mon Sep 17 00:00:00 2001 From: Szymon Date: Wed, 28 Sep 2022 10:48:55 +0200 Subject: [PATCH 01/16] use faster implementation for merging --- lib/Onyx.js | 3 +- lib/OnyxCache.js | 3 +- lib/fastMerge.js | 99 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 lib/fastMerge.js diff --git a/lib/Onyx.js b/lib/Onyx.js index 0337555e8..e49ce00f8 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -7,6 +7,7 @@ import * as Logger from './Logger'; import cache from './OnyxCache'; import createDeferredTask from './createDeferredTask'; import mergeWithCustomized from './mergeWithCustomized'; +import { merge } from './fastMerge'; // Keeps track of the last connectionID that was used so we can keep incrementing it let lastConnectionID = 0; @@ -761,7 +762,7 @@ function applyMerge(key, data) { if (_.isObject(data) || _.every(mergeValues, _.isObject)) { // Object values are merged one after the other return _.reduce(mergeValues, (modifiedData, mergeValue) => { - const newData = mergeWithCustomized({}, modifiedData, mergeValue); + const newData = Object.assign({}, merge(modifiedData, mergeValue)); // We will also delete any object keys that are undefined or null. // Deleting keys is not supported by AsyncStorage so we do it this way. diff --git a/lib/OnyxCache.js b/lib/OnyxCache.js index 35a2eaaa3..b2875502c 100644 --- a/lib/OnyxCache.js +++ b/lib/OnyxCache.js @@ -1,5 +1,6 @@ import _ from 'underscore'; import mergeWithCustomized from './mergeWithCustomized'; +import { merge } from './fastMerge'; const isDefined = _.negate(_.isUndefined); @@ -110,7 +111,7 @@ class OnyxCache { * @param {Record} data - a map of (cache) key - values */ merge(data) { - this.storageMap = mergeWithCustomized({}, this.storageMap, data); + this.storageMap = Object.assign({}, merge(this.storageMap, data)); const storageKeys = this.getAllKeys(); const mergedKeys = _.keys(data); diff --git a/lib/fastMerge.js b/lib/fastMerge.js new file mode 100644 index 000000000..e1b90e2c5 --- /dev/null +++ b/lib/fastMerge.js @@ -0,0 +1,99 @@ +// Mostly copied from https://medium.com/@lubaka.a/how-to-remove-lodash-performance-improvement-b306669ad0e1 + +/** // Mostly copied from https://medium.com/@lubaka.a/how-to-remove-lodash-performance-improvement-b306669ad0e1 +* Is the object Mergeable +* +* @param val +* @returns {*|boolean} +*/ +function isMergeableObject(val) { + var nonNullObject = val && typeof val === 'object'; + return nonNullObject && + Object.prototype.toString.call(val) !== '[object RegExp]' && + Object.prototype.toString.call(val) !== '[object Date]'; +} +/** ++* Empty the Target +* +* @param val +* @returns {*} +*/ +function emptyTarget(val) { + return Array.isArray(val) ? [] : {} +} +/** +* Clone if Necessary +* +* @param value ++* @param optionsArgument +* @returns {*} +*/ +function cloneIfNecessary(value, optionsArgument) { + var clone = optionsArgument && optionsArgument.clone === true; + return (clone && isMergeableObject(value)) ? merge(emptyTarget(value), value, optionsArgument) : value; +} +/** +* Default Array Merge +* +* @param target +* @param source +* @param optionsArgument +* @returns {*} +*/ +let valArr; +function defaultArrayMerge(target, source, optionsArgument) { + var destination = target.slice(); + for (let i = 0; i < source.length; ++i) { + valArr = source[i]; + if (i >= destination.length) { + destination.push(cloneIfNecessary(valArr, optionsArgument)); + } else if (typeof destination[i] === 'undefined') { + destination[i] = cloneIfNecessary(valArr, optionsArgument); + } else if (isMergeableObject(valArr)) { + destination[i] = merge(target[i], valArr, optionsArgument); + } + } + return destination; +} +/** +* Merge Object +* +* @param target +* @param source +* @param optionsArgument +* @returns {{}} +*/ +function mergeObject(target, source, optionsArgument) { + var destination = {}; + if (isMergeableObject(target)) { + Object.keys(target).forEach(function (key) { + destination[key] = cloneIfNecessary(target[key], optionsArgument) + }) + } + Object.keys(source).forEach(function (key) { + if (!isMergeableObject(source[key]) || !target[key]) { + destination[key] = cloneIfNecessary(source[key], optionsArgument) + } else { + destination[key] = merge(target[key], source[key], optionsArgument) + } + }); + return destination +} +/** + * Merge Object and Arrays + * + * @param target + * @param source + * @param optionsArgument + * @returns {*} + */ +export function merge(target, source, optionsArgument) { + var array = Array.isArray(source); + var options = optionsArgument || { arrayMerge: defaultArrayMerge }; + var arrayMerge = options.arrayMerge || defaultArrayMerge; + if (array) { // We may not even need that as mergeWithCustomized suggests it + return Array.isArray(target) ? arrayMerge(target, source, optionsArgument) : cloneIfNecessary(source, optionsArgument); + } else { + return mergeObject(target, source, optionsArgument); + } +} \ No newline at end of file From 569717739161d0b287299f28d9fd663f81943cbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kapa=C5=82a?= Date: Wed, 28 Sep 2022 16:31:33 +0200 Subject: [PATCH 02/16] Update fastMerge.js --- lib/fastMerge.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/fastMerge.js b/lib/fastMerge.js index e1b90e2c5..0dc1729ae 100644 --- a/lib/fastMerge.js +++ b/lib/fastMerge.js @@ -13,7 +13,7 @@ function isMergeableObject(val) { Object.prototype.toString.call(val) !== '[object Date]'; } /** -+* Empty the Target +* Empty the Target * * @param val * @returns {*} @@ -25,7 +25,7 @@ function emptyTarget(val) { * Clone if Necessary * * @param value -+* @param optionsArgument +* @param optionsArgument * @returns {*} */ function cloneIfNecessary(value, optionsArgument) { @@ -96,4 +96,4 @@ export function merge(target, source, optionsArgument) { } else { return mergeObject(target, source, optionsArgument); } -} \ No newline at end of file +} From d0a96421f274042e27424fca2ab516ad66fe1492 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kapa=C5=82a?= Date: Wed, 28 Sep 2022 16:31:47 +0200 Subject: [PATCH 03/16] Update lib/fastMerge.js Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com> --- lib/fastMerge.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fastMerge.js b/lib/fastMerge.js index 0dc1729ae..97997fff0 100644 --- a/lib/fastMerge.js +++ b/lib/fastMerge.js @@ -1,6 +1,6 @@ // Mostly copied from https://medium.com/@lubaka.a/how-to-remove-lodash-performance-improvement-b306669ad0e1 -/** // Mostly copied from https://medium.com/@lubaka.a/how-to-remove-lodash-performance-improvement-b306669ad0e1 +/** * Is the object Mergeable * * @param val From a45cc7b4d3c1059b9faa18a1aaca86ce8378b75e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kapa=C5=82a?= Date: Wed, 28 Sep 2022 16:32:16 +0200 Subject: [PATCH 04/16] Update lib/fastMerge.js Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com> --- lib/fastMerge.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/fastMerge.js b/lib/fastMerge.js index 97997fff0..5398cfa80 100644 --- a/lib/fastMerge.js +++ b/lib/fastMerge.js @@ -80,13 +80,13 @@ function mergeObject(target, source, optionsArgument) { return destination } /** - * Merge Object and Arrays - * - * @param target - * @param source - * @param optionsArgument - * @returns {*} - */ + * Merge Object and Arrays + * + * @param target + * @param source + * @param optionsArgument + * @returns {*} + */ export function merge(target, source, optionsArgument) { var array = Array.isArray(source); var options = optionsArgument || { arrayMerge: defaultArrayMerge }; From da670b63c9ef3336507d4f63fa365688982f13f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kapa=C5=82a?= Date: Fri, 30 Sep 2022 11:02:18 +0200 Subject: [PATCH 05/16] Update fastMerge.js --- lib/fastMerge.js | 101 ++++++++++------------------------------------- 1 file changed, 21 insertions(+), 80 deletions(-) diff --git a/lib/fastMerge.js b/lib/fastMerge.js index 5398cfa80..cac6cb66a 100644 --- a/lib/fastMerge.js +++ b/lib/fastMerge.js @@ -1,99 +1,40 @@ // Mostly copied from https://medium.com/@lubaka.a/how-to-remove-lodash-performance-improvement-b306669ad0e1 -/** -* Is the object Mergeable -* -* @param val -* @returns {*|boolean} -*/ function isMergeableObject(val) { var nonNullObject = val && typeof val === 'object'; return nonNullObject && Object.prototype.toString.call(val) !== '[object RegExp]' && Object.prototype.toString.call(val) !== '[object Date]'; } -/** -* Empty the Target -* -* @param val -* @returns {*} -*/ -function emptyTarget(val) { - return Array.isArray(val) ? [] : {} -} -/** -* Clone if Necessary -* -* @param value -* @param optionsArgument -* @returns {*} -*/ -function cloneIfNecessary(value, optionsArgument) { - var clone = optionsArgument && optionsArgument.clone === true; - return (clone && isMergeableObject(value)) ? merge(emptyTarget(value), value, optionsArgument) : value; -} -/** -* Default Array Merge -* -* @param target -* @param source -* @param optionsArgument -* @returns {*} -*/ -let valArr; -function defaultArrayMerge(target, source, optionsArgument) { - var destination = target.slice(); - for (let i = 0; i < source.length; ++i) { - valArr = source[i]; - if (i >= destination.length) { - destination.push(cloneIfNecessary(valArr, optionsArgument)); - } else if (typeof destination[i] === 'undefined') { - destination[i] = cloneIfNecessary(valArr, optionsArgument); - } else if (isMergeableObject(valArr)) { - destination[i] = merge(target[i], valArr, optionsArgument); - } - } - return destination; -} -/** -* Merge Object -* -* @param target -* @param source -* @param optionsArgument -* @returns {{}} -*/ -function mergeObject(target, source, optionsArgument) { + +let key = ""; +function mergeObject(target, source) { var destination = {}; if (isMergeableObject(target)) { - Object.keys(target).forEach(function (key) { - destination[key] = cloneIfNecessary(target[key], optionsArgument) - }) + const targetKeys = Object.keys(target); + for (let i = 0; i < targetKeys.length; ++i) { + key = targetKeys[i]; + destination[key] = target[key]; + } } - Object.keys(source).forEach(function (key) { + const sourceKeys = Object.keys(source); + for (let i = 0; i < sourceKeys.length; ++i) { + const key = sourceKeys[i]; if (!isMergeableObject(source[key]) || !target[key]) { - destination[key] = cloneIfNecessary(source[key], optionsArgument) + destination[key] = source[key]; } else { - destination[key] = merge(target[key], source[key], optionsArgument) + destination[key] = merge(target[key], source[key]) } - }); - return destination + } + + return destination; } -/** - * Merge Object and Arrays - * - * @param target - * @param source - * @param optionsArgument - * @returns {*} - */ -export function merge(target, source, optionsArgument) { + +export function merge(target, source) { var array = Array.isArray(source); - var options = optionsArgument || { arrayMerge: defaultArrayMerge }; - var arrayMerge = options.arrayMerge || defaultArrayMerge; - if (array) { // We may not even need that as mergeWithCustomized suggests it - return Array.isArray(target) ? arrayMerge(target, source, optionsArgument) : cloneIfNecessary(source, optionsArgument); + if (array) { + return source; } else { - return mergeObject(target, source, optionsArgument); + return mergeObject(target, source); } } From 41961d7ad811fe6d211cd81a391fe96e0fd642f1 Mon Sep 17 00:00:00 2001 From: Szymon Date: Fri, 30 Sep 2022 11:45:08 +0200 Subject: [PATCH 06/16] cleanup --- lib/Onyx.js | 9 +++-- lib/OnyxCache.js | 7 ++-- lib/fastMerge.js | 55 +++++++++++++++------------- lib/mergeWithCustomized.js | 26 ------------- lib/storage/providers/LocalForage.js | 6 ++- 5 files changed, 42 insertions(+), 61 deletions(-) delete mode 100644 lib/mergeWithCustomized.js diff --git a/lib/Onyx.js b/lib/Onyx.js index e49ce00f8..820921196 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -6,8 +6,8 @@ import Storage from './storage'; import * as Logger from './Logger'; import cache from './OnyxCache'; import createDeferredTask from './createDeferredTask'; -import mergeWithCustomized from './mergeWithCustomized'; -import { merge } from './fastMerge'; +// eslint-disable-next-line rulesdir/prefer-import-module-contents +import {merge as fastMerge} from './fastMerge'; // Keeps track of the last connectionID that was used so we can keep incrementing it let lastConnectionID = 0; @@ -762,7 +762,8 @@ function applyMerge(key, data) { if (_.isObject(data) || _.every(mergeValues, _.isObject)) { // Object values are merged one after the other return _.reduce(mergeValues, (modifiedData, mergeValue) => { - const newData = Object.assign({}, merge(modifiedData, mergeValue)); + // eslint-disable-next-line prefer-object-spread, rulesdir/prefer-underscore-method + const newData = Object.assign({}, fastMerge(modifiedData, mergeValue)); // We will also delete any object keys that are undefined or null. // Deleting keys is not supported by AsyncStorage so we do it this way. @@ -833,7 +834,7 @@ function initializeWithDefaultKeyStates() { .then((pairs) => { const asObject = _.object(pairs); - const merged = mergeWithCustomized(asObject, defaultKeyStates); + const merged = fastMerge(asObject, defaultKeyStates); cache.merge(merged); _.each(merged, (val, key) => keyChanged(key, val)); }); diff --git a/lib/OnyxCache.js b/lib/OnyxCache.js index b2875502c..18a9fcc12 100644 --- a/lib/OnyxCache.js +++ b/lib/OnyxCache.js @@ -1,6 +1,6 @@ import _ from 'underscore'; -import mergeWithCustomized from './mergeWithCustomized'; -import { merge } from './fastMerge'; +// eslint-disable-next-line rulesdir/prefer-import-module-contents +import {merge as fastMerge} from './fastMerge'; const isDefined = _.negate(_.isUndefined); @@ -111,7 +111,8 @@ class OnyxCache { * @param {Record} data - a map of (cache) key - values */ merge(data) { - this.storageMap = Object.assign({}, merge(this.storageMap, data)); + // eslint-disable-next-line prefer-object-spread, rulesdir/prefer-underscore-method + this.storageMap = Object.assign({}, fastMerge(this.storageMap, data)); const storageKeys = this.getAllKeys(); const mergedKeys = _.keys(data); diff --git a/lib/fastMerge.js b/lib/fastMerge.js index cac6cb66a..a796f209c 100644 --- a/lib/fastMerge.js +++ b/lib/fastMerge.js @@ -1,40 +1,43 @@ // Mostly copied from https://medium.com/@lubaka.a/how-to-remove-lodash-performance-improvement-b306669ad0e1 function isMergeableObject(val) { - var nonNullObject = val && typeof val === 'object'; - return nonNullObject && - Object.prototype.toString.call(val) !== '[object RegExp]' && - Object.prototype.toString.call(val) !== '[object Date]'; + const nonNullObject = val && typeof val === 'object'; + return nonNullObject + && Object.prototype.toString.call(val) !== '[object RegExp]' + && Object.prototype.toString.call(val) !== '[object Date]'; } -let key = ""; function mergeObject(target, source) { - var destination = {}; - if (isMergeableObject(target)) { - const targetKeys = Object.keys(target); - for (let i = 0; i < targetKeys.length; ++i) { - key = targetKeys[i]; - destination[key] = target[key]; + const destination = {}; + if (isMergeableObject(target)) { + // eslint-disable-next-line rulesdir/prefer-underscore-method + const targetKeys = Object.keys(target); + for (let i = 0; i < targetKeys.length; ++i) { + const key = targetKeys[i]; + destination[key] = target[key]; + } } - } - const sourceKeys = Object.keys(source); - for (let i = 0; i < sourceKeys.length; ++i) { - const key = sourceKeys[i]; - if (!isMergeableObject(source[key]) || !target[key]) { - destination[key] = source[key]; - } else { - destination[key] = merge(target[key], source[key]) + // eslint-disable-next-line rulesdir/prefer-underscore-method + const sourceKeys = Object.keys(source); + for (let i = 0; i < sourceKeys.length; ++i) { + const key = sourceKeys[i]; + if (!isMergeableObject(source[key]) || !target[key]) { + destination[key] = source[key]; + } else { + // eslint-disable-next-line no-use-before-define + destination[key] = merge(target[key], source[key]); + } } - } - return destination; + return destination; } +// eslint-disable-next-line import/prefer-default-export, rulesdir/no-inline-named-export export function merge(target, source) { - var array = Array.isArray(source); - if (array) { - return source; - } else { + // eslint-disable-next-line rulesdir/prefer-underscore-method + const array = Array.isArray(source); + if (array) { + return source; + } return mergeObject(target, source); - } } diff --git a/lib/mergeWithCustomized.js b/lib/mergeWithCustomized.js deleted file mode 100644 index 4a7ffe42e..000000000 --- a/lib/mergeWithCustomized.js +++ /dev/null @@ -1,26 +0,0 @@ -import lodashMergeWith from 'lodash/mergeWith'; - -/** - * When merging 2 objects into onyx that contain an array, we want to completely replace the array instead of the default - * behavior which is to merge each item by its index. - * ie: - * merge({a: [1]}, {a: [2,3]}): - * - In the default implementation would produce {a:[1,3]} - * - With this function would produce: {a: [2,3]} - * @param {*} objValue - * @param {*} srcValue - * @return {*} - */ -// eslint-disable-next-line rulesdir/prefer-early-return -function customizerForMergeWith(objValue, srcValue) { - // eslint-disable-next-line rulesdir/prefer-underscore-method - if (Array.isArray(objValue)) { - return srcValue; - } -} - -function mergeWithCustomized(...args) { - return lodashMergeWith(...args, customizerForMergeWith); -} - -export default mergeWithCustomized; diff --git a/lib/storage/providers/LocalForage.js b/lib/storage/providers/LocalForage.js index 64cded662..f85fbba1e 100644 --- a/lib/storage/providers/LocalForage.js +++ b/lib/storage/providers/LocalForage.js @@ -7,7 +7,8 @@ import localforage from 'localforage'; import _ from 'underscore'; import SyncQueue from '../../SyncQueue'; -import mergeWithCustomized from '../../mergeWithCustomized'; +// eslint-disable-next-line rulesdir/prefer-import-module-contents +import {merge as fastMerge} from '../../fastMerge'; localforage.config({ name: 'OnyxDB', @@ -24,7 +25,8 @@ const provider = { return localforage.getItem(key) .then((existingValue) => { const newValue = _.isObject(existingValue) - ? mergeWithCustomized({}, existingValue, value) + // eslint-disable-next-line prefer-object-spread, rulesdir/prefer-underscore-method + ? Object.assign({}, fastMerge(existingValue, value)) : value; return localforage.setItem(key, newValue); }); From f57cacb1d6c3542d42b0bfc57e4239dbb4cf26f5 Mon Sep 17 00:00:00 2001 From: Szymon Date: Fri, 30 Sep 2022 11:47:48 +0200 Subject: [PATCH 07/16] cleanup2 --- lib/Onyx.js | 2 +- lib/OnyxCache.js | 2 +- lib/fastMerge.js | 2 +- lib/storage/providers/LocalForage.js | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Onyx.js b/lib/Onyx.js index 820921196..1bf1a08d5 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -7,7 +7,7 @@ import * as Logger from './Logger'; import cache from './OnyxCache'; import createDeferredTask from './createDeferredTask'; // eslint-disable-next-line rulesdir/prefer-import-module-contents -import {merge as fastMerge} from './fastMerge'; +import {fastMerge} from './fastMerge'; // Keeps track of the last connectionID that was used so we can keep incrementing it let lastConnectionID = 0; diff --git a/lib/OnyxCache.js b/lib/OnyxCache.js index 18a9fcc12..1189e50dd 100644 --- a/lib/OnyxCache.js +++ b/lib/OnyxCache.js @@ -1,6 +1,6 @@ import _ from 'underscore'; // eslint-disable-next-line rulesdir/prefer-import-module-contents -import {merge as fastMerge} from './fastMerge'; +import {fastMerge} from './fastMerge'; const isDefined = _.negate(_.isUndefined); diff --git a/lib/fastMerge.js b/lib/fastMerge.js index a796f209c..fcaf24749 100644 --- a/lib/fastMerge.js +++ b/lib/fastMerge.js @@ -33,7 +33,7 @@ function mergeObject(target, source) { } // eslint-disable-next-line import/prefer-default-export, rulesdir/no-inline-named-export -export function merge(target, source) { +export function fastMerge(target, source) { // eslint-disable-next-line rulesdir/prefer-underscore-method const array = Array.isArray(source); if (array) { diff --git a/lib/storage/providers/LocalForage.js b/lib/storage/providers/LocalForage.js index f85fbba1e..2c48a172d 100644 --- a/lib/storage/providers/LocalForage.js +++ b/lib/storage/providers/LocalForage.js @@ -8,7 +8,7 @@ import localforage from 'localforage'; import _ from 'underscore'; import SyncQueue from '../../SyncQueue'; // eslint-disable-next-line rulesdir/prefer-import-module-contents -import {merge as fastMerge} from '../../fastMerge'; +import {fastMerge} from '../../fastMerge'; localforage.config({ name: 'OnyxDB', From d0351ab801e358724d59b2b88179ee2d5cf9eed1 Mon Sep 17 00:00:00 2001 From: Szymon Date: Fri, 30 Sep 2022 11:48:44 +0200 Subject: [PATCH 08/16] cleanup3 --- lib/fastMerge.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fastMerge.js b/lib/fastMerge.js index fcaf24749..e8df117dd 100644 --- a/lib/fastMerge.js +++ b/lib/fastMerge.js @@ -1,4 +1,4 @@ -// Mostly copied from https://medium.com/@lubaka.a/how-to-remove-lodash-performance-improvement-b306669ad0e1 +// Mostly copied from https://medium.com/@lubaka.a/how-to-remove-lodash-performance-improvement-b306669ad0e1 function isMergeableObject(val) { const nonNullObject = val && typeof val === 'object'; From c1b1df4d8486b2becc4bfeea20f869ea981ad516 Mon Sep 17 00:00:00 2001 From: Szymon Date: Fri, 30 Sep 2022 13:24:19 +0200 Subject: [PATCH 09/16] cleanup4 --- lib/fastMerge.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fastMerge.js b/lib/fastMerge.js index e8df117dd..583906e8b 100644 --- a/lib/fastMerge.js +++ b/lib/fastMerge.js @@ -25,7 +25,7 @@ function mergeObject(target, source) { destination[key] = source[key]; } else { // eslint-disable-next-line no-use-before-define - destination[key] = merge(target[key], source[key]); + destination[key] = fastMerge(target[key], source[key]); } } From 6decdc2e3705585e1bcfd650c6975e4632da8ecc Mon Sep 17 00:00:00 2001 From: Szymon Date: Fri, 30 Sep 2022 13:53:11 +0200 Subject: [PATCH 10/16] cleanup5 --- lib/fastMerge.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/fastMerge.js b/lib/fastMerge.js index 583906e8b..9f6e4f37f 100644 --- a/lib/fastMerge.js +++ b/lib/fastMerge.js @@ -21,7 +21,9 @@ function mergeObject(target, source) { const sourceKeys = Object.keys(source); for (let i = 0; i < sourceKeys.length; ++i) { const key = sourceKeys[i]; - if (!isMergeableObject(source[key]) || !target[key]) { + if (source[key] === undefined) { + // noop + } else if (!isMergeableObject(source[key]) || !target[key]) { destination[key] = source[key]; } else { // eslint-disable-next-line no-use-before-define From 6415da0548cb250e01a48eb9fd39a1b2c16f1390 Mon Sep 17 00:00:00 2001 From: Szymon Date: Tue, 4 Oct 2022 09:20:26 +0200 Subject: [PATCH 11/16] apply review suggestions --- lib/Onyx.js | 4 ++-- lib/OnyxCache.js | 4 ++-- lib/fastMerge.js | 56 ++++++++++++++++++++++++++++++++++-------------- 3 files changed, 44 insertions(+), 20 deletions(-) diff --git a/lib/Onyx.js b/lib/Onyx.js index 1bf1a08d5..eec9304a9 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -7,7 +7,7 @@ import * as Logger from './Logger'; import cache from './OnyxCache'; import createDeferredTask from './createDeferredTask'; // eslint-disable-next-line rulesdir/prefer-import-module-contents -import {fastMerge} from './fastMerge'; +import fastMerge from './fastMerge'; // Keeps track of the last connectionID that was used so we can keep incrementing it let lastConnectionID = 0; @@ -763,7 +763,7 @@ function applyMerge(key, data) { // Object values are merged one after the other return _.reduce(mergeValues, (modifiedData, mergeValue) => { // eslint-disable-next-line prefer-object-spread, rulesdir/prefer-underscore-method - const newData = Object.assign({}, fastMerge(modifiedData, mergeValue)); + const newData = Object.assign({}, fastMerge(modifiedData, mergeValue)); // lodash adds a small overhead so we don't use it here // We will also delete any object keys that are undefined or null. // Deleting keys is not supported by AsyncStorage so we do it this way. diff --git a/lib/OnyxCache.js b/lib/OnyxCache.js index 1189e50dd..ee0b0511c 100644 --- a/lib/OnyxCache.js +++ b/lib/OnyxCache.js @@ -1,6 +1,6 @@ import _ from 'underscore'; // eslint-disable-next-line rulesdir/prefer-import-module-contents -import {fastMerge} from './fastMerge'; +import fastMerge from './fastMerge'; const isDefined = _.negate(_.isUndefined); @@ -112,7 +112,7 @@ class OnyxCache { */ merge(data) { // eslint-disable-next-line prefer-object-spread, rulesdir/prefer-underscore-method - this.storageMap = Object.assign({}, fastMerge(this.storageMap, data)); + this.storageMap = Object.assign({}, fastMerge(this.storageMap, data)); // lodash adds a small overhead so we don't use it here const storageKeys = this.getAllKeys(); const mergedKeys = _.keys(data); diff --git a/lib/fastMerge.js b/lib/fastMerge.js index 9f6e4f37f..106712ea8 100644 --- a/lib/fastMerge.js +++ b/lib/fastMerge.js @@ -1,5 +1,13 @@ // Mostly copied from https://medium.com/@lubaka.a/how-to-remove-lodash-performance-improvement-b306669ad0e1 +let mergeObject; + +/** +* Is the object Mergeable +* +* @param {*} val +* @returns {*|boolean} +*/ function isMergeableObject(val) { const nonNullObject = val && typeof val === 'object'; return nonNullObject @@ -7,39 +15,55 @@ function isMergeableObject(val) { && Object.prototype.toString.call(val) !== '[object Date]'; } -function mergeObject(target, source) { +/** + * Merge Object and Arrays + * + * @param {{}|[]} target + * @param {{}|[]} source + * @returns {{}|[]} + */ +function fastMerge(target, source) { + // eslint-disable-next-line rulesdir/prefer-underscore-method + const array = Array.isArray(source); // lodash adds a small overhead so we don't use it here + if (array) { + return source; + } + return mergeObject(target, source); +} + +/** +* Merge Object +* +* @param {{}} target +* @param {{}} source +* @returns {{}} +*/ +mergeObject = function (target, source) { const destination = {}; if (isMergeableObject(target)) { // eslint-disable-next-line rulesdir/prefer-underscore-method - const targetKeys = Object.keys(target); + const targetKeys = Object.keys(target); // lodash adds a small overhead so we don't use it here for (let i = 0; i < targetKeys.length; ++i) { const key = targetKeys[i]; destination[key] = target[key]; } } // eslint-disable-next-line rulesdir/prefer-underscore-method - const sourceKeys = Object.keys(source); + const sourceKeys = Object.keys(source); // lodash adds a small overhead so we don't use it here for (let i = 0; i < sourceKeys.length; ++i) { const key = sourceKeys[i]; if (source[key] === undefined) { - // noop - } else if (!isMergeableObject(source[key]) || !target[key]) { + // eslint-disable-next-line no-continue + continue; + } + if (!isMergeableObject(source[key]) || !target[key]) { destination[key] = source[key]; } else { - // eslint-disable-next-line no-use-before-define destination[key] = fastMerge(target[key], source[key]); } } return destination; -} +}; -// eslint-disable-next-line import/prefer-default-export, rulesdir/no-inline-named-export -export function fastMerge(target, source) { - // eslint-disable-next-line rulesdir/prefer-underscore-method - const array = Array.isArray(source); - if (array) { - return source; - } - return mergeObject(target, source); -} +export default fastMerge; From 494c152963927c75cc810ceca201b70bcc5b3c21 Mon Sep 17 00:00:00 2001 From: Szymon Date: Tue, 4 Oct 2022 09:21:07 +0200 Subject: [PATCH 12/16] apply review suggestions 2 --- lib/Onyx.js | 1 - lib/OnyxCache.js | 1 - 2 files changed, 2 deletions(-) diff --git a/lib/Onyx.js b/lib/Onyx.js index eec9304a9..0579de49c 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -6,7 +6,6 @@ import Storage from './storage'; import * as Logger from './Logger'; import cache from './OnyxCache'; import createDeferredTask from './createDeferredTask'; -// eslint-disable-next-line rulesdir/prefer-import-module-contents import fastMerge from './fastMerge'; // Keeps track of the last connectionID that was used so we can keep incrementing it diff --git a/lib/OnyxCache.js b/lib/OnyxCache.js index ee0b0511c..8252af06c 100644 --- a/lib/OnyxCache.js +++ b/lib/OnyxCache.js @@ -1,5 +1,4 @@ import _ from 'underscore'; -// eslint-disable-next-line rulesdir/prefer-import-module-contents import fastMerge from './fastMerge'; const isDefined = _.negate(_.isUndefined); From 3e6cf77e279ea979854452d90894a793272c366f Mon Sep 17 00:00:00 2001 From: Szymon Date: Tue, 4 Oct 2022 09:37:54 +0200 Subject: [PATCH 13/16] apply review suggestions 3 --- lib/storage/providers/LocalForage.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/storage/providers/LocalForage.js b/lib/storage/providers/LocalForage.js index 2c48a172d..5170d961c 100644 --- a/lib/storage/providers/LocalForage.js +++ b/lib/storage/providers/LocalForage.js @@ -7,8 +7,7 @@ import localforage from 'localforage'; import _ from 'underscore'; import SyncQueue from '../../SyncQueue'; -// eslint-disable-next-line rulesdir/prefer-import-module-contents -import {fastMerge} from '../../fastMerge'; +import fastMerge from '../../fastMerge'; localforage.config({ name: 'OnyxDB', @@ -26,7 +25,7 @@ const provider = { .then((existingValue) => { const newValue = _.isObject(existingValue) // eslint-disable-next-line prefer-object-spread, rulesdir/prefer-underscore-method - ? Object.assign({}, fastMerge(existingValue, value)) + ? Object.assign({}, fastMerge(existingValue, value)) // lodash adds a small overhead so we don't use it here : value; return localforage.setItem(key, newValue); }); From 3ba5681cc0776007c9b2ce3308bdc885ace90f37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kapa=C5=82a?= Date: Wed, 5 Oct 2022 13:35:33 +0200 Subject: [PATCH 14/16] Apply suggestions from code review Co-authored-by: Tim Golen --- lib/fastMerge.js | 13 +++++-------- lib/storage/providers/LocalForage.js | 3 ++- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/fastMerge.js b/lib/fastMerge.js index 106712ea8..a403f0933 100644 --- a/lib/fastMerge.js +++ b/lib/fastMerge.js @@ -16,8 +16,6 @@ function isMergeableObject(val) { } /** - * Merge Object and Arrays - * * @param {{}|[]} target * @param {{}|[]} source * @returns {{}|[]} @@ -32,17 +30,16 @@ function fastMerge(target, source) { } /** -* Merge Object -* -* @param {{}} target -* @param {{}} source -* @returns {{}} + * @param {Object} target + * @param {Object} source + * @returns {Object} */ mergeObject = function (target, source) { const destination = {}; if (isMergeableObject(target)) { + // lodash adds a small overhead so we don't use it here // eslint-disable-next-line rulesdir/prefer-underscore-method - const targetKeys = Object.keys(target); // lodash adds a small overhead so we don't use it here + const targetKeys = Object.keys(target); for (let i = 0; i < targetKeys.length; ++i) { const key = targetKeys[i]; destination[key] = target[key]; diff --git a/lib/storage/providers/LocalForage.js b/lib/storage/providers/LocalForage.js index 5170d961c..20d707958 100644 --- a/lib/storage/providers/LocalForage.js +++ b/lib/storage/providers/LocalForage.js @@ -24,8 +24,9 @@ const provider = { return localforage.getItem(key) .then((existingValue) => { const newValue = _.isObject(existingValue) + // lodash adds a small overhead so we don't use it here // eslint-disable-next-line prefer-object-spread, rulesdir/prefer-underscore-method - ? Object.assign({}, fastMerge(existingValue, value)) // lodash adds a small overhead so we don't use it here + ? Object.assign({}, fastMerge(existingValue, value)) : value; return localforage.setItem(key, newValue); }); From 44db3fcc5765c06822caf11fd09a7b3c43131d20 Mon Sep 17 00:00:00 2001 From: Szymon Date: Wed, 5 Oct 2022 13:46:19 +0200 Subject: [PATCH 15/16] cleanup --- lib/Onyx.js | 3 ++- lib/OnyxCache.js | 3 ++- lib/fastMerge.js | 52 ++++++++++++++++++++++++------------------------ 3 files changed, 30 insertions(+), 28 deletions(-) diff --git a/lib/Onyx.js b/lib/Onyx.js index 0579de49c..1d6778c70 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -761,8 +761,9 @@ function applyMerge(key, data) { if (_.isObject(data) || _.every(mergeValues, _.isObject)) { // Object values are merged one after the other return _.reduce(mergeValues, (modifiedData, mergeValue) => { + // lodash adds a small overhead so we don't use it here // eslint-disable-next-line prefer-object-spread, rulesdir/prefer-underscore-method - const newData = Object.assign({}, fastMerge(modifiedData, mergeValue)); // lodash adds a small overhead so we don't use it here + const newData = Object.assign({}, fastMerge(modifiedData, mergeValue)); // We will also delete any object keys that are undefined or null. // Deleting keys is not supported by AsyncStorage so we do it this way. diff --git a/lib/OnyxCache.js b/lib/OnyxCache.js index 8252af06c..5b3fd8f75 100644 --- a/lib/OnyxCache.js +++ b/lib/OnyxCache.js @@ -110,8 +110,9 @@ class OnyxCache { * @param {Record} data - a map of (cache) key - values */ merge(data) { + // lodash adds a small overhead so we don't use it here // eslint-disable-next-line prefer-object-spread, rulesdir/prefer-underscore-method - this.storageMap = Object.assign({}, fastMerge(this.storageMap, data)); // lodash adds a small overhead so we don't use it here + this.storageMap = Object.assign({}, fastMerge(this.storageMap, data)); const storageKeys = this.getAllKeys(); const mergedKeys = _.keys(data); diff --git a/lib/fastMerge.js b/lib/fastMerge.js index a403f0933..a79825d44 100644 --- a/lib/fastMerge.js +++ b/lib/fastMerge.js @@ -1,32 +1,14 @@ // Mostly copied from https://medium.com/@lubaka.a/how-to-remove-lodash-performance-improvement-b306669ad0e1 -let mergeObject; - /** -* Is the object Mergeable -* -* @param {*} val -* @returns {*|boolean} + * @param {mixed} val + * @returns {boolean} */ function isMergeableObject(val) { - const nonNullObject = val && typeof val === 'object'; - return nonNullObject + const nonNullObject = val != null ? typeof val === 'object' : false; + return (nonNullObject && Object.prototype.toString.call(val) !== '[object RegExp]' - && Object.prototype.toString.call(val) !== '[object Date]'; -} - -/** - * @param {{}|[]} target - * @param {{}|[]} source - * @returns {{}|[]} - */ -function fastMerge(target, source) { - // eslint-disable-next-line rulesdir/prefer-underscore-method - const array = Array.isArray(source); // lodash adds a small overhead so we don't use it here - if (array) { - return source; - } - return mergeObject(target, source); + && Object.prototype.toString.call(val) !== '[object Date]'); } /** @@ -34,7 +16,7 @@ function fastMerge(target, source) { * @param {Object} source * @returns {Object} */ -mergeObject = function (target, source) { +function mergeObject(target, source) { const destination = {}; if (isMergeableObject(target)) { // lodash adds a small overhead so we don't use it here @@ -45,8 +27,10 @@ mergeObject = function (target, source) { destination[key] = target[key]; } } + + // lodash adds a small overhead so we don't use it here // eslint-disable-next-line rulesdir/prefer-underscore-method - const sourceKeys = Object.keys(source); // lodash adds a small overhead so we don't use it here + const sourceKeys = Object.keys(source); for (let i = 0; i < sourceKeys.length; ++i) { const key = sourceKeys[i]; if (source[key] === undefined) { @@ -56,11 +40,27 @@ mergeObject = function (target, source) { if (!isMergeableObject(source[key]) || !target[key]) { destination[key] = source[key]; } else { + // eslint-disable-next-line no-use-before-define destination[key] = fastMerge(target[key], source[key]); } } return destination; -}; +} + +/** + * @param {Object|Array} target + * @param {Object|Array} source + * @returns {Object|Array} +*/ +function fastMerge(target, source) { + // lodash adds a small overhead so we don't use it here + // eslint-disable-next-line rulesdir/prefer-underscore-method + const array = Array.isArray(source); + if (array) { + return source; + } + return mergeObject(target, source); +} export default fastMerge; From 0cf356f1774a698ab0a803abf683f7756f550ceb Mon Sep 17 00:00:00 2001 From: Szymon Date: Wed, 5 Oct 2022 13:47:26 +0200 Subject: [PATCH 16/16] cleanup 2 add missing line --- lib/storage/providers/LocalForage.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/storage/providers/LocalForage.js b/lib/storage/providers/LocalForage.js index 20d707958..ea85a9153 100644 --- a/lib/storage/providers/LocalForage.js +++ b/lib/storage/providers/LocalForage.js @@ -24,6 +24,7 @@ const provider = { return localforage.getItem(key) .then((existingValue) => { const newValue = _.isObject(existingValue) + // lodash adds a small overhead so we don't use it here // eslint-disable-next-line prefer-object-spread, rulesdir/prefer-underscore-method ? Object.assign({}, fastMerge(existingValue, value))