diff --git a/API-INTERNAL.md b/API-INTERNAL.md index 38a5d76c..4e29291a 100644 --- a/API-INTERNAL.md +++ b/API-INTERNAL.md @@ -26,12 +26,6 @@
initStoreValues(keys, initialKeyStates, evictableKeys)

Sets the initial values for the Onyx store

-
maybeFlushBatchUpdates()
-

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.

-
reduceCollectionWithSelector()

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. @@ -61,6 +55,9 @@ to the values for those keys (correctly typed) such as [OnyxCollection<

Checks to see if the subscriber's supplied key is associated with a collection of keys.

+
isCollectionMember(key) ⇒
+

Checks if a given key is a collection member key (not just a collection key).

+
splitCollectionMemberKey(key, collectionKey) ⇒

Splits a collection member key into the collection key part and the ID part.

@@ -98,11 +95,14 @@ run out of storage the least recently accessed key can be removed.

getCollectionDataAndSendAsObject()

Gets the data for a given an array of matching keys, combines them into an object, and sends the result back to the subscriber.

+
prepareSubscriberUpdate(callback)
+

Delays promise resolution until the next macrotask to prevent race condition if the key subscription is in progress.

+
scheduleSubscriberUpdate()

Schedules an update that will be appended to the macro task queue (so it doesn't update the subscribers immediately).

scheduleNotifyCollectionSubscribers()
-

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.

@@ -230,15 +230,6 @@ Sets the initial values for the Onyx store | initialKeyStates | initial data to set when `init()` and `clear()` are called | | evictableKeys | This is an array of keys (individual or collection patterns) that when provided to Onyx are flagged as "safe" for removal. | - - -## maybeFlushBatchUpdates() -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. - -**Kind**: global function ## reduceCollectionWithSelector() @@ -304,6 +295,18 @@ Checks to see if the subscriber's supplied key is associated with a collection of keys. **Kind**: global function + + +## isCollectionMember(key) ⇒ +Checks if a given key is a collection member key (not just a collection key). + +**Kind**: global function +**Returns**: true if the key is a collection member, false otherwise + +| Param | Description | +| --- | --- | +| key | The key to check | + ## splitCollectionMemberKey(key, collectionKey) ⇒ @@ -402,6 +405,17 @@ run out of storage the least recently accessed key can be removed. Gets the data for a given an array of matching keys, combines them into an object, and sends the result back to the subscriber. **Kind**: global function + + +## prepareSubscriberUpdate(callback) +Delays promise resolution until the next macrotask to prevent race condition if the key subscription is in progress. + +**Kind**: global function + +| Param | Description | +| --- | --- | +| callback | The keyChanged/keysChanged callback | + ## scheduleSubscriberUpdate() @@ -415,7 +429,7 @@ scheduleSubscriberUpdate(key, value, subscriber => subscriber.initWithStoredValu ## scheduleNotifyCollectionSubscribers() -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. diff --git a/API.md b/API.md index 8870f788..545adbfa 100644 --- a/API.md +++ b/API.md @@ -177,7 +177,7 @@ applied in the order they were called. Note: `Onyx.set()` calls do not work this **Example** ```js Onyx.merge(ONYXKEYS.EMPLOYEE_LIST, ['Joe']); // -> ['Joe'] -Onyx.merge(ONYXKEYS.EMPLOYEE_LIST, ['Jack']); // -> ['Jack'] +Onyx.merge(ONYXKEYS.EMPLOYEE_LIST, ['Jack']); // -> ['Joe', 'Jack'] Onyx.merge(ONYXKEYS.POLICY, {id: 1}); // -> {id: 1} Onyx.merge(ONYXKEYS.POLICY, {name: 'My Workspace'}); // -> {id: 1, name: 'My Workspace'} ``` 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 bfae1c73..7be47472 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 3376b9b9..88939742 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -7,7 +7,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, @@ -76,6 +75,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> = {}; @@ -88,9 +90,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>(); @@ -212,43 +211,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(() => { - for (const applyUpdates of updatesCopy) { - 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. @@ -634,7 +596,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(). @@ -670,10 +631,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) { @@ -723,7 +680,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 @@ -765,9 +721,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; } @@ -850,6 +803,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). * @@ -862,13 +832,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. */ @@ -877,9 +845,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)); } /** @@ -1692,7 +1658,6 @@ function clearOnyxUtilsInternals() { mergeQueuePromise = {}; callbackToStateMapping = {}; onyxKeyToSubscriptionIDs = new Map(); - batchUpdatesQueue = []; lastConnectionCallbackData = new Map(); } @@ -1704,8 +1669,6 @@ const OnyxUtils = { getDeferredInitTask, initStoreValues, sendActionToDevTools, - maybeFlushBatchUpdates, - batchUpdates, get, getAllKeys, getCollectionKeys, @@ -1763,10 +1726,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 318a2e1f..8a6d1b8f 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": "^8.51.0", @@ -55,7 +54,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", @@ -74,7 +72,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", @@ -4304,16 +4301,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", @@ -13471,20 +13458,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 d53841e9..abae1660 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": "^8.51.0", @@ -89,7 +88,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", @@ -104,7 +102,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 2dd9b7f4..28e30e79 100644 --- a/tests/perf-test/OnyxUtils.perf-test.ts +++ b/tests/perf-test/OnyxUtils.perf-test.ts @@ -110,13 +110,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