Fix: nullish keys not removed/merged in IDB-Keyval (web)

lib/Onyx.js

@@ -8,6 +8,7 @@ import createDeferredTask from './createDeferredTask';
 import fastMerge from './fastMerge';
 import * as PerformanceUtils from './metrics/PerformanceUtils';
 import Storage from './storage';
+import removeNullObjectValues from './utils/removeNullObjectValues';
 import DevTools from './DevTools';
 
 // Method constants
@@ -935,24 +936,6 @@ function hasPendingMergeForKey(key) {
     return Boolean(mergeQueue[key]);
 }
 
-/**
- * We generally want to remove top-level nullish 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 nullish values for "set" operations.
- * On web, IndexedDB will keep the top-level keys along with a null value and this uses up storage and memory.
- * This method will ensure that keys for null values are removed before an object is written to disk and cache so that all platforms are storing the data in the same efficient way.
- * @private
- * @param {*} value
- * @returns {*}
- */
-function removeNullObjectValues(value) {
-    if (_.isArray(value) || !_.isObject(value)) {
-        return value;
-    }
-
-    return _.omit(value, objectValue => _.isNull(objectValue));
-}
-
 /**
  * Write a value to our store with the given key
  *

lib/fastMerge.js

@@ -56,8 +56,8 @@ function mergeObject(target, source) {
 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) {
+    const isArray = Array.isArray(source);
+    if (isArray || source == null) {
         return source;
     }
     return mergeObject(target, source);

lib/storage/providers/IDBKeyVal.js

@@ -12,6 +12,7 @@ import {
 } from 'idb-keyval';
 import _ from 'underscore';
 import fastMerge from '../../fastMerge';
+import removeNullObjectValues from '../../utils/removeNullObjectValues';
 
 // We don't want to initialize the store while the JS bundle loads as idb-keyval will try to use global.indexedDB
 // which might not be available in certain environments that load the bundle (e.g. electron main process).
@@ -54,9 +55,17 @@ const provider = {
 
         return getValues.then((values) => {
             const upsertMany = _.map(pairs, ([key, value], index) => {
+                // If the value is null, we want to delete the key from storage,
+                // to be consistent with the native implementation with SQLite.
+                // SQLite by default removes keys from storage that are set to nullish values
+                if (value == null) {
+                    return promisifyRequest(store.delete(key));
+                }
+
                 const prev = values[index];
                 const newValue = _.isObject(prev) ? fastMerge(prev, value) : value;
-                return promisifyRequest(store.put(newValue, key));
+
+                return promisifyRequest(store.put(removeNullObjectValues(newValue), key));
             });
             return Promise.all(upsertMany);
         });

lib/utils/removeNullObjectValues.js

@@ -0,0 +1,19 @@
+import _ from 'underscore';
+
+/**
+ * We generally want to remove top-level nullish 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 nullish values for "set" operations.
+ * On web, IndexedDB will keep the top-level keys along with a null value and this uses up storage and memory.
+ * This method will ensure that keys for null values are removed before an object is written to disk and cache so that all platforms are storing the data in the same efficient way.
+ * @private
+ * @param {*} value
+ * @returns {*}
+ */
+export default function removeNullObjectValues(value) {
+    if (_.isArray(value) || !_.isObject(value)) {
+        return value;
+    }
+
+    return _.omit(value, objectValue => _.isNull(objectValue));
+}