Merge branch 'main' into feature/remotedev

# Conflicts:
#	lib/Onyx.js

lib/ActiveClientManager/index.native.js

@@ -15,9 +15,4 @@ function init() {}
 
 function subscribeToClientChange() {}
 
-export {
-    isClientTheLeader,
-    init,
-    isReady,
-    subscribeToClientChange,
-};
+export {isClientTheLeader, init, isReady, subscribeToClientChange};

lib/ActiveClientManager/index.web.js

@@ -63,14 +63,14 @@ function init() {
                 }
                 activeClientID = message.data.clientID;
 
-                subscribers.forEach(callback => callback());
+                subscribers.forEach((callback) => callback());
                 break;
             }
             case REMOVED_LEADER_MESSAGE:
                 activeClientID = clientID;
                 timestamp = Date.now();
                 Broadcast.sendMessage({type: NEW_LEADER_MESSAGE, clientID, timestamp});
-                subscribers.forEach(callback => callback());
+                subscribers.forEach((callback) => callback());
                 break;
             default:
                 break;
@@ -91,9 +91,4 @@ function init() {
     });
 }
 
-export {
-    isClientTheLeader,
-    init,
-    isReady,
-    subscribeToClientChange,
-};
+export {isClientTheLeader, init, isReady, subscribeToClientChange};

lib/Onyx.js

@@ -1034,17 +1034,18 @@ function evictStorageAndRetry(error, onyxMethod, ...args) {
  *
  * @param {String} key
  * @param {*} value
- * @param {Boolean} hasChanged
  * @param {String} method
+ * @param {Boolean} hasChanged
+ * @param {Boolean} wasRemoved
  * @returns {Promise}
  */
-function broadcastUpdate(key, value, hasChanged, method) {
+function broadcastUpdate(key, value, method, hasChanged, wasRemoved = false) {
     // Logging properties only since values could be sensitive things we don't want to log
     Logger.logInfo(`${method}() called for key: ${key}${_.isObject(value) ? ` properties: ${_.keys(value).join(',')}` : ''}`);
 
     // 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).
-    if (hasChanged) {
+    if (hasChanged && !wasRemoved) {
         cache.set(key, value);
     } else {
         cache.addToAccessedKeys(key);
@@ -1066,18 +1067,18 @@ function hasPendingMergeForKey(key) {
  * Otherwise removes all nested null values in objects and returns the object
  * @param {String} key
  * @param {Mixed} value
- * @returns {Mixed} `null` if the key got removed completely, otherwise the value without null values
+ * @returns {Mixed} The value without null values and a boolean "wasRemoved", which indicates if the key got removed completely
  */
 function removeNullValues(key, value) {
     if (_.isNull(value)) {
         remove(key);
-        return null;
+        return {value, wasRemoved: 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 utils.removeNestedNullValues(value);
+    return {value: utils.removeNestedNullValues(value), wasRemoved: false};
 }
 
 /**
@@ -1098,29 +1099,25 @@ function set(key, value) {
         return Promise.resolve();
     }
 
-    const valueWithoutNull = removeNullValues(key, value);
-
-    if (valueWithoutNull === null) {
-        sendActionToDevTools(METHOD.SET, key, undefined);
-        return Promise.resolve();
-    }
+    // If the value is null, we remove the key from storage
+    const {value: valueAfterRemoving, wasRemoved} = removeNullValues(key, value);
 
     if (hasPendingMergeForKey(key)) {
-        Logger.logAlert(`Onyx.set() called after Onyx.merge() for key: ${key}. It is recommended to use set() or merge() not both.`);
+        delete mergeQueue[key];
     }
 
-    const hasChanged = cache.hasValueChanged(key, valueWithoutNull);
+    const hasChanged = cache.hasValueChanged(key, valueAfterRemoving);
 
     // This approach prioritizes fast UI changes without waiting for data to be stored in device storage.
-    const updatePromise = broadcastUpdate(key, valueWithoutNull, hasChanged, 'set');
+    const updatePromise = broadcastUpdate(key, valueAfterRemoving, 'set', hasChanged, wasRemoved);
 
-    // If the value has not changed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead.
-    if (!hasChanged) {
+    // If the value has not changed or the key got removed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead.
+    if (!hasChanged || wasRemoved) {
         return updatePromise;
     }
 
-    return Storage.setItem(key, valueWithoutNull)
-        .catch((error) => evictStorageAndRetry(error, set, key, valueWithoutNull))
+    return Storage.setItem(key, valueAfterRemoving)
+        .catch((error) => evictStorageAndRetry(error, set, key, valueAfterRemoving))
         .then(() => {
             sendActionToDevTools(METHOD.SET, key, valueWithoutNull);
             return updatePromise;
@@ -1130,13 +1127,23 @@ function set(key, value) {
 /**
  * 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
+ * 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
  * @return {Array} an array of key - value pairs <[key, value]>
  */
 function prepareKeyValuePairsForStorage(data) {
-    return _.map(data, (value, key) => [key, value]);
+    const keyValuePairs = [];
+
+    _.forEach(data, (value, key) => {
+        const {value: valueAfterRemoving, wasRemoved} = removeNullValues(key, value);
+
+        if (wasRemoved) return;
+
+        keyValuePairs.push([key, valueAfterRemoving]);
+    });
+
+    return keyValuePairs;
 }
 
 /**
@@ -1159,25 +1166,13 @@ function multiSet(data) {
 
     const keyValuePairs = prepareKeyValuePairsForStorage(data);
 
-    const updatePromises = _.map(data, (val, key) => {
+    const updatePromises = _.map(keyValuePairs, ([key, value]) => {
         // Update cache and optimistically inform subscribers on the next tick
-        cache.set(key, val);
-        return scheduleSubscriberUpdate(key, val);
+        cache.set(key, value);
+        return scheduleSubscriberUpdate(key, value);
     });
 
-    const keyValuePairsWithoutNull = _.filter(
-        _.map(keyValuePairs, ([key, value]) => {
-            const valueWithoutNull = removeNullValues(key, value);
-
-            if (valueWithoutNull === null) {
-                return;
-            }
-            return [key, valueWithoutNull];
-        }),
-        Boolean,
-    );
-
-    return Storage.multiSet(keyValuePairsWithoutNull)
+    return Storage.multiSet(keyValuePairs)
         .catch((error) => evictStorageAndRetry(error, multiSet, data))
         .then(() => {
             sendActionToDevTools(METHOD.MULTI_SET, undefined, data);
@@ -1256,6 +1251,9 @@ function merge(key, changes) {
     mergeQueue[key] = [changes];
 
     mergeQueuePromise[key] = get(key).then((existingValue) => {
+        // Calls to Onyx.set after a merge will terminate the current merge process and clear the merge queue
+        if (mergeQueue[key] == null) return;
+
         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)
             // We don't want to remove null values from the "batchedChanges", because SQLite uses them to remove keys from storage natively.
@@ -1270,10 +1268,7 @@ function merge(key, changes) {
             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;
-            }
+            const {wasRemoved} = removeNullValues(key, batchedChanges);
 
             // 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.
@@ -1291,10 +1286,10 @@ function merge(key, changes) {
             const hasChanged = cache.hasValueChanged(key, modifiedData);
 
             // This approach prioritizes fast UI changes without waiting for data to be stored in device storage.
-            const updatePromise = broadcastUpdate(key, modifiedData, hasChanged, 'merge');
+            const updatePromise = broadcastUpdate(key, modifiedData, 'merge', hasChanged, wasRemoved);
 
             // If the value has not changed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead.
-            if (!hasChanged || isClearing) {
+            if (!hasChanged || isClearing || wasRemoved) {
                 return updatePromise;
             }
 

lib/broadcast/index.native.js

@@ -9,6 +9,4 @@ function subscribe() {}
 
 function disconnect() {}
 
-export {
-    sendMessage, subscribe, disconnect,
-};
+export {sendMessage, subscribe, disconnect};

lib/broadcast/index.web.js

@@ -19,7 +19,7 @@ function sendMessage(message) {
 function subscribe(callback) {
     subscriptions.push(callback);
     channel.onmessage = (message) => {
-        subscriptions.forEach(c => c(message));
+        subscriptions.forEach((c) => c(message));
     };
 }
 
@@ -30,6 +30,4 @@ function disconnect() {
     channel.close();
 }
 
-export {
-    sendMessage, subscribe, disconnect,
-};
+export {sendMessage, subscribe, disconnect};

package.json

@@ -1,6 +1,6 @@
 {
   "name": "react-native-onyx",
-  "version": "1.0.122",
+  "version": "1.0.125",
   "author": "Expensify, Inc.",
   "homepage": "https://expensify.com",
   "description": "State management for React Native",

playwright.config.js

@@ -59,4 +59,3 @@ module.exports = defineConfig({
         reuseExistingServer: !process.env.CI,
     },
 });
-

tests/e2e/app/jsconfig.paths.json

@@ -1,10 +1,8 @@
 {
-  "compilerOptions": {
-    "baseUrl": ".",
-    "paths": {
-      "react-native-onyx/*": [
-        "../../../lib/*"
-      ]
+    "compilerOptions": {
+        "baseUrl": ".",
+        "paths": {
+            "react-native-onyx/*": ["../../../lib/*"]
+        }
     }
-  }
 }

tests/e2e/app/metro.config.js

@@ -17,14 +17,7 @@ module.exports = {
     // We need to make sure that only one version is loaded for peerDependencies
     // So we block them at the root, and alias them to the versions in app's node_modules
     resolver: {
-        blockList: exclusionList(
-            _.map(
-                modules,
-                m => new RegExp(
-                    `^${escape(path.join(root, 'node_modules', m))}\\/.*$`,
-                ),
-            ),
-        ),
+        blockList: exclusionList(_.map(modules, (m) => new RegExp(`^${escape(path.join(root, 'node_modules', m))}\\/.*$`))),
 
         extraNodeModules: {
             ..._.reduce(

tests/e2e/app/src/App.js

@@ -19,9 +19,7 @@ function App(props) {
 
     return (
         <View>
-            {isAuthenticated && (
-                <Text aria-label="logged-in">You are logged in</Text>
-            )}
+            {isAuthenticated && <Text aria-label="logged-in">You are logged in</Text>}
             <Main />
         </View>
     );

tests/e2e/app/src/Main.js

@@ -1,7 +1,5 @@
 import React, {useEffect, useState} from 'react';
-import {
-    Button, StyleSheet, Text, View,
-} from 'react-native';
+import {Button, StyleSheet, Text, View} from 'react-native';
 import PropTypes from 'prop-types';
 import lodashGet from 'lodash/get';
 import _ from 'underscore';
@@ -76,14 +74,8 @@ function Main(props) {
         const date = Date.now();
 
         for (let i = 0; i <= (small ? 10 : 100); i++) {
-            Onyx.merge(
-                `${ONYXKEYS.COLLECTION.METEORITES}${date}${i}`,
-                Data.meteorites,
-            );
-            Onyx.merge(
-                `${ONYXKEYS.COLLECTION.ASTEROIDS}${date}${i}`,
-                Data.asteroids,
-            );
+            Onyx.merge(`${ONYXKEYS.COLLECTION.METEORITES}${date}${i}`, Data.meteorites);
+            Onyx.merge(`${ONYXKEYS.COLLECTION.ASTEROIDS}${date}${i}`, Data.asteroids);
         }
     };
 
@@ -115,26 +107,31 @@ function Main(props) {
                     </View>
                 </View>
             ) : (
-                <Button title="Log In" onPress={onLogIn} />
+                <Button
+                    title="Log In"
+                    onPress={onLogIn}
+                />
             )}
-            <Text aria-label="leader">
-                {isLeader ? 'leader' : 'non-leader'}
-            </Text>
-            <Text aria-label="data-number">
-                {lodashGet(props, 'randomNumber.number', undefined)}
-            </Text>
-            <Text aria-label="data-pokedex">
-                {lodashGet(props, 'pokedex.pokemon.length', undefined)}
-            </Text>
-            <Text aria-label="data-meteorites" numberOfLines={10}>
-                {_.filter(_.map(Object.entries(props.allMeteorites || {}),
-                    ([key, value]) => (value ? `${key}-${value && value.length}` : undefined)), v => !!v)
-                    .join(',')}
+            <Text aria-label="leader">{isLeader ? 'leader' : 'non-leader'}</Text>
+            <Text aria-label="data-number">{lodashGet(props, 'randomNumber.number', undefined)}</Text>
+            <Text aria-label="data-pokedex">{lodashGet(props, 'pokedex.pokemon.length', undefined)}</Text>
+            <Text
+                aria-label="data-meteorites"
+                numberOfLines={10}
+            >
+                {_.filter(
+                    _.map(Object.entries(props.allMeteorites || {}), ([key, value]) => (value ? `${key}-${value && value.length}` : undefined)),
+                    (v) => !!v,
+                ).join(',')}
             </Text>
-            <Text aria-label="data-asteroids" numberOfLines={10}>
-                {_.filter(_.map(Object.entries(props.allAsteroids || {}),
-                    ([key, value]) => (value ? `${key}-${value && value.length}` : undefined)), v => !!v)
-                    .join(',')}
+            <Text
+                aria-label="data-asteroids"
+                numberOfLines={10}
+            >
+                {_.filter(
+                    _.map(Object.entries(props.allAsteroids || {}), ([key, value]) => (value ? `${key}-${value && value.length}` : undefined)),
+                    (v) => !!v,
+                ).join(',')}
             </Text>
             <Text
                 aria-label="data-updates"
@@ -143,7 +140,10 @@ function Main(props) {
             >
                 {JSON.stringify(Updates.updates)}
             </Text>
-            <Button title="Clear updates" onPress={Updates.clear} />
+            <Button
+                title="Clear updates"
+                onPress={Updates.clear}
+            />
         </View>
     );
 }