mergeCollection handle null values

API-INTERNAL.md

@@ -120,7 +120,7 @@ Otherwise removes all nested null values in objects and returns the object</p>
 <dt><a href="#prepareKeyValuePairsForStorage">prepareKeyValuePairsForStorage()</a> ⇒</dt>
 <dd><p>Storage expects array like: [[&quot;@MyApp_user&quot;, value_1], [&quot;@MyApp_key&quot;, value_2]]
 This method transforms an object like {&#39;@MyApp_user&#39;: myUserValue, &#39;@MyApp_key&#39;: myKeyValue}
-to an array of key-value pairs in the above format and removes key-value pairs that are being set to null</p>
+to an array of key-value pairs in the above format</p>
 </dd>
 <dt><a href="#applyMerge">applyMerge(changes)</a></dt>
 <dd><p>Merges an array of changes with an existing value</p>
@@ -376,7 +376,7 @@ Otherwise removes all nested null values in objects and returns the object
 ## prepareKeyValuePairsForStorage() ⇒
 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
+to an array of key-value pairs in the above format
 
 **Kind**: global function  
 **Returns**: an array of key - value pairs <[key, value]>  

lib/Onyx.ts

@@ -245,7 +245,7 @@ function set<TKey extends OnyxKey>(key: TKey, value: OnyxEntry<KeyValueMapping[T
  * @param data object keyed by ONYXKEYS and the values to set
  */
 function multiSet(data: Partial<NullableKeyValueMapping>): Promise<void> {
-    const keyValuePairs = OnyxUtils.prepareKeyValuePairsForStorage(data);
+    const keyValuePairs = OnyxUtils.prepareKeyValuePairsForStorage(data, true);
 
     const updatePromises = keyValuePairs.map(([key, value]) => {
         const prevValue = cache.getValue(key, false);
@@ -419,8 +419,10 @@ function mergeCollection<TKey extends CollectionKeyBase, TMap>(collectionKey: TK
                 obj[key] = mergedCollection[key];
                 return obj;
             }, {});
-            const keyValuePairsForExistingCollection = OnyxUtils.prepareKeyValuePairsForStorage(existingKeyCollection);
-            const keyValuePairsForNewCollection = OnyxUtils.prepareKeyValuePairsForStorage(newCollection);
+
+            // We don't want to remove null values because the provider's merge method uses them to remove their respective keys
+            const keyValuePairsForExistingCollection = OnyxUtils.prepareKeyValuePairsForStorage(existingKeyCollection, false);
+            const keyValuePairsForNewCollection = OnyxUtils.prepareKeyValuePairsForStorage(newCollection, true);
 
             const promises = [];
 

lib/OnyxUtils.ts

@@ -982,11 +982,15 @@ function removeNullValues(key: OnyxKey, value: OnyxValue<OnyxKey>): RemoveNullVa
 /**
  * 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]>
+ * to an array of key-value pairs in the above format
+ *
+ * @return an array of key - value pairs <[key, value]>
  */
-function prepareKeyValuePairsForStorage(data: Record<OnyxKey, OnyxValue<OnyxKey>>): Array<[OnyxKey, OnyxValue<OnyxKey>]> {
+function prepareKeyValuePairsForStorage(data: Record<OnyxKey, OnyxValue<OnyxKey>>, shouldRemoveNullObjectValues: boolean): Array<[OnyxKey, OnyxValue<OnyxKey>]> {
+    if (!shouldRemoveNullObjectValues) {
+        return Object.entries(data);
+    }
+
     const keyValuePairs: Array<[OnyxKey, OnyxValue<OnyxKey>]> = [];
 
     Object.entries(data).forEach(([key, value]) => {

tests/unit/onyxMultiMergeWebStorageTest.js

@@ -143,6 +143,68 @@ describe('Onyx.mergeCollection() and WebStorage', () => {
             });
     });
 
+    it('merge with null values', () => {
+        // Given empty storage
+        expect(StorageMock.getMockStore().test_1).toBeFalsy();
+        expect(StorageMock.getMockStore().test_2).toBeFalsy();
+        expect(StorageMock.getMockStore().test_3).toBeFalsy();
+
+        // And an empty cache values for the collection keys
+        expect(OnyxCache.getValue('test_1')).toBeFalsy();
+        expect(OnyxCache.getValue('test_2')).toBeFalsy();
+        expect(OnyxCache.getValue('test_3')).toBeFalsy();
+
+        // When we merge additional data and wait for the change
+        const data = {a: 'a', b: 'b'};
+        Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {
+            test_1: data,
+            test_2: data,
+            test_3: data,
+        });
+
+        return waitForPromisesToResolve()
+            .then(() => {
+                // Then the cache and storage should match
+                expect(OnyxCache.getValue('test_1')).toEqual(data);
+                expect(OnyxCache.getValue('test_2')).toEqual(data);
+                expect(OnyxCache.getValue('test_3')).toEqual(data);
+                expect(StorageMock.getMockStore().test_1).toEqual(data);
+                expect(StorageMock.getMockStore().test_2).toEqual(data);
+                expect(StorageMock.getMockStore().test_3).toEqual(data);
+
+                // When we merge additional data containing null values and wait for the change
+                const additionalData = {b: null, c: 'c'};
+                Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {
+                    test_1: additionalData,
+                    test_2: additionalData,
+                    test_3: additionalData,
+                });
+
+                return waitForPromisesToResolve();
+            })
+            .then(() => {
+                const finalObjectCache = {
+                    a: 'a',
+                    b: null,
+                    c: 'c',
+                };
+                const finalObject = {
+                    a: 'a',
+                    c: 'c',
+                };
+
+                // Then our new data should merge with the existing data in the cache
+                expect(OnyxCache.getValue('test_1')).toEqual(finalObjectCache);
+                expect(OnyxCache.getValue('test_2')).toEqual(finalObjectCache);
+                expect(OnyxCache.getValue('test_3')).toEqual(finalObjectCache);
+
+                // And the storage should reflect the same state but with nulled key-values removed
+                expect(StorageMock.getMockStore().test_1).toEqual(finalObject);
+                expect(StorageMock.getMockStore().test_2).toEqual(finalObject);
+                expect(StorageMock.getMockStore().test_3).toEqual(finalObject);
+            });
+    });
+
     it('setItem() and multiMerge()', () => {
         // Onyx should be empty after clear() is called
         expect(StorageMock.getMockStore()).toEqual({});