[AUDIT] Remove batching

jestSetup.js

@@ -10,6 +10,3 @@ jest.mock('react-native-nitro-sqlite', () => ({
 }));
 
 jest.useRealTimers();
-
-const unstable_batchedUpdates_jest = require('react-test-renderer').unstable_batchedUpdates;
-require('./lib/batch.native').default = unstable_batchedUpdates_jest;

lib/OnyxUtils.ts

@@ -8,7 +8,6 @@ import * as Logger from './Logger';
 import type Onyx from './Onyx';
 import cache, {TASK} from './OnyxCache';
 import * as Str from './Str';
-import unstable_batchedUpdates from './batch';
 import Storage from './storage';
 import type {
     CollectionKey,
@@ -67,9 +66,6 @@ let onyxKeyToSubscriptionIDs = new Map();
 // Optional user-provided key value states set when Onyx initializes or clears
 let defaultKeyStates: Record<OnyxKey, OnyxValue<OnyxKey>> = {};
 
-let batchUpdatesPromise: Promise<void> | null = null;
-let batchUpdatesQueue: Array<() => void> = [];
-
 // Used for comparison with a new update to avoid invoking the Onyx.connect callback with the same data.
 let lastConnectionCallbackData = new Map<number, OnyxValue<OnyxKey>>();
 
@@ -191,43 +187,6 @@ function sendActionToDevTools(
     DevTools.registerAction(utils.formatActionName(method, key), value, key ? {[key]: mergedValue || value} : (value as OnyxCollection<KeyValueMapping[OnyxKey]>));
 }
 
-/**
- * We are batching together onyx updates. This helps with use cases where we schedule onyx updates after each other.
- * This happens for example in the Onyx.update function, where we process API responses that might contain a lot of
- * update operations. Instead of calling the subscribers for each update operation, we batch them together which will
- * cause react to schedule the updates at once instead of after each other. This is mainly a performance optimization.
- */
-function maybeFlushBatchUpdates(): Promise<void> {
-    if (batchUpdatesPromise) {
-        return batchUpdatesPromise;
-    }
-
-    batchUpdatesPromise = new Promise((resolve) => {
-        /* We use (setTimeout, 0) here which should be called once native module calls are flushed (usually at the end of the frame)
-         * We may investigate if (setTimeout, 1) (which in React Native is equal to requestAnimationFrame) works even better
-         * then the batch will be flushed on next frame.
-         */
-        setTimeout(() => {
-            const updatesCopy = batchUpdatesQueue;
-            batchUpdatesQueue = [];
-            batchUpdatesPromise = null;
-            unstable_batchedUpdates(() => {
-                updatesCopy.forEach((applyUpdates) => {
-                    applyUpdates();
-                });
-            });
-
-            resolve();
-        }, 0);
-    });
-    return batchUpdatesPromise;
-}
-
-function batchUpdates(updates: () => void): Promise<void> {
-    batchUpdatesQueue.push(updates);
-    return maybeFlushBatchUpdates();
-}
-
 /**
  * Takes a collection of items (eg. {testKey_1:{a:'a'}, testKey_2:{b:'b'}})
  * and runs it through a reducer function to return a subset of the data according to a selector.
@@ -597,7 +556,6 @@ function keysChanged<TKey extends CollectionKeyBase>(
     collectionKey: TKey,
     partialCollection: OnyxCollection<KeyValueMapping[TKey]>,
     partialPreviousCollection: OnyxCollection<KeyValueMapping[TKey]> | undefined,
-    notifyConnectSubscribers = true,
 ): void {
     // We prepare the "cached collection" which is the entire collection + the new partial data that
     // was merged in via mergeCollection().
@@ -633,10 +591,6 @@ function keysChanged<TKey extends CollectionKeyBase>(
 
         // Regular Onyx.connect() subscriber found.
         if (typeof subscriber.callback === 'function') {
-            if (!notifyConnectSubscribers) {
-                continue;
-            }
-
             // If they are subscribed to the collection key and using waitForCollectionCallback then we'll
             // send the whole cached collection.
             if (isSubscribedToCollectionKey) {
@@ -682,12 +636,7 @@ function keysChanged<TKey extends CollectionKeyBase>(
  * @example
  * keyChanged(key, value, subscriber => subscriber.initWithStoredValues === false)
  */
-function keyChanged<TKey extends OnyxKey>(
-    key: TKey,
-    value: OnyxValue<TKey>,
-    canUpdateSubscriber: (subscriber?: CallbackToStateMapping<OnyxKey>) => boolean = () => true,
-    notifyConnectSubscribers = true,
-): void {
+function keyChanged<TKey extends OnyxKey>(key: TKey, value: OnyxValue<TKey>, canUpdateSubscriber: (subscriber?: CallbackToStateMapping<OnyxKey>) => boolean = () => true): void {
     // Add or remove this key from the recentlyAccessedKeys lists
     if (value !== null) {
         cache.addLastAccessedKey(key, isCollectionKey(key));
@@ -727,9 +676,6 @@ function keyChanged<TKey extends OnyxKey>(
 
         // Subscriber is a regular call to connect() and provided a callback
         if (typeof subscriber.callback === 'function') {
-            if (!notifyConnectSubscribers) {
-                continue;
-            }
             if (lastConnectionCallbackData.has(subscriber.subscriptionID) && lastConnectionCallbackData.get(subscriber.subscriptionID) === value) {
                 continue;
             }
@@ -807,6 +753,17 @@ function getCollectionDataAndSendAsObject<TKey extends OnyxKey>(matchingKeys: Co
     });
 }
 
+// !!!DO NOT MERGE THIS CODE, METHODS FOR READABILITY ONLY
+const nextMicrotask = () => Promise.resolve();
+let nextMacrotaskPromise: Promise<void> | null = null;
+const nextMacrotask = () =>
+    new Promise<void>((resolve) => {
+        setTimeout(() => {
+            nextMacrotaskPromise = null;
+            resolve();
+        }, 0);
+    });
+
 /**
  * Schedules an update that will be appended to the macro task queue (so it doesn't update the subscribers immediately).
  *
@@ -818,9 +775,10 @@ function scheduleSubscriberUpdate<TKey extends OnyxKey>(
     value: OnyxValue<TKey>,
     canUpdateSubscriber: (subscriber?: CallbackToStateMapping<OnyxKey>) => boolean = () => true,
 ): Promise<void> {
-    const promise = Promise.resolve().then(() => keyChanged(key, value, canUpdateSubscriber, true));
-    batchUpdates(() => keyChanged(key, value, canUpdateSubscriber, false));
-    return Promise.all([maybeFlushBatchUpdates(), promise]).then(() => undefined);
+    if (!nextMacrotaskPromise) {
+        nextMacrotaskPromise = nextMacrotask();
+    }
+    return Promise.all([nextMacrotaskPromise, nextMicrotask().then(() => keyChanged(key, value, canUpdateSubscriber))]).then(() => undefined);
 }
 
 /**
@@ -833,9 +791,10 @@ function scheduleNotifyCollectionSubscribers<TKey extends OnyxKey>(
     value: OnyxCollection<KeyValueMapping[TKey]>,
     previousValue?: OnyxCollection<KeyValueMapping[TKey]>,
 ): Promise<void> {
-    const promise = Promise.resolve().then(() => keysChanged(key, value, previousValue, true));
-    batchUpdates(() => keysChanged(key, value, previousValue, false));
-    return Promise.all([maybeFlushBatchUpdates(), promise]).then(() => undefined);
+    if (!nextMacrotaskPromise) {
+        nextMacrotaskPromise = nextMacrotask();
+    }
+    return Promise.all([nextMacrotaskPromise, nextMicrotask().then(() => keysChanged(key, value, previousValue))]).then(() => undefined);
 }
 
 /**
@@ -1420,7 +1379,6 @@ function clearOnyxUtilsInternals() {
     mergeQueuePromise = {};
     callbackToStateMapping = {};
     onyxKeyToSubscriptionIDs = new Map();
-    batchUpdatesQueue = [];
     lastConnectionCallbackData = new Map();
 }
 
@@ -1432,8 +1390,6 @@ const OnyxUtils = {
     getDeferredInitTask,
     initStoreValues,
     sendActionToDevTools,
-    maybeFlushBatchUpdates,
-    batchUpdates,
     get,
     getAllKeys,
     getCollectionKeys,
@@ -1487,10 +1443,6 @@ GlobalSettings.addGlobalSettingsChangeListener(({enablePerformanceMetrics}) => {
 
     // @ts-expect-error Reassign
     initStoreValues = decorateWithMetrics(initStoreValues, 'OnyxUtils.initStoreValues');
-    // @ts-expect-error Reassign
-    maybeFlushBatchUpdates = decorateWithMetrics(maybeFlushBatchUpdates, 'OnyxUtils.maybeFlushBatchUpdates');
-    // @ts-expect-error Reassign
-    batchUpdates = decorateWithMetrics(batchUpdates, 'OnyxUtils.batchUpdates');
     // @ts-expect-error Complex type signature
     get = decorateWithMetrics(get, 'OnyxUtils.get');
     // @ts-expect-error Reassign

package.json

@@ -64,7 +64,6 @@
     "@types/lodash": "^4.14.202",
     "@types/node": "^20.11.5",
     "@types/react": "^18.2.14",
-    "@types/react-dom": "^18.2.18",
     "@types/react-native": "^0.70.0",
     "@types/underscore": "^1.11.15",
     "@typescript-eslint/eslint-plugin": "^6.19.0",
@@ -86,7 +85,6 @@
     "prettier": "^2.8.8",
     "prop-types": "^15.7.2",
     "react": "18.2.0",
-    "react-dom": "18.2.0",
     "react-native": "0.76.3",
     "react-native-device-info": "^10.3.0",
     "react-native-nitro-modules": "^0.26.2",
@@ -101,7 +99,6 @@
   "peerDependencies": {
     "idb-keyval": "^6.2.1",
     "react": ">=18.1.0",
-    "react-dom": ">=18.1.0",
     "react-native": ">=0.75.0",
     "react-native-device-info": "^10.3.0",
     "react-native-nitro-modules": ">=0.26.2",

tests/perf-test/OnyxUtils.perf-test.ts

@@ -112,13 +112,6 @@ describe('OnyxUtils', () => {
         });
     });
 
-    describe('batchUpdates / maybeFlushBatchUpdates', () => {
-        test('one call with 1k updates', async () => {
-            const updates: Array<() => void> = Array.from({length: 1000}, () => jest.fn);
-            await measureAsyncFunction(() => Promise.all(updates.map((update) => OnyxUtils.batchUpdates(update))));
-        });
-    });
-
     describe('get', () => {
         test('10k calls with heavy objects', async () => {
             await measureAsyncFunction(() => Promise.all(mockedReportActionsKeys.map((key) => OnyxUtils.get(key))), {

tests/unit/onyxUtilsTest.ts

@@ -283,7 +283,6 @@ describe('OnyxUtils', () => {
                 ONYXKEYS.COLLECTION.TEST_KEY,
                 {[entryKey]: updatedEntryData}, // new collection
                 initialCollection, // previous collection
-                true, // notify connect subscribers
             );
 
             // Should be called again because data changed