Fix minor issues after introducing incremental JSON_PATCH merge

lib/Onyx.js

@@ -849,6 +849,7 @@ function evictStorageAndRetry(error, onyxMethod, ...args) {
  * @param {String} key
  * @param {*} value
  * @param {String} method
+ * @returns {Boolean} whether the value for the given key has changed
  */
 function broadcastUpdate(key, value, method) {
     // Logging properties only since values could be sensitive things we don't want to log
@@ -857,8 +858,24 @@ function broadcastUpdate(key, value, method) {
     // Update subscribers if the cached value has changed, or when the subscriber specifically requires
     // all updates regardless of value changes (indicated by initWithStoredValues set to false).
     const hasChanged = cache.hasValueChanged(key, value);
-    cache.set(key, value);
+    if (hasChanged) {
+        cache.set(key, value);
+    } else {
+        cache.addToAccessedKeys(key);
+    }
+
     notifySubscribersOnNextTick(key, value, subscriber => hasChanged || subscriber.initWithStoredValues === false);
+
+    return hasChanged;
+}
+
+/**
+ * @private
+ * @param {String} key
+ * @returns {Boolean}
+ */
+function hasPendingMergeForKey(key) {
+    return Boolean(mergeQueue[key]);
 }
 
 /**
@@ -874,8 +891,14 @@ function set(key, value) {
         return remove(key);
     }
 
+    if (hasPendingMergeForKey(key)) {
+        Logger.logAlert(`Onyx.set() called after Onyx.merge() for key: ${key}. It is recommended to use set() or merge() not both.`);
+    }
+
     // This approach prioritizes fast UI changes without waiting for data to be stored in device storage.
-    broadcastUpdate(key, value, 'set');
+    const hasChanged = broadcastUpdate(key, value, 'set');
+
+    if (!hasChanged) { return Promise.resolve(); }
 
     return Storage.setItem(key, value)
         .catch(error => evictStorageAndRetry(error, set, key, value));
@@ -918,11 +941,12 @@ function multiSet(data) {
  * Merges an array of changes with an existing value
  *
  * @private
- * @param {Array<*>} changes Array of changes that should be applied to the existing value
  * @param {*} existingValue
+ * @param {Array<*>} changes Array of changes that should be applied to the existing value
+ * @param {bool} omitNull Whether null top-level null values should be omitted
  * @returns {*}
  */
-function applyMerge(changes, existingValue) {
+function applyMerge(existingValue, changes, omitNull = true) {
     const lastChange = _.last(changes);
 
     if (_.isArray(existingValue) || _.isArray(lastChange)) {
@@ -937,7 +961,7 @@ function applyMerge(changes, existingValue) {
             const newData = Object.assign({}, fastMerge(modifiedData, change));
 
             // Remove all first level keys that are explicitly set to null.
-            return _.omit(newData, value => _.isNull(value));
+            return omitNull ? _.omit(newData, value => _.isNull(value)) : newData;
         }, existingValue || {});
     }
 
@@ -979,17 +1003,19 @@ function merge(key, changes) {
         .then((existingValue) => {
             try {
                 // We first only merge the changes, so we can provide these to the native implementation (SQLite uses only delta changes in "JSON_PATCH" to merge)
-                const batchedChanges = applyMerge(mergeQueue[key]);
+                const batchedChanges = applyMerge(undefined, mergeQueue[key], false);
 
                 // Clean up the write queue so we
                 // don't apply these changes again
                 delete mergeQueue[key];
 
                 // After that we merge the batched changes with the existing value
-                const modifiedData = applyMerge([batchedChanges], existingValue);
+                const modifiedData = applyMerge(existingValue, [batchedChanges]);
 
                 // This approach prioritizes fast UI changes without waiting for data to be stored in device storage.
-                broadcastUpdate(key, modifiedData, 'merge');
+                const hasChanged = broadcastUpdate(key, modifiedData, 'merge');
+
+                if (!hasChanged) { return Promise.resolve(); }
 
                 return Storage.mergeItem(key, batchedChanges, modifiedData);
             } catch (error) {
@@ -1000,15 +1026,6 @@ function merge(key, changes) {
         });
 }
 
-/**
- * @private
- * @param {String} key
- * @returns {Boolean}
- */
-function hasPendingMergeForKey(key) {
-    return Boolean(mergeQueue[key]);
-}
-
 /**
  * Merge user provided default key value pairs.
  * @private

lib/storage/__mocks__/index.js

@@ -4,9 +4,8 @@ import fastMerge from '../../fastMerge';
 let storageMapInternal = {};
 
 const set = jest.fn((key, value) => {
-    // eslint-disable-next-line no-param-reassign
     storageMapInternal[key] = value;
-    return Promise.resolve(storageMapInternal[key]);
+    return Promise.resolve(value);
 });
 
 const localForageMock = {