Merge pull request #426 from margelo/fix/setting-key-to-null-prevents…

…-subsequent-merges Fix: Setting key to null prevents subsequent merges
Expensify · Nov 13, 2023 · f155571 · f155571
2 parents 2fe9b57 + 6ca9b30
commit f155571
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 5 deletions.
diff --git a/lib/Onyx.js b/lib/Onyx.js
@@ -1197,11 +1197,6 @@ function merge(key, changes) {
                 // We don't want to remove null values from the "batchedChanges", because SQLite uses them to remove keys from storage natively.
                 let batchedChanges = applyMerge(undefined, mergeQueue[key], false);
 
-                if (_.isNull(batchedChanges)) {
-                    remove(key);
-                    return;
-                }
-
                 // The presence of a `null` in the merge queue instructs us to drop the existing value.
                 // In this case, we can't simply merge the batched changes with the existing value, because then the null in the merge queue would have no effect
                 const shouldOverwriteExistingValue = _.includes(mergeQueue[key], null);
@@ -1210,6 +1205,12 @@ function merge(key, changes) {
                 delete mergeQueue[key];
                 delete mergeQueuePromise[key];
 
+                // If the batched changes equal null, we want to remove the key from storage, to reduce storage size
+                if (_.isNull(batchedChanges)) {
+                    remove(key);
+                    return;
+                }
+
                 // After that we merge the batched changes with the existing value
                 // We can remove null values from the "modifiedData", because "null" implicates that the user wants to remove a value from storage.
                 // The "modifiedData" will be directly "set" in storage instead of being merged

diff --git a/tests/unit/onyxTest.js b/tests/unit/onyxTest.js
@@ -979,4 +979,29 @@ describe('Onyx', () => {
                 });
             });
     });
+
+    it('should merge a key with null and allow subsequent updates', () => {
+        let testKeyValue;
+
+        connectionID = Onyx.connect({
+            key: ONYX_KEYS.TEST_KEY,
+            initWithStoredValues: false,
+            callback: (value) => {
+                testKeyValue = value;
+            },
+        });
+
+        return Onyx.set(ONYX_KEYS.TEST_KEY, 1)
+            .then(() => {
+                expect(testKeyValue).toEqual(1);
+                Onyx.merge(ONYX_KEYS.TEST_KEY, null);
+                return waitForPromisesToResolve();
+            })
+            .then(() => {
+                expect(testKeyValue).toEqual(null);
+                return Onyx.merge(ONYX_KEYS.TEST_KEY, 2);
+            }).then(() => {
+                expect(testKeyValue).toEqual(2);
+            });
+    });
 });