diff --git a/jestSetup.js b/jestSetup.js index 5c09ddf4..f51f0d54 100644 --- a/jestSetup.js +++ b/jestSetup.js @@ -9,6 +9,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/Onyx.ts b/lib/Onyx.ts index 27751ae6..bebe3cb9 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -62,7 +62,7 @@ function init({ // Setting isProcessingCollectionUpdate=true prevents triggering collection callbacks for each individual update const isKeyCollectionMember = OnyxUtils.isCollectionMember(key); - OnyxUtils.keyChanged(key, value as OnyxValue, undefined, true, isKeyCollectionMember); + OnyxUtils.keyChanged(key, value as OnyxValue, undefined, isKeyCollectionMember); }); } diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 669eda86..05deec4b 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, @@ -77,6 +76,9 @@ type OnyxMethod = ValueOf; let mergeQueue: Record>> = {}; let mergeQueuePromise: Record> = {}; +// Used to schedule subscriber update to the macro task queue +let nextMacrotaskPromise: Promise | null = null; + // Holds a mapping of all the React components that want their state subscribed to a store key let callbackToStateMapping: Record> = {}; @@ -89,9 +91,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>(); @@ -213,43 +212,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. @@ -635,7 +597,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(). @@ -671,10 +632,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) { @@ -724,7 +681,6 @@ function keyChanged( key: TKey, value: OnyxValue, canUpdateSubscriber: (subscriber?: CallbackToStateMapping) => boolean = () => true, - notifyConnectSubscribers = true, isProcessingCollectionUpdate = false, ): void { // Add or remove this key from the recentlyAccessedKeys lists @@ -766,9 +722,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; } @@ -851,6 +804,23 @@ function getCollectionDataAndSendAsObject(matchingKeys: Co }); } +/** + * Delays promise resolution until the next macrotask to prevent race condition if the key subscription is in progress. + * + * @param callback The keyChanged/keysChanged callback + * */ +function prepareSubscriberUpdate(callback: () => void): Promise { + if (!nextMacrotaskPromise) { + nextMacrotaskPromise = new Promise((resolve) => { + setTimeout(() => { + nextMacrotaskPromise = null; + resolve(); + }, 0); + }); + } + return Promise.all([nextMacrotaskPromise, Promise.resolve().then(callback)]).then(); +} + /** * Schedules an update that will be appended to the macro task queue (so it doesn't update the subscribers immediately). * @@ -863,13 +833,11 @@ function scheduleSubscriberUpdate( canUpdateSubscriber: (subscriber?: CallbackToStateMapping) => boolean = () => true, isProcessingCollectionUpdate = false, ): Promise { - const promise = Promise.resolve().then(() => keyChanged(key, value, canUpdateSubscriber, true, isProcessingCollectionUpdate)); - batchUpdates(() => keyChanged(key, value, canUpdateSubscriber, false, isProcessingCollectionUpdate)); - return Promise.all([maybeFlushBatchUpdates(), promise]).then(() => undefined); + return prepareSubscriberUpdate(() => keyChanged(key, value, canUpdateSubscriber, isProcessingCollectionUpdate)); } /** - * This method is similar to notifySubscribersOnNextTick but it is built for working specifically with collections + * This method is similar to scheduleSubscriberUpdate but it is built for working specifically with collections * so that keysChanged() is triggered for the collection and not keyChanged(). If this was not done, then the * subscriber callbacks receive the data in a different format than they normally expect and it breaks code. */ @@ -878,9 +846,7 @@ 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); + return prepareSubscriberUpdate(() => keysChanged(key, value, previousValue)); } /** @@ -1691,7 +1657,6 @@ function clearOnyxUtilsInternals() { mergeQueuePromise = {}; callbackToStateMapping = {}; onyxKeyToSubscriptionIDs = new Map(); - batchUpdatesQueue = []; lastConnectionCallbackData = new Map(); } @@ -1703,8 +1668,6 @@ const OnyxUtils = { getDeferredInitTask, initStoreValues, sendActionToDevTools, - maybeFlushBatchUpdates, - batchUpdates, get, getAllKeys, getCollectionKeys, @@ -1762,10 +1725,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 fb7ef4ee..00000000 --- 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 3ff0368f..00000000 --- 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 3787067d..1404602e 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", @@ -54,7 +53,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.27.2", @@ -73,7 +71,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.27.2", @@ -4112,16 +4109,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", @@ -12862,20 +12849,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 9812f3c5..1d092ac1 100644 --- a/package.json +++ b/package.json @@ -65,7 +65,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", @@ -88,7 +87,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.27.2", @@ -103,7 +101,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.27.2", diff --git a/tests/perf-test/OnyxUtils.perf-test.ts b/tests/perf-test/OnyxUtils.perf-test.ts index 26905822..041b5a16 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 a390ed79..36e1dd0e 100644 --- a/tests/unit/onyxUtilsTest.ts +++ b/tests/unit/onyxUtilsTest.ts @@ -292,7 +292,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