From b6b0dd9ae4db79d0fb71c851f64979040de4e824 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Mon, 25 Mar 2024 16:26:44 +0100 Subject: [PATCH 01/27] add mergeCollection test --- tests/unit/onyxTest.js | 50 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/unit/onyxTest.js b/tests/unit/onyxTest.js index a75fd6bb..a63883d8 100644 --- a/tests/unit/onyxTest.js +++ b/tests/unit/onyxTest.js @@ -1039,6 +1039,56 @@ describe('Onyx', () => { }); }); }); + + it('mergeCollection should omit nested null values', () => { + let result; + + const routineRoute = `${ONYX_KEYS.COLLECTION.TEST_KEY}routine`; + const holidayRoute = `${ONYX_KEYS.COLLECTION.TEST_KEY}holiday`; + + connectionID = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_KEY, + initWithStoredValues: false, + callback: (value) => (result = value), + waitForCollectionCallback: true, + }); + + return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { + [routineRoute]: { + waypoints: { + 1: 'Home', + 2: 'Work', + 3: 'Gym', + }, + }, + [holidayRoute]: { + waypoints: { + 1: 'Home', + 2: 'Beach', + 3: null, + }, + }, + }).then(() => { + console.log(JSON.stringify(result)); + + expect(result).toEqual({ + [routineRoute]: { + waypoints: { + 1: 'Home', + 2: 'Work', + 3: 'Gym', + }, + }, + [holidayRoute]: { + waypoints: { + 1: 'Home', + 2: 'Beach', + }, + }, + }); + }); + }); + it('should apply updates in order with Onyx.update', () => { let testKeyValue; From 2452f722bb443a228516f1a0fa9b6a13341db71a Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Wed, 27 Mar 2024 18:35:47 +0100 Subject: [PATCH 02/27] don't remove null values when merging with multiMerge directly --- lib/Onyx.ts | 13 +++++++++++-- lib/OnyxUtils.d.ts | 22 ++++++++++++++++------ lib/OnyxUtils.js | 16 +++++++++------- 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 98b9413c..a7ab135b 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -326,6 +326,8 @@ function merge(key: TKey, changes: OnyxEntry(collectionKey: TK obj[key] = mergedCollection[key]; return obj; }, {}); - const keyValuePairsForExistingCollection = OnyxUtils.prepareKeyValuePairsForStorage(existingKeyCollection); - const keyValuePairsForNewCollection = OnyxUtils.prepareKeyValuePairsForStorage(newCollection); + + // When (multi-)merging the values with the existing values in storage, + // we don't want to remove nested null values from the data that we pass to the storage layer, + // because the storage layer uses them to remove nested keys from storage natively. + const keyValuePairsForExistingCollection = OnyxUtils.prepareKeyValuePairsForStorage(existingKeyCollection, false); + + // We can safely remove nested null values when using (multi-)set, + // because we will simply overwrite the existing values in storage. + const keyValuePairsForNewCollection = OnyxUtils.prepareKeyValuePairsForStorage(newCollection, true); const promises = []; diff --git a/lib/OnyxUtils.d.ts b/lib/OnyxUtils.d.ts index 0df7a186..c4b4851f 100644 --- a/lib/OnyxUtils.d.ts +++ b/lib/OnyxUtils.d.ts @@ -247,11 +247,15 @@ declare function hasPendingMergeForKey(key: OnyxKey): boolean; /** * Removes a key from storage if the value is null. * Otherwise removes all nested null values in objects and returns the object - * @returns The value without null values and a boolean "wasRemoved", which indicates if the key got removed completely + * @param {String} key + * @param {Mixed} value + * @param {Boolean} shouldRemoveNestedNullsInObjects + * @returns {Mixed} The value without null values and a boolean "wasRemoved", which indicates if the key got removed completely */ declare function removeNullValues( key: TKey, value: OnyxValue, + shouldRemoveNestedNullsInObjects?: boolean, ): { value: OnyxValue; wasRemoved: boolean; @@ -260,16 +264,22 @@ declare function removeNullValues( * Storage expects array like: [["@MyApp_user", value_1], ["@MyApp_key", value_2]] * This method transforms an object like {'@MyApp_user': myUserValue, '@MyApp_key': myKeyValue} * to an array of key-value pairs in the above format and removes key-value pairs that are being set to null - * - * @return an array of key - value pairs <[key, value]> + * @private + * @param {Record} data + * @param {Boolean} shouldRemoveNestedNullsInObjects + * @return {Array} an array of key - value pairs <[key, value]> */ -declare function prepareKeyValuePairsForStorage(data: Record>): Array<[OnyxKey, OnyxValue]>; +declare function prepareKeyValuePairsForStorage(data: Record>, shouldRemoveNestedNullsInObjects: boolean): Array<[OnyxKey, OnyxValue]>; /** * Merges an array of changes with an existing value * - * @param changes Array of changes that should be applied to the existing value + * @private + * @param {*} existingValue + * @param {Array<*>} changes Array of changes that should be applied to the existing value + * @param {Boolean} shouldRemoveNestedNullsInObjects + * @returns {*} */ -declare function applyMerge(existingValue: OnyxValue, changes: Array>, shouldRemoveNullObjectValues: boolean): any; +declare function applyMerge(existingValue: OnyxValue, changes: Array>, shouldRemoveNestedNullsInObjects: boolean): any; /** * Merge user provided default key value pairs. */ diff --git a/lib/OnyxUtils.js b/lib/OnyxUtils.js index 6a74adf5..2aba8d8f 100644 --- a/lib/OnyxUtils.js +++ b/lib/OnyxUtils.js @@ -1066,9 +1066,10 @@ function hasPendingMergeForKey(key) { * Otherwise removes all nested null values in objects and returns the object * @param {String} key * @param {Mixed} value + * @param {Boolean} shouldRemoveNestedNullsInObjects * @returns {Mixed} The value without null values and a boolean "wasRemoved", which indicates if the key got removed completely */ -function removeNullValues(key, value) { +function removeNullValues(key, value, shouldRemoveNestedNullsInObjects = true) { if (_.isNull(value)) { remove(key); return {value, wasRemoved: true}; @@ -1077,7 +1078,7 @@ function removeNullValues(key, value) { // We can remove all null values in an object by merging it with itself // utils.fastMerge recursively goes through the object and removes all null values // Passing two identical objects as source and target to fastMerge will not change it, but only remove the null values - return {value: utils.removeNestedNullValues(value), wasRemoved: false}; + return {value: shouldRemoveNestedNullsInObjects ? utils.removeNestedNullValues(value) : value, wasRemoved: false}; } /** @@ -1086,13 +1087,14 @@ function removeNullValues(key, value) { * to an array of key-value pairs in the above format and removes key-value pairs that are being set to null * @private * @param {Record} data + * @param {Boolean} shouldRemoveNestedNullsInObjects * @return {Array} an array of key - value pairs <[key, value]> */ -function prepareKeyValuePairsForStorage(data) { +function prepareKeyValuePairsForStorage(data, shouldRemoveNestedNullsInObjects) { const keyValuePairs = []; _.forEach(data, (value, key) => { - const {value: valueAfterRemoving, wasRemoved} = removeNullValues(key, value); + const {value: valueAfterRemoving, wasRemoved} = removeNullValues(key, value, shouldRemoveNestedNullsInObjects); if (wasRemoved) return; @@ -1108,10 +1110,10 @@ function prepareKeyValuePairsForStorage(data) { * @private * @param {*} existingValue * @param {Array<*>} changes Array of changes that should be applied to the existing value - * @param {Boolean} shouldRemoveNullObjectValues + * @param {Boolean} shouldRemoveNestedNullsInObjects * @returns {*} */ -function applyMerge(existingValue, changes, shouldRemoveNullObjectValues) { +function applyMerge(existingValue, changes, shouldRemoveNestedNullsInObjects) { const lastChange = _.last(changes); if (_.isArray(lastChange)) { @@ -1120,7 +1122,7 @@ function applyMerge(existingValue, changes, shouldRemoveNullObjectValues) { if (_.some(changes, _.isObject)) { // Object values are then merged one after the other - return _.reduce(changes, (modifiedData, change) => utils.fastMerge(modifiedData, change, shouldRemoveNullObjectValues), existingValue || {}); + return _.reduce(changes, (modifiedData, change) => utils.fastMerge(modifiedData, change, shouldRemoveNestedNullsInObjects), existingValue || {}); } // If we have anything else we can't merge it so we'll From 32e441e7f605839d386cd5abf1b96f85522e154f Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 28 Mar 2024 11:21:44 +0100 Subject: [PATCH 03/27] add missing property --- lib/Onyx.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index a7ab135b..eab90404 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -243,7 +243,7 @@ function set(key: TKey, value: OnyxEntry): Promise { - const keyValuePairs = OnyxUtils.prepareKeyValuePairsForStorage(data); + const keyValuePairs = OnyxUtils.prepareKeyValuePairsForStorage(data, true); const updatePromises = keyValuePairs.map(([key, value]) => { const prevValue = cache.getValue(key, false); From 86ccc68859649e980856f9c070eb15632c92aa61 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 28 Mar 2024 11:22:15 +0100 Subject: [PATCH 04/27] fix: test --- tests/unit/onyxTest.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/unit/onyxTest.js b/tests/unit/onyxTest.js index a63883d8..6d58ebe9 100644 --- a/tests/unit/onyxTest.js +++ b/tests/unit/onyxTest.js @@ -1069,8 +1069,6 @@ describe('Onyx', () => { }, }, }).then(() => { - console.log(JSON.stringify(result)); - expect(result).toEqual({ [routineRoute]: { waypoints: { From 209784ee5c578d3b13e47f6e2119e8bec47e3de4 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 28 Mar 2024 11:22:24 +0100 Subject: [PATCH 05/27] simplify return in MemoryOnlyProvider --- lib/storage/providers/MemoryOnlyProvider.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/storage/providers/MemoryOnlyProvider.ts b/lib/storage/providers/MemoryOnlyProvider.ts index 74d9e3a8..3c7f61f2 100644 --- a/lib/storage/providers/MemoryOnlyProvider.ts +++ b/lib/storage/providers/MemoryOnlyProvider.ts @@ -68,11 +68,7 @@ const provider: StorageProvider = { multiSet(pairs) { const setPromises = _.map(pairs, ([key, value]) => this.setItem(key, value)); - return new Promise((resolve) => { - Promise.all(setPromises).then(() => { - resolve(undefined); - }); - }); + return Promise.all(setPromises).then(() => undefined); }, /** @@ -89,6 +85,8 @@ const provider: StorageProvider = { */ multiMerge(pairs) { _.forEach(pairs, ([key, value]) => { + console.log({key, value}); + const existingValue = store[key] as Record; const newValue = utils.fastMerge(existingValue, value as Record) as OnyxValue; From 9c72a6d97c97a245eafcffd8aba64854cf6dbe25 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 28 Mar 2024 11:54:18 +0100 Subject: [PATCH 06/27] remove log --- lib/storage/providers/MemoryOnlyProvider.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/storage/providers/MemoryOnlyProvider.ts b/lib/storage/providers/MemoryOnlyProvider.ts index 3c7f61f2..8446ba8a 100644 --- a/lib/storage/providers/MemoryOnlyProvider.ts +++ b/lib/storage/providers/MemoryOnlyProvider.ts @@ -85,8 +85,6 @@ const provider: StorageProvider = { */ multiMerge(pairs) { _.forEach(pairs, ([key, value]) => { - console.log({key, value}); - const existingValue = store[key] as Record; const newValue = utils.fastMerge(existingValue, value as Record) as OnyxValue; From c089aa573162ffd6ea034c71fcfcaa5aa1d1777c Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 28 Mar 2024 15:08:47 +0100 Subject: [PATCH 07/27] remove nulls from cached collections --- lib/OnyxUtils.js | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/OnyxUtils.js b/lib/OnyxUtils.js index 2aba8d8f..62e6bb11 100644 --- a/lib/OnyxUtils.js +++ b/lib/OnyxUtils.js @@ -201,6 +201,8 @@ const reduceCollectionWithSelector = (collection, selector, withOnyxInstanceStat function get(key) { // When we already have the value in cache - resolve right away if (cache.hasCacheForKey(key)) { + // TODO: fix this + // const cachedValueWithoutNestedNulls = utils.removeNestedNullValues(cache.getValue(key)); return Promise.resolve(cache.getValue(key)); } @@ -518,6 +520,8 @@ function keysChanged(collectionKey, partialCollection, notifyRegularSubscibers = // We prepare the "cached collection" which is the entire collection + the new partial data that // was merged in via mergeCollection(). const cachedCollection = getCachedCollection(collectionKey); + const cachedCollectionWithoutNestedNulls = utils.removeNestedNullValues(cachedCollection); + // const cachedCollectionWithoutNestedNulls = cachedCollection; // Regular Onyx.connect() subscriber found. if (_.isFunction(subscriber.callback)) { @@ -529,7 +533,7 @@ function keysChanged(collectionKey, partialCollection, notifyRegularSubscibers = // send the whole cached collection. if (isSubscribedToCollectionKey) { if (subscriber.waitForCollectionCallback) { - subscriber.callback(cachedCollection); + subscriber.callback(cachedCollectionWithoutNestedNulls); continue; } @@ -538,7 +542,7 @@ function keysChanged(collectionKey, partialCollection, notifyRegularSubscibers = const dataKeys = _.keys(partialCollection); for (let j = 0; j < dataKeys.length; j++) { const dataKey = dataKeys[j]; - subscriber.callback(cachedCollection[dataKey], dataKey); + subscriber.callback(cachedCollectionWithoutNestedNulls[dataKey], dataKey); } continue; } @@ -546,7 +550,7 @@ function keysChanged(collectionKey, partialCollection, notifyRegularSubscibers = // And if the subscriber is specifically only tracking a particular collection member key then we will // notify them with the cached data for that key only. if (isSubscribedToCollectionMemberKey) { - subscriber.callback(cachedCollection[subscriber.key], subscriber.key); + subscriber.callback(cachedCollectionWithoutNestedNulls[subscriber.key], subscriber.key); continue; } @@ -567,7 +571,7 @@ function keysChanged(collectionKey, partialCollection, notifyRegularSubscibers = if (subscriber.selector) { subscriber.withOnyxInstance.setStateProxy((prevState) => { const previousData = prevState[subscriber.statePropertyName]; - const newData = reduceCollectionWithSelector(cachedCollection, subscriber.selector, subscriber.withOnyxInstance.state); + const newData = reduceCollectionWithSelector(cachedCollectionWithoutNestedNulls, subscriber.selector, subscriber.withOnyxInstance.state); if (!deepEqual(previousData, newData)) { return { @@ -584,7 +588,7 @@ function keysChanged(collectionKey, partialCollection, notifyRegularSubscibers = const dataKeys = _.keys(partialCollection); for (let j = 0; j < dataKeys.length; j++) { const dataKey = dataKeys[j]; - finalCollection[dataKey] = cachedCollection[dataKey]; + finalCollection[dataKey] = cachedCollectionWithoutNestedNulls[dataKey]; } PerformanceUtils.logSetStateCall(subscriber, prevState[subscriber.statePropertyName], finalCollection, 'keysChanged', collectionKey); @@ -610,7 +614,7 @@ function keysChanged(collectionKey, partialCollection, notifyRegularSubscibers = if (subscriber.selector) { subscriber.withOnyxInstance.setStateProxy((prevState) => { const prevData = prevState[subscriber.statePropertyName]; - const newData = getSubsetOfData(cachedCollection[subscriber.key], subscriber.selector, subscriber.withOnyxInstance.state); + const newData = getSubsetOfData(cachedCollectionWithoutNestedNulls[subscriber.key], subscriber.selector, subscriber.withOnyxInstance.state); if (!deepEqual(prevData, newData)) { PerformanceUtils.logSetStateCall(subscriber, prevData, newData, 'keysChanged', collectionKey); return { @@ -624,6 +628,7 @@ function keysChanged(collectionKey, partialCollection, notifyRegularSubscibers = } subscriber.withOnyxInstance.setStateProxy((prevState) => { + // TODO: Fix this const data = cachedCollection[subscriber.key]; const previousData = prevState[subscriber.statePropertyName]; @@ -684,8 +689,9 @@ function keyChanged(key, data, prevData, canUpdateSubscriber = () => true, notif } if (isCollectionKey(subscriber.key) && subscriber.waitForCollectionCallback) { const cachedCollection = getCachedCollection(subscriber.key); - cachedCollection[key] = data; - subscriber.callback(cachedCollection); + const cachedCollectionWithoutNestedNulls = utils.removeNestedNullValues(cachedCollection); + cachedCollectionWithoutNestedNulls[key] = data; + subscriber.callback(cachedCollectionWithoutNestedNulls); continue; } From c403842cd3cfad45faf5e6b4fd504bc5991c6e7b Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 28 Mar 2024 15:16:31 +0100 Subject: [PATCH 08/27] fix: remove invalid null removals (for comparison) --- lib/OnyxUtils.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/OnyxUtils.js b/lib/OnyxUtils.js index 62e6bb11..34f04912 100644 --- a/lib/OnyxUtils.js +++ b/lib/OnyxUtils.js @@ -571,7 +571,7 @@ function keysChanged(collectionKey, partialCollection, notifyRegularSubscibers = if (subscriber.selector) { subscriber.withOnyxInstance.setStateProxy((prevState) => { const previousData = prevState[subscriber.statePropertyName]; - const newData = reduceCollectionWithSelector(cachedCollectionWithoutNestedNulls, subscriber.selector, subscriber.withOnyxInstance.state); + const newData = reduceCollectionWithSelector(cachedCollection, subscriber.selector, subscriber.withOnyxInstance.state); if (!deepEqual(previousData, newData)) { return { @@ -588,7 +588,7 @@ function keysChanged(collectionKey, partialCollection, notifyRegularSubscibers = const dataKeys = _.keys(partialCollection); for (let j = 0; j < dataKeys.length; j++) { const dataKey = dataKeys[j]; - finalCollection[dataKey] = cachedCollectionWithoutNestedNulls[dataKey]; + finalCollection[dataKey] = cachedCollection[dataKey]; } PerformanceUtils.logSetStateCall(subscriber, prevState[subscriber.statePropertyName], finalCollection, 'keysChanged', collectionKey); @@ -614,7 +614,7 @@ function keysChanged(collectionKey, partialCollection, notifyRegularSubscibers = if (subscriber.selector) { subscriber.withOnyxInstance.setStateProxy((prevState) => { const prevData = prevState[subscriber.statePropertyName]; - const newData = getSubsetOfData(cachedCollectionWithoutNestedNulls[subscriber.key], subscriber.selector, subscriber.withOnyxInstance.state); + const newData = getSubsetOfData(cachedCollection[subscriber.key], subscriber.selector, subscriber.withOnyxInstance.state); if (!deepEqual(prevData, newData)) { PerformanceUtils.logSetStateCall(subscriber, prevData, newData, 'keysChanged', collectionKey); return { @@ -628,7 +628,6 @@ function keysChanged(collectionKey, partialCollection, notifyRegularSubscibers = } subscriber.withOnyxInstance.setStateProxy((prevState) => { - // TODO: Fix this const data = cachedCollection[subscriber.key]; const previousData = prevState[subscriber.statePropertyName]; From 325e205ef701d80c90d08912784090932f9921ad Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 28 Mar 2024 15:27:58 +0100 Subject: [PATCH 09/27] fix remaining callback calls --- lib/OnyxUtils.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/OnyxUtils.js b/lib/OnyxUtils.js index 34f04912..675a4f6a 100644 --- a/lib/OnyxUtils.js +++ b/lib/OnyxUtils.js @@ -201,8 +201,6 @@ const reduceCollectionWithSelector = (collection, selector, withOnyxInstanceStat function get(key) { // When we already have the value in cache - resolve right away if (cache.hasCacheForKey(key)) { - // TODO: fix this - // const cachedValueWithoutNestedNulls = utils.removeNestedNullValues(cache.getValue(key)); return Promise.resolve(cache.getValue(key)); } @@ -694,7 +692,8 @@ function keyChanged(key, data, prevData, canUpdateSubscriber = () => true, notif continue; } - subscriber.callback(data, key); + const dataWithoutNestedNulls = utils.removeNestedNullValues(data); + subscriber.callback(dataWithoutNestedNulls, key); continue; } @@ -831,7 +830,8 @@ function sendDataToConnection(mapping, val, matchedKey, isBatched) { } if (_.isFunction(mapping.callback)) { - mapping.callback(val, matchedKey); + const valWithoutNestedNulls = utils.removeNestedNullValues(val); + mapping.callback(valWithoutNestedNulls, matchedKey); } } From ef053d3d785437d146c23ca3346bee49842a1484 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 28 Mar 2024 15:41:38 +0100 Subject: [PATCH 10/27] never return null values to the user in withOnyx --- lib/withOnyx.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/withOnyx.js b/lib/withOnyx.js index c89838db..32ced492 100644 --- a/lib/withOnyx.js +++ b/lib/withOnyx.js @@ -312,6 +312,7 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) { render() { // Remove any null values so that React replaces them with default props const propsToPass = _.omit(this.props, _.isNull); + const propsToPassWithoutNestedNulls = utils.removeNestedNullValues(propsToPass); if (this.state.loading) { return null; @@ -321,15 +322,16 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) { // that should not be passed to a wrapped component let stateToPass = _.omit(this.state, 'loading'); stateToPass = _.omit(stateToPass, _.isNull); + const stateToPassWithoutNestedNulls = utils.removeNestedNullValues(stateToPass); // Spreading props and state is necessary in an HOC where the data cannot be predicted return ( ); From 0727ec9c48d45d009d87e193f735d1b8612bc135 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 28 Mar 2024 15:57:43 +0100 Subject: [PATCH 11/27] rename parameter name --- lib/Onyx.ts | 2 +- lib/utils.ts | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index eab90404..f1eff25d 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -326,7 +326,7 @@ function merge(key: TKey, changes: OnyxEntry { * Merges the source object into the target object. * @param target - The target object. * @param source - The source object. - * @param shouldRemoveNullObjectValues - If true, null object values will be removed. + * @param shouldRemoveNestedNullsInObjects - If true, null object values will be removed. * @returns - The merged object. */ -function mergeObject>(target: TObject | null, source: TObject, shouldRemoveNullObjectValues = true): TObject { +function mergeObject>(target: TObject | null, source: TObject, shouldRemoveNestedNullsInObjects = true): TObject { const destination: Record = {}; // First we want to copy over all keys from the target into the destination object, // in case "target" is a mergable object. - // If "shouldRemoveNullObjectValues" is true, we want to remove null values from the merged object + // If "shouldRemoveNestedNullsInObjects" is true, we want to remove null values from the merged object // and therefore we need to omit keys where either the source or target value is null. if (isMergeableObject(target)) { const targetKeys = Object.keys(target); @@ -41,10 +41,10 @@ function mergeObject>(target: TObject | const sourceValue = source?.[key]; const targetValue = target?.[key]; - // If "shouldRemoveNullObjectValues" is true, we want to remove null values from the merged object. + // If "shouldRemoveNestedNullsInObjects" is true, we want to remove null values from the merged object. // Therefore, if either target or source value is null, we want to prevent the key from being set. const isSourceOrTargetNull = targetValue === null || sourceValue === null; - const shouldOmitTargetKey = shouldRemoveNullObjectValues && isSourceOrTargetNull; + const shouldOmitTargetKey = shouldRemoveNestedNullsInObjects && isSourceOrTargetNull; if (!shouldOmitTargetKey) { destination[key] = targetValue; @@ -60,14 +60,14 @@ function mergeObject>(target: TObject | const targetValue = target?.[key]; // If undefined is passed as the source value for a key, we want to generally ignore it. - // If "shouldRemoveNullObjectValues" is set to true and the source value is null, + // If "shouldRemoveNestedNullsInObjects" is set to true and the source value is null, // we don't want to set/merge the source value into the merged object. - const shouldIgnoreNullSourceValue = shouldRemoveNullObjectValues && sourceValue === null; + const shouldIgnoreNullSourceValue = shouldRemoveNestedNullsInObjects && sourceValue === null; const shouldOmitSourceKey = sourceValue === undefined || shouldIgnoreNullSourceValue; if (!shouldOmitSourceKey) { // If the source value is a mergable object, we want to merge it into the target value. - // If "shouldRemoveNullObjectValues" is true, "fastMerge" will recursively + // If "shouldRemoveNestedNullsInObjects" is true, "fastMerge" will recursively // remove nested null values from the merged object. // If source value is any other value we need to set the source value it directly. if (isMergeableObject(sourceValue)) { @@ -75,7 +75,7 @@ function mergeObject>(target: TObject | // so that we can still use "fastMerge" to merge the source value, // to ensure that nested null values are removed from the merged object. const targetValueWithFallback = (targetValue ?? {}) as TObject; - destination[key] = fastMerge(targetValueWithFallback, sourceValue, shouldRemoveNullObjectValues); + destination[key] = fastMerge(targetValueWithFallback, sourceValue, shouldRemoveNestedNullsInObjects); } else { destination[key] = sourceValue; } @@ -86,13 +86,13 @@ function mergeObject>(target: TObject | } /** - * Merges two objects and removes null values if "shouldRemoveNullObjectValues" is set to true + * Merges two objects and removes null values if "shouldRemoveNestedNullsInObjects" is set to true * * We generally want to remove null values from objects written to disk and cache, because it decreases the amount of data stored in memory and on disk. * On native, when merging an existing value with new changes, SQLite will use JSON_PATCH, which removes top-level nullish values. * To be consistent with the behaviour for merge, we'll also want to remove null values for "set" operations. */ -function fastMerge>(target: TObject | null, source: TObject | null, shouldRemoveNullObjectValues = true): TObject | null { +function fastMerge>(target: TObject | null, source: TObject | null, shouldRemoveNestedNullsInObjects = true): TObject | null { // We have to ignore arrays and nullish values here, // otherwise "mergeObject" will throw an error, // because it expects an object as "source" @@ -100,7 +100,7 @@ function fastMerge>(target: TObject | nu return source; } - return mergeObject(target, source, shouldRemoveNullObjectValues); + return mergeObject(target, source, shouldRemoveNestedNullsInObjects); } /** Deep removes the nested null values from the given value. */ From f0c266810c0d436b8b56d6b4b67accfa06fbb617 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 28 Mar 2024 16:00:11 +0100 Subject: [PATCH 12/27] simplify prepareKeyValuePairsForStorage function based on @paultsimura 's approach in #515 --- lib/OnyxUtils.js | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/OnyxUtils.js b/lib/OnyxUtils.js index 675a4f6a..738a744c 100644 --- a/lib/OnyxUtils.js +++ b/lib/OnyxUtils.js @@ -1096,17 +1096,19 @@ function removeNullValues(key, value, shouldRemoveNestedNullsInObjects = true) { * @return {Array} an array of key - value pairs <[key, value]> */ function prepareKeyValuePairsForStorage(data, shouldRemoveNestedNullsInObjects) { - const keyValuePairs = []; - - _.forEach(data, (value, key) => { - const {value: valueAfterRemoving, wasRemoved} = removeNullValues(key, value, shouldRemoveNestedNullsInObjects); - - if (wasRemoved) return; + return _.reduce( + data, + (pairs, value, key) => { + const {value: valueAfterRemoving, wasRemoved} = removeNullValues(key, value, shouldRemoveNestedNullsInObjects); - keyValuePairs.push([key, valueAfterRemoving]); - }); + if (!wasRemoved) { + pairs.push([key, valueAfterRemoving]); + } - return keyValuePairs; + return pairs; + }, + [], + ); } /** From 28ca063756f0ef6b457aa3bf30227de20b4e8d75 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 28 Mar 2024 16:00:16 +0100 Subject: [PATCH 13/27] fix: comments --- lib/Onyx.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index f1eff25d..aa382559 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -282,7 +282,7 @@ function merge(key: TKey, changes: OnyxEntry(key: TKey, changes: OnyxEntry Date: Thu, 28 Mar 2024 16:06:08 +0100 Subject: [PATCH 14/27] simplify tests and add one for mergeCollection --- tests/unit/onyxTest.js | 567 ++++++++++++++++++++++------------------- 1 file changed, 306 insertions(+), 261 deletions(-) diff --git a/tests/unit/onyxTest.js b/tests/unit/onyxTest.js index 6d58ebe9..30428df3 100644 --- a/tests/unit/onyxTest.js +++ b/tests/unit/onyxTest.js @@ -2,6 +2,7 @@ import _ from 'underscore'; import Onyx from '../../lib'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; import OnyxUtils from '../../lib/OnyxUtils'; +import StorageMock from '../../lib/storage/__mocks__'; const ONYX_KEYS = { TEST_KEY: 'test', @@ -37,60 +38,323 @@ describe('Onyx', () => { return Onyx.clear(); }); - it('should remove key value from OnyxCache/Storage when set is called with null value', () => - Onyx.set(ONYX_KEYS.OTHER_TEST, 42) - .then(() => OnyxUtils.getAllKeys()) - .then((keys) => { - expect(keys.has(ONYX_KEYS.OTHER_TEST)).toBe(true); - return Onyx.set(ONYX_KEYS.OTHER_TEST, null); - }) - .then(() => { - // Checks if cache value is removed. - expect(cache.getAllKeys().size).toBe(0); - - // When cache keys length is 0, we fetch the keys from storage. - return OnyxUtils.getAllKeys(); - }) - .then((keys) => { - expect(keys.has(ONYX_KEYS.OTHER_TEST)).toBe(false); - })); + describe('set', () => { + it('should remove key value from OnyxCache/Storage when set is called with null value', () => + Onyx.set(ONYX_KEYS.OTHER_TEST, 42) + .then(() => OnyxUtils.getAllKeys()) + .then((keys) => { + expect(keys.has(ONYX_KEYS.OTHER_TEST)).toBe(true); + return Onyx.set(ONYX_KEYS.OTHER_TEST, null); + }) + .then(() => { + // Checks if cache value is removed. + expect(cache.getAllKeys().size).toBe(0); - it('should set a simple key', () => { - let testKeyValue; + // When cache keys length is 0, we fetch the keys from storage. + return OnyxUtils.getAllKeys(); + }) + .then((keys) => { + expect(keys.has(ONYX_KEYS.OTHER_TEST)).toBe(false); + })); + + it('should set a simple key', () => { + let testKeyValue; + + connectionID = Onyx.connect({ + key: ONYX_KEYS.TEST_KEY, + initWithStoredValues: false, + callback: (value) => { + testKeyValue = value; + }, + }); - connectionID = Onyx.connect({ - key: ONYX_KEYS.TEST_KEY, - initWithStoredValues: false, - callback: (value) => { - testKeyValue = value; - }, + // Set a simple key + return Onyx.set(ONYX_KEYS.TEST_KEY, 'test').then(() => { + expect(testKeyValue).toBe('test'); + }); }); + }); - // Set a simple key - return Onyx.set(ONYX_KEYS.TEST_KEY, 'test').then(() => { - expect(testKeyValue).toBe('test'); + describe('merge', () => { + it('should merge an object with another object', () => { + let testKeyValue; + + connectionID = Onyx.connect({ + key: ONYX_KEYS.TEST_KEY, + initWithStoredValues: false, + callback: (value) => { + testKeyValue = value; + }, + }); + + return Onyx.set(ONYX_KEYS.TEST_KEY, {test1: 'test1'}) + .then(() => { + expect(testKeyValue).toEqual({test1: 'test1'}); + return Onyx.merge(ONYX_KEYS.TEST_KEY, {test2: 'test2'}); + }) + .then(() => { + expect(testKeyValue).toEqual({test1: 'test1', test2: 'test2'}); + }); }); }); - it('should merge an object with another object', () => { - let testKeyValue; + describe('clear', () => {}); - connectionID = Onyx.connect({ - key: ONYX_KEYS.TEST_KEY, - initWithStoredValues: false, - callback: (value) => { - testKeyValue = value; - }, + describe('mergeCollection', () => { + it('should call subscriber callback and storage multiMerge with the same data', () => { + const storageSpy = jest.spyOn(StorageMock, 'multiMerge'); + const routineRoute = `${ONYX_KEYS.COLLECTION.ROUTES}routine`; + + let result; + connectionID = Onyx.connect({ + key: routineRoute, + initWithStoredValues: false, + callback: (value) => (result = value), + }); + + const initialValue = { + pendingFields: { + waypoints: 'add', + }, + }; + const updatedValue = { + pendingFields: { + waypoints: null, + }, + }; + + return Onyx.set(routineRoute, initialValue) + .then(() => { + expect(result).toEqual(initialValue); + Onyx.mergeCollection(ONYX_KEYS.COLLECTION.ROUTES, { + [routineRoute]: updatedValue, + }); + return waitForPromisesToResolve(); + }) + .then(() => { + expect(result).toEqual(updatedValue); + expect(storageSpy).toHaveBeenCalledWith([[routineRoute, updatedValue]]); + }); }); - return Onyx.set(ONYX_KEYS.TEST_KEY, {test1: 'test1'}) - .then(() => { - expect(testKeyValue).toEqual({test1: 'test1'}); - return Onyx.merge(ONYX_KEYS.TEST_KEY, {test2: 'test2'}); + it('should properly set and merge when using mergeCollection', () => { + const valuesReceived = {}; + const mockCallback = jest.fn((data) => (valuesReceived[data.ID] = data.value)); + connectionID = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_KEY, + initWithStoredValues: false, + callback: mockCallback, + }); + + // The first time we call mergeCollection we'll be doing a multiSet internally + return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { + test_1: { + ID: 123, + value: 'one', + }, + test_2: { + ID: 234, + value: 'two', + }, + test_3: { + ID: 345, + value: 'three', + }, }) - .then(() => { - expect(testKeyValue).toEqual({test1: 'test1', test2: 'test2'}); + .then(() => + // 2 key values to update and 2 new keys to add. + // MergeCollection will perform a mix of multiSet and multiMerge + Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { + test_1: { + ID: 123, + value: 'five', + }, + test_2: { + ID: 234, + value: 'four', + }, + test_4: { + ID: 456, + value: 'two', + }, + test_5: { + ID: 567, + value: 'one', + }, + }), + ) + .then(() => { + // 3 items on the first mergeCollection + 4 items the next mergeCollection + expect(mockCallback).toHaveBeenCalledTimes(7); + expect(mockCallback).toHaveBeenNthCalledWith(1, {ID: 123, value: 'one'}, 'test_1'); + expect(mockCallback).toHaveBeenNthCalledWith(2, {ID: 234, value: 'two'}, 'test_2'); + expect(mockCallback).toHaveBeenNthCalledWith(3, {ID: 345, value: 'three'}, 'test_3'); + expect(mockCallback).toHaveBeenNthCalledWith(4, {ID: 123, value: 'five'}, 'test_1'); + expect(mockCallback).toHaveBeenNthCalledWith(5, {ID: 234, value: 'four'}, 'test_2'); + expect(mockCallback).toHaveBeenNthCalledWith(6, {ID: 456, value: 'two'}, 'test_4'); + expect(mockCallback).toHaveBeenNthCalledWith(7, {ID: 567, value: 'one'}, 'test_5'); + expect(valuesReceived[123]).toEqual('five'); + expect(valuesReceived[234]).toEqual('four'); + expect(valuesReceived[345]).toEqual('three'); + expect(valuesReceived[456]).toEqual('two'); + expect(valuesReceived[567]).toEqual('one'); + }); + }); + + it('should skip the update when a key not belonging to collection key is present in mergeCollection', () => { + const valuesReceived = {}; + connectionID = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_KEY, + initWithStoredValues: false, + callback: (data, key) => (valuesReceived[key] = data), + }); + + return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {test_1: {ID: 123}, notMyTest: {beep: 'boop'}}).then(() => { + expect(valuesReceived).toEqual({}); + }); + }); + + it('should return full object to callback when calling mergeCollection()', () => { + const valuesReceived = {}; + connectionID = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_KEY, + initWithStoredValues: false, + callback: (data, key) => (valuesReceived[key] = data), + }); + + return Onyx.multiSet({ + test_1: { + existingData: 'test', + }, + test_2: { + existingData: 'test', + }, + }) + .then(() => + Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { + test_1: { + ID: 123, + value: 'one', + }, + test_2: { + ID: 234, + value: 'two', + }, + }), + ) + .then(() => { + expect(valuesReceived).toEqual({ + test_1: { + ID: 123, + value: 'one', + existingData: 'test', + }, + test_2: { + ID: 234, + value: 'two', + existingData: 'test', + }, + }); + }); + }); + + it('should use update data object to set/merge keys', () => { + let testKeyValue; + connectionID = Onyx.connect({ + key: ONYX_KEYS.TEST_KEY, + initWithStoredValues: false, + callback: (value) => { + testKeyValue = value; + }, + }); + + let otherTestKeyValue; + connectionID = Onyx.connect({ + key: ONYX_KEYS.OTHER_TEST, + initWithStoredValues: false, + callback: (value) => { + otherTestKeyValue = value; + }, }); + + return waitForPromisesToResolve() + .then(() => { + // Given the initial Onyx state: {test: true, otherTest: {test1: 'test1'}} + Onyx.set(ONYX_KEYS.TEST_KEY, true); + Onyx.set(ONYX_KEYS.OTHER_TEST, {test1: 'test1'}); + return waitForPromisesToResolve(); + }) + .then(() => { + expect(testKeyValue).toBe(true); + expect(otherTestKeyValue).toEqual({test1: 'test1'}); + + // When we pass a data object to Onyx.update + return Onyx.update([ + { + onyxMethod: 'set', + key: ONYX_KEYS.TEST_KEY, + value: 'one', + }, + { + onyxMethod: 'merge', + key: ONYX_KEYS.OTHER_TEST, + value: {test2: 'test2'}, + }, + ]); + }) + .then(() => { + // Then the final Onyx state should be {test: 'one', otherTest: {test1: 'test1', test2: 'test2'}} + expect(testKeyValue).toBe('one'); + expect(otherTestKeyValue).toEqual({test1: 'test1', test2: 'test2'}); + }); + }); + + it('mergeCollection should omit nested null values', () => { + let result; + + const routineRoute = `${ONYX_KEYS.COLLECTION.TEST_KEY}routine`; + const holidayRoute = `${ONYX_KEYS.COLLECTION.TEST_KEY}holiday`; + + connectionID = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_KEY, + initWithStoredValues: false, + callback: (value) => (result = value), + waitForCollectionCallback: true, + }); + + return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { + [routineRoute]: { + waypoints: { + 1: 'Home', + 2: 'Work', + 3: 'Gym', + }, + }, + [holidayRoute]: { + waypoints: { + 1: 'Home', + 2: 'Beach', + 3: null, + }, + }, + }).then(() => { + expect(result).toEqual({ + [routineRoute]: { + waypoints: { + 1: 'Home', + 2: 'Work', + 3: 'Gym', + }, + }, + [holidayRoute]: { + waypoints: { + 1: 'Home', + 2: 'Beach', + }, + }, + }); + }); + }); }); it('should notify subscribers when data has been cleared', () => { @@ -353,178 +617,6 @@ describe('Onyx', () => { }); }); - it('should properly set and merge when using mergeCollection', () => { - const valuesReceived = {}; - const mockCallback = jest.fn((data) => (valuesReceived[data.ID] = data.value)); - connectionID = Onyx.connect({ - key: ONYX_KEYS.COLLECTION.TEST_KEY, - initWithStoredValues: false, - callback: mockCallback, - }); - - // The first time we call mergeCollection we'll be doing a multiSet internally - return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { - test_1: { - ID: 123, - value: 'one', - }, - test_2: { - ID: 234, - value: 'two', - }, - test_3: { - ID: 345, - value: 'three', - }, - }) - .then(() => - // 2 key values to update and 2 new keys to add. - // MergeCollection will perform a mix of multiSet and multiMerge - Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { - test_1: { - ID: 123, - value: 'five', - }, - test_2: { - ID: 234, - value: 'four', - }, - test_4: { - ID: 456, - value: 'two', - }, - test_5: { - ID: 567, - value: 'one', - }, - }), - ) - .then(() => { - // 3 items on the first mergeCollection + 4 items the next mergeCollection - expect(mockCallback).toHaveBeenCalledTimes(7); - expect(mockCallback).toHaveBeenNthCalledWith(1, {ID: 123, value: 'one'}, 'test_1'); - expect(mockCallback).toHaveBeenNthCalledWith(2, {ID: 234, value: 'two'}, 'test_2'); - expect(mockCallback).toHaveBeenNthCalledWith(3, {ID: 345, value: 'three'}, 'test_3'); - expect(mockCallback).toHaveBeenNthCalledWith(4, {ID: 123, value: 'five'}, 'test_1'); - expect(mockCallback).toHaveBeenNthCalledWith(5, {ID: 234, value: 'four'}, 'test_2'); - expect(mockCallback).toHaveBeenNthCalledWith(6, {ID: 456, value: 'two'}, 'test_4'); - expect(mockCallback).toHaveBeenNthCalledWith(7, {ID: 567, value: 'one'}, 'test_5'); - expect(valuesReceived[123]).toEqual('five'); - expect(valuesReceived[234]).toEqual('four'); - expect(valuesReceived[345]).toEqual('three'); - expect(valuesReceived[456]).toEqual('two'); - expect(valuesReceived[567]).toEqual('one'); - }); - }); - - it('should skip the update when a key not belonging to collection key is present in mergeCollection', () => { - const valuesReceived = {}; - connectionID = Onyx.connect({ - key: ONYX_KEYS.COLLECTION.TEST_KEY, - initWithStoredValues: false, - callback: (data, key) => (valuesReceived[key] = data), - }); - - return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {test_1: {ID: 123}, notMyTest: {beep: 'boop'}}).then(() => { - expect(valuesReceived).toEqual({}); - }); - }); - - it('should return full object to callback when calling mergeCollection()', () => { - const valuesReceived = {}; - connectionID = Onyx.connect({ - key: ONYX_KEYS.COLLECTION.TEST_KEY, - initWithStoredValues: false, - callback: (data, key) => (valuesReceived[key] = data), - }); - - return Onyx.multiSet({ - test_1: { - existingData: 'test', - }, - test_2: { - existingData: 'test', - }, - }) - .then(() => - Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { - test_1: { - ID: 123, - value: 'one', - }, - test_2: { - ID: 234, - value: 'two', - }, - }), - ) - .then(() => { - expect(valuesReceived).toEqual({ - test_1: { - ID: 123, - value: 'one', - existingData: 'test', - }, - test_2: { - ID: 234, - value: 'two', - existingData: 'test', - }, - }); - }); - }); - - it('should use update data object to set/merge keys', () => { - let testKeyValue; - connectionID = Onyx.connect({ - key: ONYX_KEYS.TEST_KEY, - initWithStoredValues: false, - callback: (value) => { - testKeyValue = value; - }, - }); - - let otherTestKeyValue; - connectionID = Onyx.connect({ - key: ONYX_KEYS.OTHER_TEST, - initWithStoredValues: false, - callback: (value) => { - otherTestKeyValue = value; - }, - }); - - return waitForPromisesToResolve() - .then(() => { - // Given the initial Onyx state: {test: true, otherTest: {test1: 'test1'}} - Onyx.set(ONYX_KEYS.TEST_KEY, true); - Onyx.set(ONYX_KEYS.OTHER_TEST, {test1: 'test1'}); - return waitForPromisesToResolve(); - }) - .then(() => { - expect(testKeyValue).toBe(true); - expect(otherTestKeyValue).toEqual({test1: 'test1'}); - - // When we pass a data object to Onyx.update - return Onyx.update([ - { - onyxMethod: 'set', - key: ONYX_KEYS.TEST_KEY, - value: 'one', - }, - { - onyxMethod: 'merge', - key: ONYX_KEYS.OTHER_TEST, - value: {test2: 'test2'}, - }, - ]); - }) - .then(() => { - // Then the final Onyx state should be {test: 'one', otherTest: {test1: 'test1', test2: 'test2'}} - expect(testKeyValue).toBe('one'); - expect(otherTestKeyValue).toEqual({test1: 'test1', test2: 'test2'}); - }); - }); - it('should use update data object to merge a collection of keys', () => { const valuesReceived = {}; const mockCallback = jest.fn((data) => (valuesReceived[data.ID] = data.value)); @@ -1040,53 +1132,6 @@ describe('Onyx', () => { }); }); - it('mergeCollection should omit nested null values', () => { - let result; - - const routineRoute = `${ONYX_KEYS.COLLECTION.TEST_KEY}routine`; - const holidayRoute = `${ONYX_KEYS.COLLECTION.TEST_KEY}holiday`; - - connectionID = Onyx.connect({ - key: ONYX_KEYS.COLLECTION.TEST_KEY, - initWithStoredValues: false, - callback: (value) => (result = value), - waitForCollectionCallback: true, - }); - - return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { - [routineRoute]: { - waypoints: { - 1: 'Home', - 2: 'Work', - 3: 'Gym', - }, - }, - [holidayRoute]: { - waypoints: { - 1: 'Home', - 2: 'Beach', - 3: null, - }, - }, - }).then(() => { - expect(result).toEqual({ - [routineRoute]: { - waypoints: { - 1: 'Home', - 2: 'Work', - 3: 'Gym', - }, - }, - [holidayRoute]: { - waypoints: { - 1: 'Home', - 2: 'Beach', - }, - }, - }); - }); - }); - it('should apply updates in order with Onyx.update', () => { let testKeyValue; From 60766dd40a93bcd73c46139f0728eb50336f3d5f Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 28 Mar 2024 16:18:17 +0100 Subject: [PATCH 15/27] rename parameter --- lib/Onyx.ts | 2 +- lib/OnyxUtils.d.ts | 12 ++++++------ lib/OnyxUtils.js | 18 +++++++++--------- lib/utils.ts | 24 ++++++++++++------------ 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index aa382559..94bd2a4b 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -326,7 +326,7 @@ function merge(key: TKey, changes: OnyxEntry( key: TKey, value: OnyxValue, - shouldRemoveNestedNullsInObjects?: boolean, + shouldRemoveNestedNulls?: boolean, ): { value: OnyxValue; wasRemoved: boolean; @@ -266,20 +266,20 @@ declare function removeNullValues( * to an array of key-value pairs in the above format and removes key-value pairs that are being set to null * @private * @param {Record} data - * @param {Boolean} shouldRemoveNestedNullsInObjects + * @param {Boolean} shouldRemoveNestedNulls * @return {Array} an array of key - value pairs <[key, value]> */ -declare function prepareKeyValuePairsForStorage(data: Record>, shouldRemoveNestedNullsInObjects: boolean): Array<[OnyxKey, OnyxValue]>; +declare function prepareKeyValuePairsForStorage(data: Record>, shouldRemoveNestedNulls: boolean): Array<[OnyxKey, OnyxValue]>; /** * Merges an array of changes with an existing value * * @private * @param {*} existingValue * @param {Array<*>} changes Array of changes that should be applied to the existing value - * @param {Boolean} shouldRemoveNestedNullsInObjects + * @param {Boolean} shouldRemoveNestedNulls * @returns {*} */ -declare function applyMerge(existingValue: OnyxValue, changes: Array>, shouldRemoveNestedNullsInObjects: boolean): any; +declare function applyMerge(existingValue: OnyxValue, changes: Array>, shouldRemoveNestedNulls: boolean): any; /** * Merge user provided default key value pairs. */ diff --git a/lib/OnyxUtils.js b/lib/OnyxUtils.js index 738a744c..6ebb963a 100644 --- a/lib/OnyxUtils.js +++ b/lib/OnyxUtils.js @@ -1071,10 +1071,10 @@ function hasPendingMergeForKey(key) { * Otherwise removes all nested null values in objects and returns the object * @param {String} key * @param {Mixed} value - * @param {Boolean} shouldRemoveNestedNullsInObjects + * @param {Boolean} shouldRemoveNestedNulls * @returns {Mixed} The value without null values and a boolean "wasRemoved", which indicates if the key got removed completely */ -function removeNullValues(key, value, shouldRemoveNestedNullsInObjects = true) { +function removeNullValues(key, value, shouldRemoveNestedNulls = true) { if (_.isNull(value)) { remove(key); return {value, wasRemoved: true}; @@ -1083,7 +1083,7 @@ function removeNullValues(key, value, shouldRemoveNestedNullsInObjects = true) { // We can remove all null values in an object by merging it with itself // utils.fastMerge recursively goes through the object and removes all null values // Passing two identical objects as source and target to fastMerge will not change it, but only remove the null values - return {value: shouldRemoveNestedNullsInObjects ? utils.removeNestedNullValues(value) : value, wasRemoved: false}; + return {value: shouldRemoveNestedNulls ? utils.removeNestedNullValues(value) : value, wasRemoved: false}; } /** @@ -1092,14 +1092,14 @@ function removeNullValues(key, value, shouldRemoveNestedNullsInObjects = true) { * to an array of key-value pairs in the above format and removes key-value pairs that are being set to null * @private * @param {Record} data - * @param {Boolean} shouldRemoveNestedNullsInObjects + * @param {Boolean} shouldRemoveNestedNulls * @return {Array} an array of key - value pairs <[key, value]> */ -function prepareKeyValuePairsForStorage(data, shouldRemoveNestedNullsInObjects) { +function prepareKeyValuePairsForStorage(data, shouldRemoveNestedNulls) { return _.reduce( data, (pairs, value, key) => { - const {value: valueAfterRemoving, wasRemoved} = removeNullValues(key, value, shouldRemoveNestedNullsInObjects); + const {value: valueAfterRemoving, wasRemoved} = removeNullValues(key, value, shouldRemoveNestedNulls); if (!wasRemoved) { pairs.push([key, valueAfterRemoving]); @@ -1117,10 +1117,10 @@ function prepareKeyValuePairsForStorage(data, shouldRemoveNestedNullsInObjects) * @private * @param {*} existingValue * @param {Array<*>} changes Array of changes that should be applied to the existing value - * @param {Boolean} shouldRemoveNestedNullsInObjects + * @param {Boolean} shouldRemoveNestedNulls * @returns {*} */ -function applyMerge(existingValue, changes, shouldRemoveNestedNullsInObjects) { +function applyMerge(existingValue, changes, shouldRemoveNestedNulls) { const lastChange = _.last(changes); if (_.isArray(lastChange)) { @@ -1129,7 +1129,7 @@ function applyMerge(existingValue, changes, shouldRemoveNestedNullsInObjects) { if (_.some(changes, _.isObject)) { // Object values are then merged one after the other - return _.reduce(changes, (modifiedData, change) => utils.fastMerge(modifiedData, change, shouldRemoveNestedNullsInObjects), existingValue || {}); + return _.reduce(changes, (modifiedData, change) => utils.fastMerge(modifiedData, change, shouldRemoveNestedNulls), existingValue || {}); } // If we have anything else we can't merge it so we'll diff --git a/lib/utils.ts b/lib/utils.ts index 1834d483..d258a64b 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -24,15 +24,15 @@ function isMergeableObject(value: unknown): value is Record { * Merges the source object into the target object. * @param target - The target object. * @param source - The source object. - * @param shouldRemoveNestedNullsInObjects - If true, null object values will be removed. + * @param shouldRemoveNestedNulls - If true, null object values will be removed. * @returns - The merged object. */ -function mergeObject>(target: TObject | null, source: TObject, shouldRemoveNestedNullsInObjects = true): TObject { +function mergeObject>(target: TObject | null, source: TObject, shouldRemoveNestedNulls = true): TObject { const destination: Record = {}; // First we want to copy over all keys from the target into the destination object, // in case "target" is a mergable object. - // If "shouldRemoveNestedNullsInObjects" is true, we want to remove null values from the merged object + // If "shouldRemoveNestedNulls" is true, we want to remove null values from the merged object // and therefore we need to omit keys where either the source or target value is null. if (isMergeableObject(target)) { const targetKeys = Object.keys(target); @@ -41,10 +41,10 @@ function mergeObject>(target: TObject | const sourceValue = source?.[key]; const targetValue = target?.[key]; - // If "shouldRemoveNestedNullsInObjects" is true, we want to remove null values from the merged object. + // If "shouldRemoveNestedNulls" is true, we want to remove null values from the merged object. // Therefore, if either target or source value is null, we want to prevent the key from being set. const isSourceOrTargetNull = targetValue === null || sourceValue === null; - const shouldOmitTargetKey = shouldRemoveNestedNullsInObjects && isSourceOrTargetNull; + const shouldOmitTargetKey = shouldRemoveNestedNulls && isSourceOrTargetNull; if (!shouldOmitTargetKey) { destination[key] = targetValue; @@ -60,14 +60,14 @@ function mergeObject>(target: TObject | const targetValue = target?.[key]; // If undefined is passed as the source value for a key, we want to generally ignore it. - // If "shouldRemoveNestedNullsInObjects" is set to true and the source value is null, + // If "shouldRemoveNestedNulls" is set to true and the source value is null, // we don't want to set/merge the source value into the merged object. - const shouldIgnoreNullSourceValue = shouldRemoveNestedNullsInObjects && sourceValue === null; + const shouldIgnoreNullSourceValue = shouldRemoveNestedNulls && sourceValue === null; const shouldOmitSourceKey = sourceValue === undefined || shouldIgnoreNullSourceValue; if (!shouldOmitSourceKey) { // If the source value is a mergable object, we want to merge it into the target value. - // If "shouldRemoveNestedNullsInObjects" is true, "fastMerge" will recursively + // If "shouldRemoveNestedNulls" is true, "fastMerge" will recursively // remove nested null values from the merged object. // If source value is any other value we need to set the source value it directly. if (isMergeableObject(sourceValue)) { @@ -75,7 +75,7 @@ function mergeObject>(target: TObject | // so that we can still use "fastMerge" to merge the source value, // to ensure that nested null values are removed from the merged object. const targetValueWithFallback = (targetValue ?? {}) as TObject; - destination[key] = fastMerge(targetValueWithFallback, sourceValue, shouldRemoveNestedNullsInObjects); + destination[key] = fastMerge(targetValueWithFallback, sourceValue, shouldRemoveNestedNulls); } else { destination[key] = sourceValue; } @@ -86,13 +86,13 @@ function mergeObject>(target: TObject | } /** - * Merges two objects and removes null values if "shouldRemoveNestedNullsInObjects" is set to true + * Merges two objects and removes null values if "shouldRemoveNestedNulls" is set to true * * We generally want to remove null values from objects written to disk and cache, because it decreases the amount of data stored in memory and on disk. * On native, when merging an existing value with new changes, SQLite will use JSON_PATCH, which removes top-level nullish values. * To be consistent with the behaviour for merge, we'll also want to remove null values for "set" operations. */ -function fastMerge>(target: TObject | null, source: TObject | null, shouldRemoveNestedNullsInObjects = true): TObject | null { +function fastMerge>(target: TObject | null, source: TObject | null, shouldRemoveNestedNulls = true): TObject | null { // We have to ignore arrays and nullish values here, // otherwise "mergeObject" will throw an error, // because it expects an object as "source" @@ -100,7 +100,7 @@ function fastMerge>(target: TObject | nu return source; } - return mergeObject(target, source, shouldRemoveNestedNullsInObjects); + return mergeObject(target, source, shouldRemoveNestedNulls); } /** Deep removes the nested null values from the given value. */ From 0ceca8cada6680f3fd7b8951811209e631de64fe Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 28 Mar 2024 17:01:23 +0100 Subject: [PATCH 16/27] remove invalid test --- tests/unit/onyxTest.js | 36 ------------------------------------ 1 file changed, 36 deletions(-) diff --git a/tests/unit/onyxTest.js b/tests/unit/onyxTest.js index 30428df3..a787ed54 100644 --- a/tests/unit/onyxTest.js +++ b/tests/unit/onyxTest.js @@ -101,42 +101,6 @@ describe('Onyx', () => { describe('clear', () => {}); describe('mergeCollection', () => { - it('should call subscriber callback and storage multiMerge with the same data', () => { - const storageSpy = jest.spyOn(StorageMock, 'multiMerge'); - const routineRoute = `${ONYX_KEYS.COLLECTION.ROUTES}routine`; - - let result; - connectionID = Onyx.connect({ - key: routineRoute, - initWithStoredValues: false, - callback: (value) => (result = value), - }); - - const initialValue = { - pendingFields: { - waypoints: 'add', - }, - }; - const updatedValue = { - pendingFields: { - waypoints: null, - }, - }; - - return Onyx.set(routineRoute, initialValue) - .then(() => { - expect(result).toEqual(initialValue); - Onyx.mergeCollection(ONYX_KEYS.COLLECTION.ROUTES, { - [routineRoute]: updatedValue, - }); - return waitForPromisesToResolve(); - }) - .then(() => { - expect(result).toEqual(updatedValue); - expect(storageSpy).toHaveBeenCalledWith([[routineRoute, updatedValue]]); - }); - }); - it('should properly set and merge when using mergeCollection', () => { const valuesReceived = {}; const mockCallback = jest.fn((data) => (valuesReceived[data.ID] = data.value)); From bbad0003c3432f8640e966b611e6ded0292d0707 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 28 Mar 2024 17:08:06 +0100 Subject: [PATCH 17/27] remove unused import --- tests/unit/onyxTest.js | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/onyxTest.js b/tests/unit/onyxTest.js index a787ed54..8d5c1732 100644 --- a/tests/unit/onyxTest.js +++ b/tests/unit/onyxTest.js @@ -2,7 +2,6 @@ import _ from 'underscore'; import Onyx from '../../lib'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; import OnyxUtils from '../../lib/OnyxUtils'; -import StorageMock from '../../lib/storage/__mocks__'; const ONYX_KEYS = { TEST_KEY: 'test', From 4404737ba5cc6b7fb1a7a553f9e379639b5acf97 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 28 Mar 2024 17:35:53 +0100 Subject: [PATCH 18/27] fix: circular fastMerge --- lib/withOnyx.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/withOnyx.js b/lib/withOnyx.js index 32ced492..2ea6dbb5 100644 --- a/lib/withOnyx.js +++ b/lib/withOnyx.js @@ -312,7 +312,6 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) { render() { // Remove any null values so that React replaces them with default props const propsToPass = _.omit(this.props, _.isNull); - const propsToPassWithoutNestedNulls = utils.removeNestedNullValues(propsToPass); if (this.state.loading) { return null; @@ -329,7 +328,7 @@ export default function (mapOnyxToState, shouldDelayUpdates = false) { Date: Thu, 28 Mar 2024 17:59:18 +0100 Subject: [PATCH 19/27] reset onyxTest --- tests/unit/onyxTest.js | 484 ++++++++++++++++++----------------------- 1 file changed, 214 insertions(+), 270 deletions(-) diff --git a/tests/unit/onyxTest.js b/tests/unit/onyxTest.js index 8d5c1732..a75fd6bb 100644 --- a/tests/unit/onyxTest.js +++ b/tests/unit/onyxTest.js @@ -37,287 +37,60 @@ describe('Onyx', () => { return Onyx.clear(); }); - describe('set', () => { - it('should remove key value from OnyxCache/Storage when set is called with null value', () => - Onyx.set(ONYX_KEYS.OTHER_TEST, 42) - .then(() => OnyxUtils.getAllKeys()) - .then((keys) => { - expect(keys.has(ONYX_KEYS.OTHER_TEST)).toBe(true); - return Onyx.set(ONYX_KEYS.OTHER_TEST, null); - }) - .then(() => { - // Checks if cache value is removed. - expect(cache.getAllKeys().size).toBe(0); - - // When cache keys length is 0, we fetch the keys from storage. - return OnyxUtils.getAllKeys(); - }) - .then((keys) => { - expect(keys.has(ONYX_KEYS.OTHER_TEST)).toBe(false); - })); - - it('should set a simple key', () => { - let testKeyValue; - - connectionID = Onyx.connect({ - key: ONYX_KEYS.TEST_KEY, - initWithStoredValues: false, - callback: (value) => { - testKeyValue = value; - }, - }); - - // Set a simple key - return Onyx.set(ONYX_KEYS.TEST_KEY, 'test').then(() => { - expect(testKeyValue).toBe('test'); - }); - }); - }); - - describe('merge', () => { - it('should merge an object with another object', () => { - let testKeyValue; - - connectionID = Onyx.connect({ - key: ONYX_KEYS.TEST_KEY, - initWithStoredValues: false, - callback: (value) => { - testKeyValue = value; - }, - }); - - return Onyx.set(ONYX_KEYS.TEST_KEY, {test1: 'test1'}) - .then(() => { - expect(testKeyValue).toEqual({test1: 'test1'}); - return Onyx.merge(ONYX_KEYS.TEST_KEY, {test2: 'test2'}); - }) - .then(() => { - expect(testKeyValue).toEqual({test1: 'test1', test2: 'test2'}); - }); - }); - }); - - describe('clear', () => {}); - - describe('mergeCollection', () => { - it('should properly set and merge when using mergeCollection', () => { - const valuesReceived = {}; - const mockCallback = jest.fn((data) => (valuesReceived[data.ID] = data.value)); - connectionID = Onyx.connect({ - key: ONYX_KEYS.COLLECTION.TEST_KEY, - initWithStoredValues: false, - callback: mockCallback, - }); + it('should remove key value from OnyxCache/Storage when set is called with null value', () => + Onyx.set(ONYX_KEYS.OTHER_TEST, 42) + .then(() => OnyxUtils.getAllKeys()) + .then((keys) => { + expect(keys.has(ONYX_KEYS.OTHER_TEST)).toBe(true); + return Onyx.set(ONYX_KEYS.OTHER_TEST, null); + }) + .then(() => { + // Checks if cache value is removed. + expect(cache.getAllKeys().size).toBe(0); - // The first time we call mergeCollection we'll be doing a multiSet internally - return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { - test_1: { - ID: 123, - value: 'one', - }, - test_2: { - ID: 234, - value: 'two', - }, - test_3: { - ID: 345, - value: 'three', - }, + // When cache keys length is 0, we fetch the keys from storage. + return OnyxUtils.getAllKeys(); }) - .then(() => - // 2 key values to update and 2 new keys to add. - // MergeCollection will perform a mix of multiSet and multiMerge - Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { - test_1: { - ID: 123, - value: 'five', - }, - test_2: { - ID: 234, - value: 'four', - }, - test_4: { - ID: 456, - value: 'two', - }, - test_5: { - ID: 567, - value: 'one', - }, - }), - ) - .then(() => { - // 3 items on the first mergeCollection + 4 items the next mergeCollection - expect(mockCallback).toHaveBeenCalledTimes(7); - expect(mockCallback).toHaveBeenNthCalledWith(1, {ID: 123, value: 'one'}, 'test_1'); - expect(mockCallback).toHaveBeenNthCalledWith(2, {ID: 234, value: 'two'}, 'test_2'); - expect(mockCallback).toHaveBeenNthCalledWith(3, {ID: 345, value: 'three'}, 'test_3'); - expect(mockCallback).toHaveBeenNthCalledWith(4, {ID: 123, value: 'five'}, 'test_1'); - expect(mockCallback).toHaveBeenNthCalledWith(5, {ID: 234, value: 'four'}, 'test_2'); - expect(mockCallback).toHaveBeenNthCalledWith(6, {ID: 456, value: 'two'}, 'test_4'); - expect(mockCallback).toHaveBeenNthCalledWith(7, {ID: 567, value: 'one'}, 'test_5'); - expect(valuesReceived[123]).toEqual('five'); - expect(valuesReceived[234]).toEqual('four'); - expect(valuesReceived[345]).toEqual('three'); - expect(valuesReceived[456]).toEqual('two'); - expect(valuesReceived[567]).toEqual('one'); - }); - }); + .then((keys) => { + expect(keys.has(ONYX_KEYS.OTHER_TEST)).toBe(false); + })); - it('should skip the update when a key not belonging to collection key is present in mergeCollection', () => { - const valuesReceived = {}; - connectionID = Onyx.connect({ - key: ONYX_KEYS.COLLECTION.TEST_KEY, - initWithStoredValues: false, - callback: (data, key) => (valuesReceived[key] = data), - }); + it('should set a simple key', () => { + let testKeyValue; - return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {test_1: {ID: 123}, notMyTest: {beep: 'boop'}}).then(() => { - expect(valuesReceived).toEqual({}); - }); + connectionID = Onyx.connect({ + key: ONYX_KEYS.TEST_KEY, + initWithStoredValues: false, + callback: (value) => { + testKeyValue = value; + }, }); - it('should return full object to callback when calling mergeCollection()', () => { - const valuesReceived = {}; - connectionID = Onyx.connect({ - key: ONYX_KEYS.COLLECTION.TEST_KEY, - initWithStoredValues: false, - callback: (data, key) => (valuesReceived[key] = data), - }); - - return Onyx.multiSet({ - test_1: { - existingData: 'test', - }, - test_2: { - existingData: 'test', - }, - }) - .then(() => - Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { - test_1: { - ID: 123, - value: 'one', - }, - test_2: { - ID: 234, - value: 'two', - }, - }), - ) - .then(() => { - expect(valuesReceived).toEqual({ - test_1: { - ID: 123, - value: 'one', - existingData: 'test', - }, - test_2: { - ID: 234, - value: 'two', - existingData: 'test', - }, - }); - }); + // Set a simple key + return Onyx.set(ONYX_KEYS.TEST_KEY, 'test').then(() => { + expect(testKeyValue).toBe('test'); }); + }); - it('should use update data object to set/merge keys', () => { - let testKeyValue; - connectionID = Onyx.connect({ - key: ONYX_KEYS.TEST_KEY, - initWithStoredValues: false, - callback: (value) => { - testKeyValue = value; - }, - }); - - let otherTestKeyValue; - connectionID = Onyx.connect({ - key: ONYX_KEYS.OTHER_TEST, - initWithStoredValues: false, - callback: (value) => { - otherTestKeyValue = value; - }, - }); - - return waitForPromisesToResolve() - .then(() => { - // Given the initial Onyx state: {test: true, otherTest: {test1: 'test1'}} - Onyx.set(ONYX_KEYS.TEST_KEY, true); - Onyx.set(ONYX_KEYS.OTHER_TEST, {test1: 'test1'}); - return waitForPromisesToResolve(); - }) - .then(() => { - expect(testKeyValue).toBe(true); - expect(otherTestKeyValue).toEqual({test1: 'test1'}); + it('should merge an object with another object', () => { + let testKeyValue; - // When we pass a data object to Onyx.update - return Onyx.update([ - { - onyxMethod: 'set', - key: ONYX_KEYS.TEST_KEY, - value: 'one', - }, - { - onyxMethod: 'merge', - key: ONYX_KEYS.OTHER_TEST, - value: {test2: 'test2'}, - }, - ]); - }) - .then(() => { - // Then the final Onyx state should be {test: 'one', otherTest: {test1: 'test1', test2: 'test2'}} - expect(testKeyValue).toBe('one'); - expect(otherTestKeyValue).toEqual({test1: 'test1', test2: 'test2'}); - }); + connectionID = Onyx.connect({ + key: ONYX_KEYS.TEST_KEY, + initWithStoredValues: false, + callback: (value) => { + testKeyValue = value; + }, }); - it('mergeCollection should omit nested null values', () => { - let result; - - const routineRoute = `${ONYX_KEYS.COLLECTION.TEST_KEY}routine`; - const holidayRoute = `${ONYX_KEYS.COLLECTION.TEST_KEY}holiday`; - - connectionID = Onyx.connect({ - key: ONYX_KEYS.COLLECTION.TEST_KEY, - initWithStoredValues: false, - callback: (value) => (result = value), - waitForCollectionCallback: true, - }); - - return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { - [routineRoute]: { - waypoints: { - 1: 'Home', - 2: 'Work', - 3: 'Gym', - }, - }, - [holidayRoute]: { - waypoints: { - 1: 'Home', - 2: 'Beach', - 3: null, - }, - }, - }).then(() => { - expect(result).toEqual({ - [routineRoute]: { - waypoints: { - 1: 'Home', - 2: 'Work', - 3: 'Gym', - }, - }, - [holidayRoute]: { - waypoints: { - 1: 'Home', - 2: 'Beach', - }, - }, - }); + return Onyx.set(ONYX_KEYS.TEST_KEY, {test1: 'test1'}) + .then(() => { + expect(testKeyValue).toEqual({test1: 'test1'}); + return Onyx.merge(ONYX_KEYS.TEST_KEY, {test2: 'test2'}); + }) + .then(() => { + expect(testKeyValue).toEqual({test1: 'test1', test2: 'test2'}); }); - }); }); it('should notify subscribers when data has been cleared', () => { @@ -580,6 +353,178 @@ describe('Onyx', () => { }); }); + it('should properly set and merge when using mergeCollection', () => { + const valuesReceived = {}; + const mockCallback = jest.fn((data) => (valuesReceived[data.ID] = data.value)); + connectionID = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_KEY, + initWithStoredValues: false, + callback: mockCallback, + }); + + // The first time we call mergeCollection we'll be doing a multiSet internally + return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { + test_1: { + ID: 123, + value: 'one', + }, + test_2: { + ID: 234, + value: 'two', + }, + test_3: { + ID: 345, + value: 'three', + }, + }) + .then(() => + // 2 key values to update and 2 new keys to add. + // MergeCollection will perform a mix of multiSet and multiMerge + Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { + test_1: { + ID: 123, + value: 'five', + }, + test_2: { + ID: 234, + value: 'four', + }, + test_4: { + ID: 456, + value: 'two', + }, + test_5: { + ID: 567, + value: 'one', + }, + }), + ) + .then(() => { + // 3 items on the first mergeCollection + 4 items the next mergeCollection + expect(mockCallback).toHaveBeenCalledTimes(7); + expect(mockCallback).toHaveBeenNthCalledWith(1, {ID: 123, value: 'one'}, 'test_1'); + expect(mockCallback).toHaveBeenNthCalledWith(2, {ID: 234, value: 'two'}, 'test_2'); + expect(mockCallback).toHaveBeenNthCalledWith(3, {ID: 345, value: 'three'}, 'test_3'); + expect(mockCallback).toHaveBeenNthCalledWith(4, {ID: 123, value: 'five'}, 'test_1'); + expect(mockCallback).toHaveBeenNthCalledWith(5, {ID: 234, value: 'four'}, 'test_2'); + expect(mockCallback).toHaveBeenNthCalledWith(6, {ID: 456, value: 'two'}, 'test_4'); + expect(mockCallback).toHaveBeenNthCalledWith(7, {ID: 567, value: 'one'}, 'test_5'); + expect(valuesReceived[123]).toEqual('five'); + expect(valuesReceived[234]).toEqual('four'); + expect(valuesReceived[345]).toEqual('three'); + expect(valuesReceived[456]).toEqual('two'); + expect(valuesReceived[567]).toEqual('one'); + }); + }); + + it('should skip the update when a key not belonging to collection key is present in mergeCollection', () => { + const valuesReceived = {}; + connectionID = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_KEY, + initWithStoredValues: false, + callback: (data, key) => (valuesReceived[key] = data), + }); + + return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {test_1: {ID: 123}, notMyTest: {beep: 'boop'}}).then(() => { + expect(valuesReceived).toEqual({}); + }); + }); + + it('should return full object to callback when calling mergeCollection()', () => { + const valuesReceived = {}; + connectionID = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_KEY, + initWithStoredValues: false, + callback: (data, key) => (valuesReceived[key] = data), + }); + + return Onyx.multiSet({ + test_1: { + existingData: 'test', + }, + test_2: { + existingData: 'test', + }, + }) + .then(() => + Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { + test_1: { + ID: 123, + value: 'one', + }, + test_2: { + ID: 234, + value: 'two', + }, + }), + ) + .then(() => { + expect(valuesReceived).toEqual({ + test_1: { + ID: 123, + value: 'one', + existingData: 'test', + }, + test_2: { + ID: 234, + value: 'two', + existingData: 'test', + }, + }); + }); + }); + + it('should use update data object to set/merge keys', () => { + let testKeyValue; + connectionID = Onyx.connect({ + key: ONYX_KEYS.TEST_KEY, + initWithStoredValues: false, + callback: (value) => { + testKeyValue = value; + }, + }); + + let otherTestKeyValue; + connectionID = Onyx.connect({ + key: ONYX_KEYS.OTHER_TEST, + initWithStoredValues: false, + callback: (value) => { + otherTestKeyValue = value; + }, + }); + + return waitForPromisesToResolve() + .then(() => { + // Given the initial Onyx state: {test: true, otherTest: {test1: 'test1'}} + Onyx.set(ONYX_KEYS.TEST_KEY, true); + Onyx.set(ONYX_KEYS.OTHER_TEST, {test1: 'test1'}); + return waitForPromisesToResolve(); + }) + .then(() => { + expect(testKeyValue).toBe(true); + expect(otherTestKeyValue).toEqual({test1: 'test1'}); + + // When we pass a data object to Onyx.update + return Onyx.update([ + { + onyxMethod: 'set', + key: ONYX_KEYS.TEST_KEY, + value: 'one', + }, + { + onyxMethod: 'merge', + key: ONYX_KEYS.OTHER_TEST, + value: {test2: 'test2'}, + }, + ]); + }) + .then(() => { + // Then the final Onyx state should be {test: 'one', otherTest: {test1: 'test1', test2: 'test2'}} + expect(testKeyValue).toBe('one'); + expect(otherTestKeyValue).toEqual({test1: 'test1', test2: 'test2'}); + }); + }); + it('should use update data object to merge a collection of keys', () => { const valuesReceived = {}; const mockCallback = jest.fn((data) => (valuesReceived[data.ID] = data.value)); @@ -1094,7 +1039,6 @@ describe('Onyx', () => { }); }); }); - it('should apply updates in order with Onyx.update', () => { let testKeyValue; From 32beb34919ac758622fa63792b3a8256618572f0 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 28 Mar 2024 17:59:22 +0100 Subject: [PATCH 20/27] add new tests --- tests/unit/onyxTest.js | 74 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/tests/unit/onyxTest.js b/tests/unit/onyxTest.js index a75fd6bb..c22dc083 100644 --- a/tests/unit/onyxTest.js +++ b/tests/unit/onyxTest.js @@ -1071,4 +1071,78 @@ describe('Onyx', () => { expect(testKeyValue).toEqual(null); }); }); + + it('should merge a non-existing key with a nested null removed', () => { + let testKeyValue; + + connectionID = Onyx.connect({ + key: ONYX_KEYS.TEST_KEY, + initWithStoredValues: false, + callback: (value) => { + testKeyValue = value; + }, + }); + + return Onyx.merge(ONYX_KEYS.TEST_KEY, { + waypoints: { + 1: 'Home', + 2: 'Work', + 3: null, + }, + }).then(() => { + expect(testKeyValue).toEqual({ + waypoints: { + 1: 'Home', + 2: 'Work', + }, + }); + }); + }); + + it('mergeCollection should omit nested null values', () => { + let result; + + const routineRoute = `${ONYX_KEYS.COLLECTION.ROUTES}routine`; + const holidayRoute = `${ONYX_KEYS.COLLECTION.ROUTES}holiday`; + + connectionID = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.ROUTES, + initWithStoredValues: false, + callback: (value) => (result = value), + waitForCollectionCallback: true, + }); + + return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.ROUTES, { + [routineRoute]: { + waypoints: { + 1: 'Home', + 2: 'Work', + 3: 'Gym', + }, + }, + [holidayRoute]: { + waypoints: { + 1: 'Home', + 2: 'Beach', + 3: null, + }, + }, + }).then(() => { + expect(result).toEqual({ + [routineRoute]: { + waypoints: { + 1: 'Home', + 2: 'Work', + 3: 'Gym', + }, + }, + [holidayRoute]: { + waypoints: { + 1: 'Home', + 2: 'Beach', + }, + }, + }); + }); + }); }); From 8c41e941620488fdbcfc63ba6c3dded47aba7cec Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 28 Mar 2024 18:01:55 +0100 Subject: [PATCH 21/27] fix: test --- tests/unit/onyxTest.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/onyxTest.js b/tests/unit/onyxTest.js index c22dc083..faeda068 100644 --- a/tests/unit/onyxTest.js +++ b/tests/unit/onyxTest.js @@ -1102,17 +1102,17 @@ describe('Onyx', () => { it('mergeCollection should omit nested null values', () => { let result; - const routineRoute = `${ONYX_KEYS.COLLECTION.ROUTES}routine`; - const holidayRoute = `${ONYX_KEYS.COLLECTION.ROUTES}holiday`; + const routineRoute = `${ONYX_KEYS.COLLECTION.TEST_KEY}routine`; + const holidayRoute = `${ONYX_KEYS.COLLECTION.TEST_KEY}holiday`; connectionID = Onyx.connect({ - key: ONYX_KEYS.COLLECTION.ROUTES, + key: ONYX_KEYS.COLLECTION.TEST_KEY, initWithStoredValues: false, callback: (value) => (result = value), waitForCollectionCallback: true, }); - return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.ROUTES, { + return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { [routineRoute]: { waypoints: { 1: 'Home', From a2b3ae80328d8dc2ba37decdaa4cb6afbbfc6e1b Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 28 Mar 2024 18:05:29 +0100 Subject: [PATCH 22/27] improve comment --- lib/OnyxUtils.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/OnyxUtils.js b/lib/OnyxUtils.js index 6ebb963a..75cf7674 100644 --- a/lib/OnyxUtils.js +++ b/lib/OnyxUtils.js @@ -1068,7 +1068,8 @@ function hasPendingMergeForKey(key) { /** * Removes a key from storage if the value is null. - * Otherwise removes all nested null values in objects and returns the object + * Otherwise removes all nested null values in objects, + * if shouldRemoveNestedNulls is true and returns the object. * @param {String} key * @param {Mixed} value * @param {Boolean} shouldRemoveNestedNulls From 7764fb353a47fb96c46b4e416a68b33ac6ab426f Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 28 Mar 2024 18:16:45 +0100 Subject: [PATCH 23/27] remove unused line --- lib/OnyxUtils.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/OnyxUtils.js b/lib/OnyxUtils.js index 75cf7674..690bb268 100644 --- a/lib/OnyxUtils.js +++ b/lib/OnyxUtils.js @@ -519,7 +519,6 @@ function keysChanged(collectionKey, partialCollection, notifyRegularSubscibers = // was merged in via mergeCollection(). const cachedCollection = getCachedCollection(collectionKey); const cachedCollectionWithoutNestedNulls = utils.removeNestedNullValues(cachedCollection); - // const cachedCollectionWithoutNestedNulls = cachedCollection; // Regular Onyx.connect() subscriber found. if (_.isFunction(subscriber.callback)) { From 5f2e012f193083e1dc8530142b07d050836ad852 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Mon, 1 Apr 2024 16:50:01 +0200 Subject: [PATCH 24/27] fix: early return --- lib/OnyxUtils.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/OnyxUtils.js b/lib/OnyxUtils.js index 690bb268..30986363 100644 --- a/lib/OnyxUtils.js +++ b/lib/OnyxUtils.js @@ -1101,11 +1101,10 @@ function prepareKeyValuePairsForStorage(data, shouldRemoveNestedNulls) { (pairs, value, key) => { const {value: valueAfterRemoving, wasRemoved} = removeNullValues(key, value, shouldRemoveNestedNulls); - if (!wasRemoved) { - pairs.push([key, valueAfterRemoving]); + if (wasRemoved) { + return pairs; } - - return pairs; + return pairs.push([key, valueAfterRemoving]); }, [], ); From 2c7e69f8dad61299806a4294e5c8281d071bda1c Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Mon, 1 Apr 2024 17:00:30 +0200 Subject: [PATCH 25/27] Revert "fix: early return" This reverts commit 5f2e012f193083e1dc8530142b07d050836ad852. --- lib/OnyxUtils.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/OnyxUtils.js b/lib/OnyxUtils.js index 30986363..690bb268 100644 --- a/lib/OnyxUtils.js +++ b/lib/OnyxUtils.js @@ -1101,10 +1101,11 @@ function prepareKeyValuePairsForStorage(data, shouldRemoveNestedNulls) { (pairs, value, key) => { const {value: valueAfterRemoving, wasRemoved} = removeNullValues(key, value, shouldRemoveNestedNulls); - if (wasRemoved) { - return pairs; + if (!wasRemoved) { + pairs.push([key, valueAfterRemoving]); } - return pairs.push([key, valueAfterRemoving]); + + return pairs; }, [], ); From 86202a8dba01c03373474231b2744935e827d4b8 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Wed, 17 Apr 2024 18:27:24 +0200 Subject: [PATCH 26/27] fix: removeNestedNullValues --- lib/utils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/utils.ts b/lib/utils.ts index 21015c39..e29e472f 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -104,12 +104,12 @@ function fastMerge>(target: TObject | nu } /** Deep removes the nested null values from the given value. */ -function removeNestedNullValues(value: unknown[] | Record): Record | unknown[] | null { +function removeNestedNullValues>(value: unknown | unknown[] | TObject): Record | unknown[] | null { if (typeof value === 'object' && !Array.isArray(value)) { - return fastMerge(value, value); + return fastMerge(value as Record | null, value as Record | null); } - return value; + return value as Record | null; } /** Formats the action name by uppercasing and adding the key if provided. */ From 5d35172e40fe035b321aba6d9a5a354cd0817f2e Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 23 Apr 2024 15:28:56 +0200 Subject: [PATCH 27/27] rename variable --- lib/OnyxUtils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index eab16ee7..fe1b9b3f 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -756,8 +756,8 @@ function sendDataToConnection(mapping: Mapping, val: return; } - const valWithoutNestedNulls = utils.removeNestedNullValues(val); - (mapping as DefaultConnectOptions).callback?.(valWithoutNestedNulls, matchedKey as TKey); + const valuesWithoutNestedNulls = utils.removeNestedNullValues(val); + (mapping as DefaultConnectOptions).callback?.(valuesWithoutNestedNulls, matchedKey as TKey); } /**