From 2f404e649c96264b849e8ffeb5cd20bf496083ed Mon Sep 17 00:00:00 2001 From: VickyStash Date: Thu, 16 Oct 2025 16:03:47 +0200 Subject: [PATCH 1/3] Remove the current batching mechanism --- jestSetup.js | 3 - lib/OnyxUtils.ts | 81 +++++--------------------- lib/batch.native.ts | 3 - lib/batch.ts | 3 - package-lock.json | 27 --------- package.json | 3 - tests/perf-test/OnyxUtils.perf-test.ts | 7 --- tests/unit/onyxUtilsTest.ts | 1 - 8 files changed, 13 insertions(+), 115 deletions(-) delete mode 100644 lib/batch.native.ts delete mode 100644 lib/batch.ts diff --git a/jestSetup.js b/jestSetup.js index 82f8f4d5d..156828a67 100644 --- a/jestSetup.js +++ b/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; diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 39e8afd06..f87638e6e 100644 --- a/lib/OnyxUtils.ts +++ b/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> = {}; -let batchUpdatesPromise: Promise | 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>(); @@ -191,43 +187,6 @@ function sendActionToDevTools( DevTools.registerAction(utils.formatActionName(method, key), value, key ? {[key]: mergedValue || value} : (value as OnyxCollection)); } -/** - * 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 { - 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 { - 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( collectionKey: TKey, partialCollection: OnyxCollection, partialPreviousCollection: OnyxCollection | 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( // 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( * @example * keyChanged(key, value, subscriber => subscriber.initWithStoredValues === false) */ -function keyChanged( - key: TKey, - value: OnyxValue, - canUpdateSubscriber: (subscriber?: CallbackToStateMapping) => boolean = () => true, - notifyConnectSubscribers = true, -): void { +function keyChanged(key: TKey, value: OnyxValue, canUpdateSubscriber: (subscriber?: CallbackToStateMapping) => 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( // 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; } @@ -818,9 +764,11 @@ function scheduleSubscriberUpdate( value: OnyxValue, canUpdateSubscriber: (subscriber?: CallbackToStateMapping) => boolean = () => true, ): Promise { - const promise = Promise.resolve().then(() => keyChanged(key, value, canUpdateSubscriber, true)); - batchUpdates(() => keyChanged(key, value, canUpdateSubscriber, false)); - return Promise.all([maybeFlushBatchUpdates(), promise]).then(() => undefined); + const promise0 = new Promise((resolve) => { + setTimeout(resolve, 0); + }); + const promise = Promise.resolve().then(() => keyChanged(key, value, canUpdateSubscriber)); + return Promise.all([promise0, promise]).then(() => undefined); } /** @@ -833,9 +781,13 @@ function scheduleNotifyCollectionSubscribers( value: OnyxCollection, previousValue?: OnyxCollection, ): Promise { - const promise = Promise.resolve().then(() => keysChanged(key, value, previousValue, true)); - batchUpdates(() => keysChanged(key, value, previousValue, false)); - return Promise.all([maybeFlushBatchUpdates(), promise]).then(() => undefined); + const promise0 = new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, 0); + }); + const promise = Promise.resolve().then(() => keysChanged(key, value, previousValue)); + return Promise.all([promise0, promise]).then(() => undefined); } /** @@ -1420,7 +1372,6 @@ function clearOnyxUtilsInternals() { mergeQueuePromise = {}; callbackToStateMapping = {}; onyxKeyToSubscriptionIDs = new Map(); - batchUpdatesQueue = []; lastConnectionCallbackData = new Map(); } @@ -1432,8 +1383,6 @@ const OnyxUtils = { getDeferredInitTask, initStoreValues, sendActionToDevTools, - maybeFlushBatchUpdates, - batchUpdates, get, getAllKeys, getCollectionKeys, @@ -1487,10 +1436,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 diff --git a/lib/batch.native.ts b/lib/batch.native.ts deleted file mode 100644 index fb7ef4ee5..000000000 --- a/lib/batch.native.ts +++ /dev/null @@ -1,3 +0,0 @@ -import {unstable_batchedUpdates} from 'react-native'; - -export default unstable_batchedUpdates; diff --git a/lib/batch.ts b/lib/batch.ts deleted file mode 100644 index 3ff0368fe..000000000 --- a/lib/batch.ts +++ /dev/null @@ -1,3 +0,0 @@ -import {unstable_batchedUpdates} from 'react-dom'; - -export default unstable_batchedUpdates; diff --git a/package-lock.json b/package-lock.json index 16b6c2bc5..3e2240108 100644 --- a/package-lock.json +++ b/package-lock.json @@ -31,7 +31,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", @@ -53,7 +52,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", @@ -72,7 +70,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", @@ -4111,16 +4108,6 @@ "csstype": "^3.0.2" } }, - "node_modules/@types/react-dom": { - "version": "18.3.7", - "resolved": "https://registry.npmjs.org/@types/react-dom/-/react-dom-18.3.7.tgz", - "integrity": "sha512-MEe3UeoENYVFXzoXEWsvcpg6ZvlrFNlOQ7EOsvhI3CfAXwzPfO8Qwuxd40nepsYKqyyVQnTdEfv68q91yLcKrQ==", - "dev": true, - "license": "MIT", - "peerDependencies": { - "@types/react": "^18.0.0" - } - }, "node_modules/@types/react-native": { "version": "0.70.19", "resolved": "https://registry.npmjs.org/@types/react-native/-/react-native-0.70.19.tgz", @@ -12745,20 +12732,6 @@ } } }, - "node_modules/react-dom": { - "version": "18.2.0", - "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-18.2.0.tgz", - "integrity": "sha512-6IMTriUmvsjHUjNtEDudZfuDQUoWXVxKHhlEGSk81n4YFS+r/Kl99wXiwlVXtPBtJenozv2P+hxDsw9eA7Xo6g==", - "dev": true, - "license": "MIT", - "dependencies": { - "loose-envify": "^1.1.0", - "scheduler": "^0.23.0" - }, - "peerDependencies": { - "react": "^18.2.0" - } - }, "node_modules/react-is": { "version": "18.3.1", "resolved": "https://registry.npmjs.org/react-is/-/react-is-18.3.1.tgz", diff --git a/package.json b/package.json index 6200c179a..94e25eaea 100644 --- a/package.json +++ b/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", diff --git a/tests/perf-test/OnyxUtils.perf-test.ts b/tests/perf-test/OnyxUtils.perf-test.ts index 5a817b859..f04deb931 100644 --- a/tests/perf-test/OnyxUtils.perf-test.ts +++ b/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))), { diff --git a/tests/unit/onyxUtilsTest.ts b/tests/unit/onyxUtilsTest.ts index 9b120a580..88ce3e8dd 100644 --- a/tests/unit/onyxUtilsTest.ts +++ b/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 From fec6d6bcebc086f1ad14170b4d91706df03fe1d1 Mon Sep 17 00:00:00 2001 From: VickyStash Date: Thu, 16 Oct 2025 16:18:09 +0200 Subject: [PATCH 2/3] Make code more readable --- lib/OnyxUtils.ts | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index f87638e6e..c7421dcfe 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -753,6 +753,13 @@ function getCollectionDataAndSendAsObject(matchingKeys: Co }); } +// !!!DO NOT MERGE THIS CODE, METHODS FOR READABILITY ONLY +const nextMicrotask = () => Promise.resolve(); +const nextMacrotask = () => + new Promise((resolve) => { + setTimeout(resolve, 0); + }); + /** * Schedules an update that will be appended to the macro task queue (so it doesn't update the subscribers immediately). * @@ -764,11 +771,7 @@ function scheduleSubscriberUpdate( value: OnyxValue, canUpdateSubscriber: (subscriber?: CallbackToStateMapping) => boolean = () => true, ): Promise { - const promise0 = new Promise((resolve) => { - setTimeout(resolve, 0); - }); - const promise = Promise.resolve().then(() => keyChanged(key, value, canUpdateSubscriber)); - return Promise.all([promise0, promise]).then(() => undefined); + return Promise.all([nextMacrotask(), nextMicrotask().then(() => keyChanged(key, value, canUpdateSubscriber))]).then(() => undefined); } /** @@ -781,13 +784,7 @@ function scheduleNotifyCollectionSubscribers( value: OnyxCollection, previousValue?: OnyxCollection, ): Promise { - const promise0 = new Promise((resolve) => { - setTimeout(() => { - resolve(); - }, 0); - }); - const promise = Promise.resolve().then(() => keysChanged(key, value, previousValue)); - return Promise.all([promise0, promise]).then(() => undefined); + return Promise.all([nextMacrotask(), nextMicrotask().then(() => keysChanged(key, value, previousValue))]).then(() => undefined); } /** From 6308802a1f0a522d0a181be42afb57bd68e5d70e Mon Sep 17 00:00:00 2001 From: VickyStash Date: Tue, 21 Oct 2025 16:19:27 +0200 Subject: [PATCH 3/3] Have one next macrotask instead of multiple --- lib/OnyxUtils.ts | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index c7421dcfe..a0e9c2842 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -755,9 +755,13 @@ function getCollectionDataAndSendAsObject(matchingKeys: Co // !!!DO NOT MERGE THIS CODE, METHODS FOR READABILITY ONLY const nextMicrotask = () => Promise.resolve(); +let nextMacrotaskPromise: Promise | null = null; const nextMacrotask = () => new Promise((resolve) => { - setTimeout(resolve, 0); + setTimeout(() => { + nextMacrotaskPromise = null; + resolve(); + }, 0); }); /** @@ -771,7 +775,10 @@ function scheduleSubscriberUpdate( value: OnyxValue, canUpdateSubscriber: (subscriber?: CallbackToStateMapping) => boolean = () => true, ): Promise { - return Promise.all([nextMacrotask(), nextMicrotask().then(() => keyChanged(key, value, canUpdateSubscriber))]).then(() => undefined); + if (!nextMacrotaskPromise) { + nextMacrotaskPromise = nextMacrotask(); + } + return Promise.all([nextMacrotaskPromise, nextMicrotask().then(() => keyChanged(key, value, canUpdateSubscriber))]).then(() => undefined); } /** @@ -784,7 +791,10 @@ function scheduleNotifyCollectionSubscribers( value: OnyxCollection, previousValue?: OnyxCollection, ): Promise { - return Promise.all([nextMacrotask(), nextMicrotask().then(() => keysChanged(key, value, previousValue))]).then(() => undefined); + if (!nextMacrotaskPromise) { + nextMacrotaskPromise = nextMacrotask(); + } + return Promise.all([nextMacrotaskPromise, nextMicrotask().then(() => keysChanged(key, value, previousValue))]).then(() => undefined); } /**