Implement RAM-only key support to replace initWithStoredValues#725
Conversation
… from initStoreValues
lib/Onyx.ts
Outdated
| // Clean up any pre-existing RAM-only keys from storage | ||
| if (ramOnlyKeys.length > 0) { | ||
| Storage.getAllKeys().then((storedKeys) => { | ||
| const keysToRemove = Array.from(storedKeys).filter((key) => OnyxUtils.isRamOnlyKey(key)); | ||
| if (keysToRemove.length > 0) { | ||
| Storage.removeItems(keysToRemove); | ||
| } | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't really get this... is this just temporary? Otherwise, how could RAM only keys be in storage at all?
There was a problem hiding this comment.
If it's only temporary, then I think we need a better way of doing this. I think it should be done as an Onyx migration.
There was a problem hiding this comment.
I don't think this is temporary, it's for the scenario when you have a regular key and you decide to convert it into a RAM-only key - it needs to be removed from storage. It's not just about rolling out this new feature, that kind of conversion might happen anytime, so we need a permanent solution. Do you think we should change it?
There was a problem hiding this comment.
Ok the scenario @JKobrynski described is real but I agree that it's probably more appropriate to deal these scenarios with a migration file in Onyx. Since this piece of code will add some overhead to initialisation every single time, maybe it's better to just put a comment above ramOnlyKeys when declaring it in E/App code, that every addition there should be followed with a migration file.
There was a problem hiding this comment.
Thanks, Fabio, I agree. It's not the job of internal Onyx code to handle and respond to a consumer adding/removing RAM only keys from the config. I think it's up to the consumer to manage that.
There was a problem hiding this comment.
Migration makes sense for this kind of a problem
…91-implement-ramonly-key-support
There was a problem hiding this comment.
NAB: add an OnyxKeyManager utility class to better encapsulate key management.
diff
diff --git a/lib/Onyx.ts b/lib/Onyx.ts
index f5f8550..3d00898 100644
--- a/lib/Onyx.ts
+++ b/lib/Onyx.ts
@@ -3,6 +3,7 @@ import cache, {TASK} from './OnyxCache';
import Storage from './storage';
import utils from './utils';
import DevTools, {initDevTools} from './DevTools';
+import keyManager from './OnyxKeyManager';
import type {
CollectionKeyBase,
ConnectOptions,
@@ -52,15 +53,16 @@ function init({
Storage.init();
+ // Initialize key manager first so it can be used by other initialization steps
+ keyManager.init(keys, ramOnlyKeys);
+
OnyxUtils.setSkippableCollectionMemberIDs(new Set(skippableCollectionMemberIDs));
OnyxUtils.setSnapshotMergeKeys(new Set(snapshotMergeKeys));
- cache.setRamOnlyKeys(new Set<OnyxKey>(ramOnlyKeys));
-
// Clean up any pre-existing RAM-only keys from storage
if (ramOnlyKeys.length > 0) {
Storage.getAllKeys().then((storedKeys) => {
- const keysToRemove = Array.from(storedKeys).filter((key) => OnyxUtils.isRamOnlyKey(key));
+ const keysToRemove = Array.from(storedKeys).filter((key) => keyManager.isRamOnlyKey(key));
if (keysToRemove.length > 0) {
Storage.removeItems(keysToRemove);
}
@@ -74,7 +76,14 @@ function init({
// Check if this is a collection member key to prevent duplicate callbacks
// When a collection is updated, individual members sync separately to other tabs
// Setting isProcessingCollectionUpdate=true prevents triggering collection callbacks for each individual update
- const isKeyCollectionMember = OnyxUtils.isCollectionMember(key);
+ let isKeyCollectionMember = false;
+ try {
+ const collectionKey = keyManager.getCollectionKey(key);
+ // If the key is longer than the collection key, it's a collection member
+ isKeyCollectionMember = key.length > collectionKey.length;
+ } catch {
+ // If getCollectionKey throws, the key is not a collection member
+ }
OnyxUtils.keyChanged(key, value as OnyxValue<typeof key>, undefined, isKeyCollectionMember);
});
@@ -84,10 +93,10 @@ function init({
cache.setRecentKeysLimit(maxCachedKeysCount);
}
- OnyxUtils.initStoreValues(keys, initialKeyStates, evictableKeys);
+ OnyxUtils.initStoreValues(initialKeyStates, evictableKeys, keys);
// Initialize all of our keys with data provided then give green light to any pending connections
- Promise.all([cache.addEvictableKeysToRecentlyAccessedList(OnyxUtils.isCollectionKey, OnyxUtils.getAllKeys), OnyxUtils.initializeWithDefaultKeyStates()]).then(
+ Promise.all([cache.addEvictableKeysToRecentlyAccessedList(keyManager.isCollectionKey.bind(keyManager), OnyxUtils.getAllKeys), OnyxUtils.initializeWithDefaultKeyStates()]).then(
OnyxUtils.getDeferredInitTask().resolve,
);
}
@@ -204,7 +213,7 @@ function merge<TKey extends OnyxKey>(key: TKey, changes: OnyxMergeInput<TKey>):
const skippableCollectionMemberIDs = OnyxUtils.getSkippableCollectionMemberIDs();
if (skippableCollectionMemberIDs.size) {
try {
- const [, collectionMemberID] = OnyxUtils.splitCollectionMemberKey(key);
+ const [, collectionMemberID] = keyManager.splitCollectionMemberKey(key);
if (skippableCollectionMemberIDs.has(collectionMemberID)) {
// The key is a skippable one, so we set the new changes to undefined.
// eslint-disable-next-line no-param-reassign
@@ -355,7 +364,7 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise<void> {
let collectionKey: string | undefined;
try {
- collectionKey = OnyxUtils.getCollectionKey(key);
+ collectionKey = keyManager.getCollectionKey(key);
} catch (e) {
// If getCollectionKey() throws an error it means the key is not a collection key.
collectionKey = undefined;
@@ -393,7 +402,7 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise<void> {
const defaultKeyValuePairs = Object.entries(
Object.keys(defaultKeyStates)
- .filter((key) => !keysToPreserve.includes(key) && !OnyxUtils.isRamOnlyKey(key))
+ .filter((key) => !keysToPreserve.includes(key) && !keyManager.isRamOnlyKey(key))
.reduce((obj: KeyValueMapping, key) => {
// eslint-disable-next-line no-param-reassign
obj[key] = defaultKeyStates[key];
@@ -416,6 +425,16 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise<void> {
return cache.captureTask(TASK.CLEAR, promise) as Promise<void>;
}
+/**
+ * Queued operations for a single key, with explicit type to indicate set vs merge.
+ */
+type QueuedOperations = {
+ /** 'set' replaces the value entirely, 'merge' merges with existing */
+ type: 'set' | 'merge';
+ /** The values to set or merge (in order) */
+ values: Array<OnyxValue<OnyxKey>>;
+};
+
/**
* Insert API responses and lifecycle data into Onyx
*
@@ -423,112 +442,142 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise<void> {
* @returns resolves when all operations are complete
*/
function update<TKey extends OnyxKey>(data: Array<OnyxUpdate<TKey>>): Promise<void> {
- // First, validate the Onyx object is in the format we expect
- for (const {onyxMethod, key, value} of data) {
- if (!Object.values(OnyxUtils.METHOD).includes(onyxMethod)) {
- throw new Error(`Invalid onyxMethod ${onyxMethod} in Onyx update.`);
- }
- if (onyxMethod === OnyxUtils.METHOD.MULTI_SET) {
- // For multiset, we just expect the value to be an object
- if (typeof value !== 'object' || Array.isArray(value) || typeof value === 'function') {
- throw new Error('Invalid value provided in Onyx multiSet. Onyx multiSet value must be of type object.');
+ // Collection member keys are routed here for potential batching: collectionKey -> memberKey -> operations
+ const collectionBatches: Record<string, Record<string, QueuedOperations>> = {};
+
+ // Non-collection keys are routed here for individual processing: key -> operations
+ const individualUpdates: Record<OnyxKey, QueuedOperations> = {};
+
+ const promises: Array<() => Promise<void>> = [];
+ let clearPromise: Promise<void> = Promise.resolve();
+
+ // Gets or creates the QueuedOperations for a key, routing to the appropriate structure
+ const getOrCreateQueuedOperations = (key: OnyxKey): QueuedOperations => {
+ const collectionKey = keyManager.getMemberCollectionKey(key);
+ if (collectionKey) {
+ if (!collectionBatches[collectionKey]) {
+ collectionBatches[collectionKey] = {};
+ }
+ if (!collectionBatches[collectionKey][key]) {
+ collectionBatches[collectionKey][key] = {type: 'merge', values: []};
}
- } else if (onyxMethod !== OnyxUtils.METHOD.CLEAR && typeof key !== 'string') {
- throw new Error(`Invalid ${typeof key} key provided in Onyx update. Onyx key must be of type string.`);
+ return collectionBatches[collectionKey][key];
}
- }
+ if (!individualUpdates[key]) {
+ individualUpdates[key] = {type: 'merge', values: []};
+ }
+ return individualUpdates[key];
+ };
- // The queue of operations within a single `update` call in the format of <item key - list of operations updating the item>.
- // This allows us to batch the operations per item and merge them into one operation in the order they were requested.
- const updateQueue: Record<OnyxKey, Array<OnyxValue<OnyxKey>>> = {};
- const enqueueSetOperation = (key: OnyxKey, value: OnyxValue<OnyxKey>) => {
- // If a `set` operation is enqueued, we should clear the whole queue.
- // Since the `set` operation replaces the value entirely, there's no need to perform any previous operations.
- // To do this, we first put `null` in the queue, which removes the existing value, and then merge the new value.
- updateQueue[key] = [null, value];
+ // Enqueue a set operation (replaces any previous operations for this key)
+ const enqueueSet = (key: OnyxKey, value: OnyxValue<OnyxKey>) => {
+ const queued = getOrCreateQueuedOperations(key);
+ queued.type = 'set';
+ queued.values = [value];
};
- const enqueueMergeOperation = (key: OnyxKey, value: OnyxValue<OnyxKey>) => {
+
+ // Enqueue a merge operation (appends to existing operations, or converts to set if value is null)
+ const enqueueMerge = (key: OnyxKey, value: OnyxValue<OnyxKey>) => {
+ const queued = getOrCreateQueuedOperations(key);
if (value === null) {
- // If we merge `null`, the value is removed and all the previous operations are discarded.
- updateQueue[key] = [null];
- } else if (!updateQueue[key]) {
- updateQueue[key] = [value];
+ // Merging null means "delete this key" - treat as a set with no value
+ queued.type = 'set';
+ queued.values = [];
} else {
- updateQueue[key].push(value);
+ queued.values.push(value);
}
};
- const promises: Array<() => Promise<void>> = [];
- let clearPromise: Promise<void> = Promise.resolve();
-
+ // Validate updates and route them to the appropriate queue
for (const {onyxMethod, key, value} of data) {
- const handlers: Record<OnyxMethodMap[keyof OnyxMethodMap], (k: typeof key, v: typeof value) => void> = {
- [OnyxUtils.METHOD.SET]: enqueueSetOperation,
- [OnyxUtils.METHOD.MERGE]: enqueueMergeOperation,
- [OnyxUtils.METHOD.MERGE_COLLECTION]: () => {
+ switch (onyxMethod) {
+ case OnyxUtils.METHOD.SET:
+ keyManager.assertValidKey(key);
+ enqueueSet(key, value);
+ break;
+
+ case OnyxUtils.METHOD.MERGE:
+ keyManager.assertValidKey(key);
+ enqueueMerge(key, value);
+ break;
+
+ case OnyxUtils.METHOD.MERGE_COLLECTION: {
+ keyManager.assertValidKey(key);
const collection = value as OnyxMergeCollectionInput<OnyxKey>;
if (!OnyxUtils.isValidNonEmptyCollectionForMerge(collection)) {
Logger.logInfo('mergeCollection enqueued within update() with invalid or empty value. Skipping this operation.');
- return;
+ break;
}
-
- // Confirm all the collection keys belong to the same parent
const collectionKeys = Object.keys(collection);
if (OnyxUtils.doAllCollectionItemsBelongToSameParent(key, collectionKeys)) {
const mergedCollection: OnyxInputKeyValueMapping = collection;
- for (const collectionKey of collectionKeys) enqueueMergeOperation(collectionKey, mergedCollection[collectionKey]);
+ for (const collectionKey of collectionKeys) {
+ enqueueMerge(collectionKey, mergedCollection[collectionKey]);
+ }
+ }
+ break;
+ }
+
+ case OnyxUtils.METHOD.SET_COLLECTION:
+ keyManager.assertValidKey(key);
+ promises.push(() => setCollection(key as TKey, value as OnyxSetCollectionInput<TKey>));
+ break;
+
+ case OnyxUtils.METHOD.MULTI_SET:
+ if (typeof value !== 'object' || Array.isArray(value) || typeof value === 'function') {
+ throw new Error('Invalid value provided in Onyx multiSet. Onyx multiSet value must be of type object.');
}
- },
- [OnyxUtils.METHOD.SET_COLLECTION]: (k, v) => promises.push(() => setCollection(k as TKey, v as OnyxSetCollectionInput<TKey>)),
- [OnyxUtils.METHOD.MULTI_SET]: (k, v) => {
- for (const [entryKey, entryValue] of Object.entries(v as Partial<OnyxInputKeyValueMapping>)) enqueueSetOperation(entryKey, entryValue);
- },
- [OnyxUtils.METHOD.CLEAR]: () => {
+ for (const [entryKey, entryValue] of Object.entries(value as Partial<OnyxInputKeyValueMapping>)) {
+ enqueueSet(entryKey, entryValue);
+ }
+ break;
+
+ case OnyxUtils.METHOD.CLEAR:
clearPromise = clear();
- },
- };
+ break;
- handlers[onyxMethod](key, value);
+ default:
+ throw new Error(`Invalid onyxMethod ${onyxMethod} in Onyx update.`);
+ }
}
- // Group all the collection-related keys and update each collection in a single `mergeCollection` call.
- // This is needed to prevent multiple `mergeCollection` calls for the same collection and `merge` calls for the individual items of the said collection.
- // This way, we ensure there is no race condition in the queued updates of the same key.
- for (const collectionKey of OnyxUtils.getCollectionKeys()) {
- const collectionItemKeys = Object.keys(updateQueue).filter((key) => OnyxUtils.isKeyMatch(collectionKey, key));
- if (collectionItemKeys.length <= 1) {
- // If there are no items of this collection in the updateQueue, we should skip it.
- // If there is only one item, we should update it individually, therefore retain it in the updateQueue.
+ // Process collection batches - batch if 2+ items, otherwise process individually
+ for (const [collectionKey, members] of Object.entries(collectionBatches)) {
+ const memberKeys = Object.keys(members);
+
+ if (memberKeys.length === 1) {
+ // Single item - process individually (more efficient than collection operation)
+ const key = memberKeys[0];
+ const queued = members[key];
+ if (queued.type === 'set') {
+ const batchedChanges = OnyxUtils.mergeChanges(queued.values).result;
+ promises.push(() => set(key, batchedChanges));
+ } else {
+ for (const value of queued.values) {
+ promises.push(() => merge(key, value));
+ }
+ }
continue;
}
- const batchedCollectionUpdates = collectionItemKeys.reduce(
- (queue: MixedOperationsQueue, key: string) => {
- const operations = updateQueue[key];
-
- // Remove the collection-related key from the updateQueue so that it won't be processed individually.
- delete updateQueue[key];
-
- const batchedChanges = OnyxUtils.mergeAndMarkChanges(operations);
- if (operations[0] === null) {
- // eslint-disable-next-line no-param-reassign
- queue.set[key] = batchedChanges.result;
- } else {
- // eslint-disable-next-line no-param-reassign
- queue.merge[key] = batchedChanges.result;
- if (batchedChanges.replaceNullPatches.length > 0) {
- // eslint-disable-next-line no-param-reassign
- queue.mergeReplaceNullPatches[key] = batchedChanges.replaceNullPatches;
- }
+ // Multiple items - batch into collection operations
+ const batchedCollectionUpdates: MixedOperationsQueue = {
+ merge: {},
+ mergeReplaceNullPatches: {},
+ set: {},
+ };
+
+ for (const [key, queued] of Object.entries(members)) {
+ const batchedChanges = OnyxUtils.mergeAndMarkChanges(queued.values);
+ if (queued.type === 'set') {
+ batchedCollectionUpdates.set[key] = batchedChanges.result;
+ } else {
+ batchedCollectionUpdates.merge[key] = batchedChanges.result;
+ if (batchedChanges.replaceNullPatches.length > 0) {
+ batchedCollectionUpdates.mergeReplaceNullPatches[key] = batchedChanges.replaceNullPatches;
}
- return queue;
- },
- {
- merge: {},
- mergeReplaceNullPatches: {},
- set: {},
- },
- );
+ }
+ }
if (!utils.isEmptyObject(batchedCollectionUpdates.merge)) {
promises.push(() =>
@@ -545,15 +594,16 @@ function update<TKey extends OnyxKey>(data: Array<OnyxUpdate<TKey>>): Promise<vo
}
}
- for (const [key, operations] of Object.entries(updateQueue)) {
- if (operations[0] === null) {
- const batchedChanges = OnyxUtils.mergeChanges(operations).result;
+ // Process individual (non-collection) updates
+ for (const [key, queued] of Object.entries(individualUpdates)) {
+ if (queued.type === 'set') {
+ const batchedChanges = OnyxUtils.mergeChanges(queued.values).result;
promises.push(() => set(key, batchedChanges));
continue;
}
- for (const operation of operations) {
- promises.push(() => merge(key, operation));
+ for (const value of queued.values) {
+ promises.push(() => merge(key, value));
}
}
diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts
index aa6d670..5ad6363 100644
--- a/lib/OnyxCache.ts
+++ b/lib/OnyxCache.ts
@@ -4,6 +4,7 @@ import type {ValueOf} from 'type-fest';
import utils from './utils';
import type {OnyxKey, OnyxValue} from './types';
import * as Str from './Str';
+import keyManager from './OnyxKeyManager';
// Task constants
const TASK = {
@@ -52,12 +53,6 @@ class OnyxCache {
/** List of keys that have been directly subscribed to or recently modified from least to most recent */
private recentlyAccessedKeys = new Set<OnyxKey>();
- /** Set of collection keys for fast lookup */
- private collectionKeys = new Set<OnyxKey>();
-
- /** Set of RAM-only keys for fast lookup */
- private ramOnlyKeys = new Set<OnyxKey>();
-
constructor() {
this.storageKeys = new Set();
this.nullishStorageKeys = new Set();
@@ -93,12 +88,8 @@ class OnyxCache {
'addLastAccessedKey',
'addEvictableKeysToRecentlyAccessedList',
'getKeyForEviction',
- 'setCollectionKeys',
- 'isCollectionKey',
- 'getCollectionKey',
+ 'initCollectionKeys',
'getCollectionData',
- 'setRamOnlyKeys',
- 'isRamOnlyKey',
);
}
@@ -172,7 +163,7 @@ class OnyxCache {
// since it will either be set to a non nullish value or removed from the cache completely.
this.nullishStorageKeys.delete(key);
- const collectionKey = this.getCollectionKey(key);
+ const collectionKey = keyManager.getMemberCollectionKey(key);
if (value === null || value === undefined) {
delete this.storageMap[key];
@@ -201,13 +192,13 @@ class OnyxCache {
delete this.storageMap[key];
// Remove from collection data cache if this is a collection member
- const collectionKey = this.getCollectionKey(key);
+ const collectionKey = keyManager.getMemberCollectionKey(key);
if (collectionKey && this.collectionData[collectionKey]) {
delete this.collectionData[collectionKey][key];
}
// If this is a collection key, clear its data
- if (this.isCollectionKey(key)) {
+ if (keyManager.isCollectionKey(key)) {
delete this.collectionData[key];
}
@@ -235,7 +226,7 @@ class OnyxCache {
this.addKey(key);
this.addToAccessedKeys(key);
- const collectionKey = this.getCollectionKey(key);
+ const collectionKey = keyManager.getMemberCollectionKey(key);
if (value === null || value === undefined) {
this.addNullishStorageKey(key);
@@ -325,7 +316,7 @@ class OnyxCache {
delete this.storageMap[key];
// Remove from collection data cache if this is a collection member
- const collectionKey = this.getCollectionKey(key);
+ const collectionKey = keyManager.getMemberCollectionKey(key);
if (collectionKey && this.collectionData[collectionKey]) {
delete this.collectionData[collectionKey][key];
}
@@ -368,13 +359,15 @@ class OnyxCache {
}
/**
- * Check if a given key matches a pattern key
- * @param configKey - Pattern that may contain a wildcard
- * @param key - Key to test against the pattern
+ * Check if a given key matches a pattern key using simple string matching.
+ *
+ * Unlike keyManager.isKeyMatch (which checks against registered collection keys),
+ * this treats ANY key ending with '_' as a collection pattern. This is necessary
+ * because evictableKeys can be arbitrary patterns not registered in ONYXKEYS.
*/
- private isKeyMatch(configKey: OnyxKey, key: OnyxKey): boolean {
- const isCollectionKey = configKey.endsWith('_');
- return isCollectionKey ? Str.startsWith(key, configKey) : configKey === key;
+ private isKeyMatch(patternKey: OnyxKey, key: OnyxKey): boolean {
+ const isCollectionPattern = patternKey.endsWith('_');
+ return isCollectionPattern ? Str.startsWith(key, patternKey) : patternKey === key;
}
/**
@@ -434,11 +427,10 @@ class OnyxCache {
}
/**
- * Set the collection keys for optimized storage
+ * Initialize the collection keys for optimized storage.
+ * Sets up the collection data cache for tracking collection members.
*/
- setCollectionKeys(collectionKeys: Set<OnyxKey>): void {
- this.collectionKeys = collectionKeys;
-
+ initCollectionKeys(collectionKeys: Set<OnyxKey>): void {
// Initialize collection data for existing collection keys
for (const collectionKey of collectionKeys) {
if (this.collectionData[collectionKey]) {
@@ -448,25 +440,6 @@ class OnyxCache {
}
}
- /**
- * Check if a key is a collection key
- */
- isCollectionKey(key: OnyxKey): boolean {
- return this.collectionKeys.has(key);
- }
-
- /**
- * Get the collection key for a given member key
- */
- getCollectionKey(key: OnyxKey): OnyxKey | null {
- for (const collectionKey of this.collectionKeys) {
- if (key.startsWith(collectionKey) && key.length > collectionKey.length) {
- return collectionKey;
- }
- }
- return null;
- }
-
/**
* Get all data for a collection key
*/
@@ -479,20 +452,6 @@ class OnyxCache {
// Return a shallow copy to ensure React detects changes when items are added/removed
return {...cachedCollection};
}
-
- /**
- * Set the RAM-only keys for optimized storage
- */
- setRamOnlyKeys(ramOnlyKeys: Set<OnyxKey>): void {
- this.ramOnlyKeys = ramOnlyKeys;
- }
-
- /**
- * Check if a key is a RAM-only key
- */
- isRamOnlyKey(key: OnyxKey): boolean {
- return this.ramOnlyKeys.has(key);
- }
}
const instance = new OnyxCache();
diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts
index b9d8d56..d9a70a5 100644
--- a/lib/OnyxConnectionManager.ts
+++ b/lib/OnyxConnectionManager.ts
@@ -2,6 +2,7 @@ import bindAll from 'lodash/bindAll';
import * as Logger from './Logger';
import type {ConnectOptions} from './Onyx';
import OnyxUtils from './OnyxUtils';
+import keyManager from './OnyxKeyManager';
import * as Str from './Str';
import type {CollectionConnectCallback, DefaultConnectCallback, DefaultConnectOptions, OnyxKey, OnyxValue} from './types';
import cache from './OnyxCache';
@@ -129,7 +130,7 @@ class OnyxConnectionManager {
if (
reuseConnection === false ||
initWithStoredValues === false ||
- (OnyxUtils.isCollectionKey(key) && (waitForCollectionCallback === undefined || waitForCollectionCallback === false))
+ (keyManager.isCollectionKey(key) && (waitForCollectionCallback === undefined || waitForCollectionCallback === false))
) {
suffix += `,uniqueID=${Str.guid()}`;
}
diff --git a/lib/OnyxKeyManager.ts b/lib/OnyxKeyManager.ts
new file mode 100644
index 0000000..67a295b
--- /dev/null
+++ b/lib/OnyxKeyManager.ts
@@ -0,0 +1,241 @@
+import type {CollectionKey, CollectionKeyBase, DeepRecord, OnyxKey} from './types';
+import * as Str from './Str';
+
+/**
+ * Manages Onyx keys, including collection keys and RAM-only keys.
+ * Centralizes key-related state and utilities that were previously
+ * distributed across OnyxUtils and OnyxCache.
+ */
+class OnyxKeyManager {
+ /** Set of registered regular (non-collection) keys for fast lookup. Note that this is a superset of ramOnlyKeys */
+ private regularKeySet = new Set<OnyxKey>();
+
+ /** Set of registered collection keys for fast lookup */
+ private collectionKeySet = new Set<OnyxKey>();
+
+ /** Set of RAM-only keys for fast lookup */
+ private ramOnlyKeys = new Set<OnyxKey>();
+
+ /**
+ * Recursively extracts all regular (non-collection) key values from the ONYXKEYS object.
+ * Excludes the COLLECTION property since those keys are handled separately.
+ *
+ * @param obj - The object to extract values from
+ * @returns A Set containing all string values found in the object
+ */
+ private extractRegularKeys = (obj: DeepRecord<string, OnyxKey>): Set<OnyxKey> => {
+ const result = new Set<OnyxKey>();
+ for (const [key, value] of Object.entries(obj)) {
+ if (key === 'COLLECTION') {
+ continue;
+ }
+ if (typeof value === 'string') {
+ result.add(value);
+ } else if (typeof value === 'object' && value !== null) {
+ const nestedKeys = this.extractRegularKeys(value as DeepRecord<string, OnyxKey>);
+ for (const nestedKey of nestedKeys) {
+ result.add(nestedKey);
+ }
+ }
+ }
+ return result;
+ };
+
+ /**
+ * Initializes the key manager by processing the ONYXKEYS object.
+ * Extracts and stores regular keys, collection keys, and RAM-only keys.
+ *
+ * @param keys - The ONYXKEYS constants object
+ * @param ramOnlyKeys - Array of keys that should only be stored in RAM
+ */
+ init = (keys: DeepRecord<string, OnyxKey>, ramOnlyKeys: OnyxKey[] = []): void => {
+ // Extract all regular (non-collection) keys from the ONYXKEYS object
+ this.regularKeySet = this.extractRegularKeys(keys);
+
+ // Extract collection keys for fast lookup
+ this.collectionKeySet = new Set<OnyxKey>(Object.values(keys.COLLECTION ?? {}) as string[]);
+
+ // Store RAM-only keys
+ this.ramOnlyKeys = new Set(ramOnlyKeys);
+ };
+
+ /**
+ * Returns the set of registered collection keys.
+ * Used by OnyxCache for optimized storage.
+ */
+ getCollectionKeys = (): Set<OnyxKey> => {
+ return this.collectionKeySet;
+ };
+
+ /**
+ * Checks if a key is a valid registered Onyx key.
+ *
+ * A key is valid if it's:
+ * - A registered regular (non-collection) key
+ * - A registered collection base key (e.g., "report_")
+ * - A collection member key (e.g., "report_123") whose collection is registered
+ *
+ * @param key - The key to validate
+ * @returns True if the key is a valid registered OnyxKey
+ */
+ isValidKey = (key: string): boolean => {
+ // Check if it's a registered regular key
+ if (this.regularKeySet.has(key)) {
+ return true;
+ }
+
+ // Check if it's a collection base key or collection member
+ return this.getCollectionKey(key) !== null;
+ };
+
+ /**
+ * Type guard that asserts a key is a valid registered OnyxKey, throwing if it's not.
+ */
+ assertValidKey = (key: unknown): asserts key is OnyxKey => {
+ if (typeof key !== 'string') {
+ throw new Error(`Invalid ${typeof key} key provided in Onyx update. Onyx key must be of type string.`);
+ }
+ if (!this.isValidKey(key)) {
+ throw new Error(`Invalid key "${key}" provided in Onyx update. Key is not a registered OnyxKey.`);
+ }
+ };
+
+ /**
+ * Checks to see if the subscriber's supplied key
+ * is associated with a collection of keys.
+ */
+ isCollectionKey = (key: OnyxKey): key is CollectionKeyBase => {
+ return this.collectionKeySet.has(key);
+ };
+
+ /**
+ * Checks if a key is a member of a specific collection.
+ */
+ isCollectionMemberKey = <TCollectionKey extends CollectionKeyBase>(collectionKey: TCollectionKey, key: string): key is `${TCollectionKey}${string}` => {
+ return key.startsWith(collectionKey) && key.length > collectionKey.length;
+ };
+
+ /**
+ * Extracts the collection identifier of a given key.
+ *
+ * For example:
+ * - `getCollectionKey("report_123")` would return "report_"
+ * - `getCollectionKey("report_")` would return "report_"
+ * - `getCollectionKey("report_-1_something")` would return "report_"
+ * - `getCollectionKey("sharedNVP_user_-1_something")` would return "sharedNVP_user_"
+ * - `getCollectionKey("nonCollectionKey")` would return null
+ *
+ * @param key - The key to process.
+ * @returns The collection key, or null if the key is not a collection key or collection member.
+ */
+ getCollectionKey = (key: OnyxKey): string | null => {
+ // Start by finding the position of the last underscore in the string
+ let lastUnderscoreIndex = key.lastIndexOf('_');
+
+ // Iterate backwards to find the longest key that ends with '_'
+ while (lastUnderscoreIndex > 0) {
+ const possibleKey = key.slice(0, lastUnderscoreIndex + 1);
+
+ // Check if the substring is a key in the Set
+ if (this.isCollectionKey(possibleKey)) {
+ // Return the matching key
+ return possibleKey;
+ }
+
+ // Move to the next underscore to check smaller possible keys
+ lastUnderscoreIndex = key.lastIndexOf('_', lastUnderscoreIndex - 1);
+ }
+
+ return null;
+ };
+
+ /**
+ * Gets the collection key for a member key only.
+ * Unlike getCollectionKey, this returns null for collection base keys.
+ *
+ * For example:
+ * - `getMemberCollectionKey("report_123")` would return "report_"
+ * - `getMemberCollectionKey("report_")` would return null (it's a base key, not a member)
+ * - `getMemberCollectionKey("nonCollectionKey")` would return null
+ *
+ * @param key - The key to process.
+ * @returns The collection key if the key is a collection member, null otherwise.
+ */
+ getMemberCollectionKey = (key: OnyxKey): string | null => {
+ const collectionKey = this.getCollectionKey(key);
+
+ // Only return for member keys, not the collection base key itself
+ if (collectionKey && key !== collectionKey) {
+ return collectionKey;
+ }
+
+ return null;
+ };
+
+ /**
+ * Splits a collection member key into the collection key part and the ID part.
+ * @param key - The collection member key to split.
+ * @param collectionKey - The collection key of the `key` param that can be passed in advance to optimize the function.
+ * @returns A tuple where the first element is the collection part and the second element is the ID part.
+ * @throws Error if the key is not a valid collection member key.
+ */
+ splitCollectionMemberKey = <TKey extends CollectionKey, CollectionKeyType = TKey extends `${infer Prefix}_${string}` ? `${Prefix}_` : never>(
+ key: TKey,
+ collectionKey?: string,
+ ): [CollectionKeyType, string] => {
+ if (collectionKey && !this.isCollectionMemberKey(collectionKey, key)) {
+ throw new Error(`Invalid '${collectionKey}' collection key provided, it isn't compatible with '${key}' key.`);
+ }
+
+ const resolvedCollectionKey = collectionKey ?? this.getCollectionKey(key);
+ if (!resolvedCollectionKey) {
+ throw new Error(`Invalid '${key}' key provided, only collection member keys are allowed.`);
+ }
+
+ return [resolvedCollectionKey as CollectionKeyType, key.slice(resolvedCollectionKey.length)];
+ };
+
+ /**
+ * Checks if a given key is a RAM-only key, RAM-only collection key, or a RAM-only collection member.
+ *
+ * For example:
+ *
+ * For the following Onyx setup:
+ * ramOnlyKeys: ["ramOnlyKey", "ramOnlyCollection_"]
+ *
+ * - `isRamOnlyKey("ramOnlyKey")` would return true
+ * - `isRamOnlyKey("ramOnlyCollection_")` would return true
+ * - `isRamOnlyKey("ramOnlyCollection_1")` would return true
+ * - `isRamOnlyKey("someOtherKey")` would return false
+ *
+ * @param key - The key to check
+ * @returns true if key is a RAM-only key, RAM-only collection key, or a RAM-only collection member
+ */
+ isRamOnlyKey = (key: OnyxKey): boolean => {
+ // First check if the key itself is a RAM-only key
+ if (this.ramOnlyKeys.has(key)) {
+ return true;
+ }
+
+ // Then check if it's a member of a RAM-only collection
+ const collectionKey = this.getCollectionKey(key);
+ return collectionKey !== null && this.ramOnlyKeys.has(collectionKey);
+ };
+
+ /**
+ * Checks if a key matches a given pattern key.
+ *
+ * - For regular keys: returns true only if the keys match exactly
+ * - For collection keys: returns true if the key is a member of that collection
+ *
+ * @param patternKey - The pattern to match against (e.g., "user" or "report_")
+ * @param key - The key being checked (e.g., "user" or "report_123")
+ * @returns True if the key matches the pattern
+ */
+ isKeyMatch = (patternKey: OnyxKey, key: OnyxKey): boolean => {
+ return this.isCollectionKey(patternKey) ? Str.startsWith(key, patternKey) : patternKey === key;
+ };
+}
+
+const onyxKeyManager = new OnyxKeyManager();
+export default onyxKeyManager;
diff --git a/lib/OnyxMerge/index.native.ts b/lib/OnyxMerge/index.native.ts
index ec8c242..37c5cc4 100644
--- a/lib/OnyxMerge/index.native.ts
+++ b/lib/OnyxMerge/index.native.ts
@@ -1,4 +1,5 @@
import OnyxUtils from '../OnyxUtils';
+import keyManager from '../OnyxKeyManager';
import type {OnyxInput, OnyxKey, OnyxValue} from '../types';
import cache from '../OnyxCache';
import Storage from '../storage';
@@ -28,7 +29,7 @@ const applyMerge: ApplyMerge = <TKey extends OnyxKey, TValue extends OnyxInput<T
// This approach prioritizes fast UI changes without waiting for data to be stored in device storage.
const updatePromise = OnyxUtils.broadcastUpdate(key, mergedValue as OnyxValue<TKey>, hasChanged);
- const shouldSkipStorageOperations = !hasChanged || OnyxUtils.isRamOnlyKey(key);
+ const shouldSkipStorageOperations = !hasChanged || keyManager.isRamOnlyKey(key);
// If the value has not changed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead.
// If the key is marked as RAM-only, it should not be saved nor updated in the storage.
diff --git a/lib/OnyxMerge/index.ts b/lib/OnyxMerge/index.ts
index 7eac789..eb5885e 100644
--- a/lib/OnyxMerge/index.ts
+++ b/lib/OnyxMerge/index.ts
@@ -1,5 +1,6 @@
import cache from '../OnyxCache';
import OnyxUtils from '../OnyxUtils';
+import keyManager from '../OnyxKeyManager';
import Storage from '../storage';
import type {OnyxInput, OnyxKey, OnyxValue} from '../types';
import type {ApplyMerge} from './types';
@@ -20,7 +21,7 @@ const applyMerge: ApplyMerge = <TKey extends OnyxKey, TValue extends OnyxInput<T
// This approach prioritizes fast UI changes without waiting for data to be stored in device storage.
const updatePromise = OnyxUtils.broadcastUpdate(key, mergedValue as OnyxValue<TKey>, hasChanged);
- const shouldSkipStorageOperations = !hasChanged || OnyxUtils.isRamOnlyKey(key);
+ const shouldSkipStorageOperations = !hasChanged || keyManager.isRamOnlyKey(key);
// If the value has not changed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead.
// If the key is marked as RAM-only, it should not be saved nor updated in the storage.
diff --git a/lib/OnyxSnapshotCache.ts b/lib/OnyxSnapshotCache.ts
index 0d6871c..b6305d6 100644
--- a/lib/OnyxSnapshotCache.ts
+++ b/lib/OnyxSnapshotCache.ts
@@ -1,4 +1,4 @@
-import OnyxUtils from './OnyxUtils';
+import keyManager from './OnyxKeyManager';
import type {OnyxKey, OnyxValue} from './types';
import type {UseOnyxOptions, UseOnyxResult, UseOnyxSelector} from './useOnyx';
@@ -133,12 +133,10 @@ class OnyxSnapshotCache {
// Always invalidate the exact key
this.snapshotCache.delete(keyToInvalidate);
- try {
- // Check if the key is a collection member and invalidate the collection base key
- const collectionBaseKey = OnyxUtils.getCollectionKey(keyToInvalidate);
+ // Check if the key is a collection member and invalidate the collection base key
+ const collectionBaseKey = keyManager.getCollectionKey(keyToInvalidate);
+ if (collectionBaseKey) {
this.snapshotCache.delete(collectionBaseKey);
- } catch (e) {
- // do nothing - this just means the key is not a collection member
}
}
diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts
index 446e7bd..0f7b083 100644
--- a/lib/OnyxUtils.ts
+++ b/lib/OnyxUtils.ts
@@ -7,8 +7,8 @@ import type Onyx from './Onyx';
import cache, {TASK} from './OnyxCache';
import * as Str from './Str';
import Storage from './storage';
+import keyManager from './OnyxKeyManager';
import type {
- CollectionKey,
CollectionKeyBase,
ConnectOptions,
DeepRecord,
@@ -80,9 +80,6 @@ let nextMacrotaskPromise: Promise<void> | null = null;
// Holds a mapping of all the React components that want their state subscribed to a store key
let callbackToStateMapping: Record<string, CallbackToStateMapping<OnyxKey>> = {};
-// Keeps a copy of the values of the onyx collection keys as a map for faster lookups
-let onyxCollectionKeySet = new Set<OnyxKey>();
-
// Holds a mapping of the connected key to the subscriptionID for faster lookups
let onyxKeyToSubscriptionIDs = new Map();
@@ -166,21 +163,14 @@ function setSnapshotMergeKeys(keys: Set<string>): void {
}
/**
- * Sets the initial values for the Onyx store
+ * Sets the initial values for the Onyx store.
+ * Note: Key manager must be initialized before calling this function.
*
- * @param keys - `ONYXKEYS` constants object from Onyx.init()
* @param initialKeyStates - initial data to set when `init()` and `clear()` are called
* @param evictableKeys - This is an array of keys (individual or collection patterns) that when provided to Onyx are flagged as "safe" for removal.
+ * @param keys - `ONYXKEYS` constants object from Onyx.init() (used for snapshot key detection)
*/
-function initStoreValues(keys: DeepRecord<string, OnyxKey>, initialKeyStates: Partial<KeyValueMapping>, evictableKeys: OnyxKey[]): void {
- // We need the value of the collection keys later for checking if a
- // key is a collection. We store it in a map for faster lookup.
- const collectionValues = Object.values(keys.COLLECTION ?? {}) as string[];
- onyxCollectionKeySet = collectionValues.reduce((acc, val) => {
- acc.add(val);
- return acc;
- }, new Set<OnyxKey>());
-
+function initStoreValues(initialKeyStates: Partial<KeyValueMapping>, evictableKeys: OnyxKey[], keys: DeepRecord<string, OnyxKey> = {}): void {
// Set our default key states to use when initializing and clearing Onyx data
defaultKeyStates = initialKeyStates;
@@ -190,7 +180,7 @@ function initStoreValues(keys: DeepRecord<string, OnyxKey>, initialKeyStates: Pa
cache.setEvictionAllowList(evictableKeys);
// Set collection keys in cache for optimized storage
- cache.setCollectionKeys(onyxCollectionKeySet);
+ cache.initCollectionKeys(keyManager.getCollectionKeys());
if (typeof keys.COLLECTION === 'object' && typeof keys.COLLECTION.SNAPSHOT === 'string') {
snapshotKey = keys.COLLECTION.SNAPSHOT;
@@ -262,7 +252,7 @@ function get<TKey extends OnyxKey, TValue extends OnyxValue<TKey>>(key: TKey): P
.then((val) => {
if (skippableCollectionMemberIDs.size) {
try {
- const [, collectionMemberID] = splitCollectionMemberKey(key);
+ const [, collectionMemberID] = keyManager.splitCollectionMemberKey(key);
if (skippableCollectionMemberIDs.has(collectionMemberID)) {
// The key is a skippable one, so we set the value to undefined.
// eslint-disable-next-line no-param-reassign
@@ -353,7 +343,7 @@ function multiGet<TKey extends OnyxKey>(keys: CollectionKeyBase[]): Promise<Map<
for (const [key, value] of values) {
if (skippableCollectionMemberIDs.size) {
try {
- const [, collectionMemberID] = splitCollectionMemberKey(key);
+ const [, collectionMemberID] = keyManager.splitCollectionMemberKey(key);
if (skippableCollectionMemberIDs.has(collectionMemberID)) {
// The key is a skippable one, so we skip this iteration.
continue;
@@ -435,134 +425,6 @@ function getAllKeys(): Promise<Set<OnyxKey>> {
return cache.captureTask(TASK.GET_ALL_KEYS, promise) as Promise<Set<OnyxKey>>;
}
-/**
- * Returns set of all registered collection keys
- */
-function getCollectionKeys(): Set<OnyxKey> {
- return onyxCollectionKeySet;
-}
-
-/**
- * Checks to see if the subscriber's supplied key
- * is associated with a collection of keys.
- */
-function isCollectionKey(key: OnyxKey): key is CollectionKeyBase {
- return onyxCollectionKeySet.has(key);
-}
-
-function isCollectionMemberKey<TCollectionKey extends CollectionKeyBase>(collectionKey: TCollectionKey, key: string): key is `${TCollectionKey}${string}` {
- return key.startsWith(collectionKey) && key.length > collectionKey.length;
-}
-
-/**
- * Checks if a given key is a collection member key (not just a collection key).
- * @param key - The key to check
- * @returns true if the key is a collection member, false otherwise
- */
-function isCollectionMember(key: OnyxKey): boolean {
- try {
- const collectionKey = getCollectionKey(key);
- // If the key is longer than the collection key, it's a collection member
- return key.length > collectionKey.length;
- } catch (e) {
- // If getCollectionKey throws, the key is not a collection member
- return false;
- }
-}
-
-/**
- * Checks if a given key is a RAM-only key, RAM-only collection key, or a RAM-only collection member
- *
- * For example:
- *
- * For the following Onyx setup
- *
- * ramOnlyKeys: ["ramOnlyKey", "ramOnlyCollection_"]
- *
- * - `isRamOnlyKey("ramOnlyKey")` would return true
- * - `isRamOnlyKey("ramOnlyCollection_")` would return true
- * - `isRamOnlyKey("ramOnlyCollection_1")` would return true
- * - `isRamOnlyKey("someOtherKey")` would return false
- *
- * @param key - The key to check
- * @returns true if key is a RAM-only key, RAM-only collection key, or a RAM-only collection member
- */
-function isRamOnlyKey(key: OnyxKey): boolean {
- try {
- const collectionKey = getCollectionKey(key);
- // If collectionKey exists for a given key, check if it's a RAM-only key
- return cache.isRamOnlyKey(collectionKey);
- } catch {
- // If getCollectionKey throws, the key is not a collection member
- }
-
- return cache.isRamOnlyKey(key);
-}
-
-/**
- * Splits a collection member key into the collection key part and the ID part.
- * @param key - The collection member key to split.
- * @param collectionKey - The collection key of the `key` param that can be passed in advance to optimize the function.
- * @returns A tuple where the first element is the collection part and the second element is the ID part,
- * or throws an Error if the key is not a collection one.
- */
-function splitCollectionMemberKey<TKey extends CollectionKey, CollectionKeyType = TKey extends `${infer Prefix}_${string}` ? `${Prefix}_` : never>(
- key: TKey,
- collectionKey?: string,
-): [CollectionKeyType, string] {
- if (collectionKey && !isCollectionMemberKey(collectionKey, key)) {
- throw new Error(`Invalid '${collectionKey}' collection key provided, it isn't compatible with '${key}' key.`);
- }
-
- if (!collectionKey) {
- // eslint-disable-next-line no-param-reassign
- collectionKey = getCollectionKey(key);
- }
-
- return [collectionKey as CollectionKeyType, key.slice(collectionKey.length)];
-}
-
-/**
- * Checks to see if a provided key is the exact configured key of our connected subscriber
- * or if the provided key is a collection member key (in case our configured key is a "collection key")
- */
-function isKeyMatch(configKey: OnyxKey, key: OnyxKey): boolean {
- return isCollectionKey(configKey) ? Str.startsWith(key, configKey) : configKey === key;
-}
-
-/**
- * Extracts the collection identifier of a given collection member key.
- *
- * For example:
- * - `getCollectionKey("report_123")` would return "report_"
- * - `getCollectionKey("report_")` would return "report_"
- * - `getCollectionKey("report_-1_something")` would return "report_"
- * - `getCollectionKey("sharedNVP_user_-1_something")` would return "sharedNVP_user_"
- *
- * @param key - The collection key to process.
- * @returns The plain collection key or throws an Error if the key is not a collection one.
- */
-function getCollectionKey(key: CollectionKey): string {
- // Start by finding the position of the last underscore in the string
- let lastUnderscoreIndex = key.lastIndexOf('_');
-
- // Iterate backwards to find the longest key that ends with '_'
- while (lastUnderscoreIndex > 0) {
- const possibleKey = key.slice(0, lastUnderscoreIndex + 1);
-
- // Check if the substring is a key in the Set
- if (isCollectionKey(possibleKey)) {
- // Return the matching key and the rest of the string
- return possibleKey;
- }
-
- // Move to the next underscore to check smaller possible keys
- lastUnderscoreIndex = key.lastIndexOf('_', lastUnderscoreIndex - 1);
- }
-
- throw new Error(`Invalid '${key}' key provided, only collection keys are allowed.`);
-}
-
/**
* Tries to get a value from the cache. If the value is not present in cache it will return the default value or undefined.
* If the requested key is a collection, it will return an object with all the collection members.
@@ -570,7 +432,7 @@ function getCollectionKey(key: CollectionKey): string {
function tryGetCachedValue<TKey extends OnyxKey>(key: TKey): OnyxValue<OnyxKey> {
let val = cache.get(key);
- if (isCollectionKey(key)) {
+ if (keyManager.isCollectionKey(key)) {
const collectionData = cache.getCollectionData(key);
if (collectionData !== undefined) {
val = collectionData;
@@ -617,7 +479,7 @@ function getCachedCollection<TKey extends CollectionKeyBase>(collectionKey: TKey
// If we don't have collectionMemberKeys array then we have to check whether a key is a collection member key.
// Because in that case the keys will be coming from `cache.getAllKeys()` and we need to filter out the keys that
// are not part of the collection.
- if (!collectionMemberKeys && !isCollectionMemberKey(collectionKey, key)) {
+ if (!collectionMemberKeys && !keyManager.isCollectionMemberKey(collectionKey, key)) {
continue;
}
@@ -671,7 +533,7 @@ function keysChanged<TKey extends CollectionKeyBase>(
/**
* e.g. Onyx.connect({key: `${ONYXKEYS.COLLECTION.REPORT}{reportID}`, callback: ...});
*/
- const isSubscribedToCollectionMemberKey = isCollectionMemberKey(collectionKey, subscriber.key);
+ const isSubscribedToCollectionMemberKey = keyManager.isCollectionMemberKey(collectionKey, subscriber.key);
// Regular Onyx.connect() subscriber found.
if (typeof subscriber.callback === 'function') {
@@ -728,7 +590,7 @@ function keyChanged<TKey extends OnyxKey>(
): void {
// Add or remove this key from the recentlyAccessedKeys lists
if (value !== null) {
- cache.addLastAccessedKey(key, isCollectionKey(key));
+ cache.addLastAccessedKey(key, keyManager.isCollectionKey(key));
} else {
cache.removeLastAccessedKey(key);
}
@@ -739,13 +601,7 @@ function keyChanged<TKey extends OnyxKey>(
// do the same in keysChanged, because we only call that function when a collection key changes, and it doesn't happen that often.
// For performance reason, we look for the given key and later if don't find it we look for the collection key, instead of checking if it is a collection key first.
let stateMappingKeys = onyxKeyToSubscriptionIDs.get(key) ?? [];
- let collectionKey: string | undefined;
- try {
- collectionKey = getCollectionKey(key);
- } catch (e) {
- // If getCollectionKey() throws an error it means the key is not a collection key.
- collectionKey = undefined;
- }
+ const collectionKey = keyManager.getCollectionKey(key);
if (collectionKey) {
// Getting the collection key from the specific key because only collection keys were stored in the mapping.
@@ -759,7 +615,7 @@ function keyChanged<TKey extends OnyxKey>(
for (const stateMappingKey of stateMappingKeys) {
const subscriber = callbackToStateMapping[stateMappingKey];
- if (!subscriber || !isKeyMatch(subscriber.key, key) || !canUpdateSubscriber(subscriber)) {
+ if (!subscriber || !keyManager.isKeyMatch(subscriber.key, key) || !canUpdateSubscriber(subscriber)) {
continue;
}
@@ -769,7 +625,7 @@ function keyChanged<TKey extends OnyxKey>(
continue;
}
- if (isCollectionKey(subscriber.key) && subscriber.waitForCollectionCallback) {
+ if (keyManager.isCollectionKey(subscriber.key) && subscriber.waitForCollectionCallback) {
// Skip individual key changes for collection callbacks during collection updates
// to prevent duplicate callbacks - the collection update will handle this properly
if (isProcessingCollectionUpdate) {
@@ -899,7 +755,7 @@ function remove<TKey extends OnyxKey>(key: TKey, isProcessingCollectionUpdate?:
cache.drop(key);
scheduleSubscriberUpdate(key, undefined as OnyxValue<TKey>, undefined, isProcessingCollectionUpdate);
- if (isRamOnlyKey(key)) {
+ if (keyManager.isRamOnlyKey(key)) {
return Promise.resolve();
}
@@ -1111,7 +967,7 @@ function isValidNonEmptyCollectionForMerge<TKey extends CollectionKeyBase>(colle
function doAllCollectionItemsBelongToSameParent<TKey extends CollectionKeyBase>(collectionKey: TKey, collectionKeys: string[]): boolean {
let hasCollectionKeyCheckFailed = false;
for (const dataKey of collectionKeys) {
- if (isKeyMatch(collectionKey, dataKey)) {
+ if (keyManager.isKeyMatch(collectionKey, dataKey)) {
continue;
}
@@ -1154,7 +1010,7 @@ function subscribeToKey<TKey extends OnyxKey>(connectOptions: ConnectOptions<TKe
// Performance improvement
// If the mapping is connected to an onyx key that is not a collection
// we can skip the call to getAllKeys() and return an array with a single item
- if (!!mapping.key && typeof mapping.key === 'string' && !isCollectionKey(mapping.key) && cache.getAllKeys().has(mapping.key)) {
+ if (!!mapping.key && typeof mapping.key === 'string' && !keyManager.isCollectionKey(mapping.key) && cache.getAllKeys().has(mapping.key)) {
return new Set([mapping.key]);
}
return getAllKeys();
@@ -1166,14 +1022,14 @@ function subscribeToKey<TKey extends OnyxKey>(connectOptions: ConnectOptions<TKe
const matchingKeys: string[] = [];
// Performance optimization: For single key subscriptions, avoid O(n) iteration
- if (!isCollectionKey(mapping.key)) {
+ if (!keyManager.isCollectionKey(mapping.key)) {
if (keys.has(mapping.key)) {
matchingKeys.push(mapping.key);
}
} else {
// Collection case - need to iterate through all keys to find matches (O(n))
for (const key of keys) {
- if (!isKeyMatch(mapping.key, key)) {
+ if (!keyManager.isKeyMatch(mapping.key, key)) {
continue;
}
matchingKeys.push(key);
@@ -1187,7 +1043,7 @@ function subscribeToKey<TKey extends OnyxKey>(connectOptions: ConnectOptions<TKe
cache.addNullishStorageKey(mapping.key);
}
- const matchedKey = isCollectionKey(mapping.key) && mapping.waitForCollectionCallback ? mapping.key : undefined;
+ const matchedKey = keyManager.isCollectionKey(mapping.key) && mapping.waitForCollectionCallback ? mapping.key : undefined;
// Here we cannot use batching because the nullish value is expected to be set immediately for default props
// or they will be undefined.
@@ -1199,7 +1055,7 @@ function subscribeToKey<TKey extends OnyxKey>(connectOptions: ConnectOptions<TKe
// into an object and just make a single call. The latter behavior is enabled by providing a waitForCollectionCallback key
// combined with a subscription to a collection key.
if (typeof mapping.callback === 'function') {
- if (isCollectionKey(mapping.key)) {
+ if (keyManager.isCollectionKey(mapping.key)) {
if (mapping.waitForCollectionCallback) {
getCollectionDataAndSendAsObject(matchingKeys, mapping);
return;
@@ -1259,7 +1115,7 @@ function updateSnapshots<TKey extends OnyxKey>(data: Array<OnyxUpdate<TKey>>, me
for (const {key, value} of data) {
// snapshots are normal keys so we want to skip update if they are written to Onyx
- if (isCollectionMemberKey(snapshotCollectionKey, key)) {
+ if (keyManager.isCollectionMemberKey(snapshotCollectionKey, key)) {
continue;
}
@@ -1327,7 +1183,7 @@ function setWithRetry<TKey extends OnyxKey>({key, value, options}: SetParams<TKe
if (skippableCollectionMemberIDs.size) {
try {
- const [, collectionMemberID] = OnyxUtils.splitCollectionMemberKey(key);
+ const [, collectionMemberID] = keyManager.splitCollectionMemberKey(key);
if (skippableCollectionMemberIDs.has(collectionMemberID)) {
// The key is a skippable one, so we set the new value to null.
// eslint-disable-next-line no-param-reassign
@@ -1379,7 +1235,7 @@ function setWithRetry<TKey extends OnyxKey>({key, value, options}: SetParams<TKe
}
// If a key is a RAM-only key or a member of RAM-only collection, we skip the step that modifies the storage
- if (isRamOnlyKey(key)) {
+ if (keyManager.isRamOnlyKey(key)) {
OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET, key, valueWithoutNestedNullValues);
return updatePromise;
}
@@ -1406,7 +1262,7 @@ function multiSetWithRetry(data: OnyxMultiSetInput, retryAttempt?: number): Prom
if (skippableCollectionMemberIDs.size) {
newData = Object.keys(newData).reduce((result: OnyxMultiSetInput, key) => {
try {
- const [, collectionMemberID] = OnyxUtils.splitCollectionMemberKey(key);
+ const [, collectionMemberID] = keyManager.splitCollectionMemberKey(key);
// If the collection member key is a skippable one we set its value to null.
// eslint-disable-next-line no-param-reassign
result[key] = !skippableCollectionMemberIDs.has(collectionMemberID) ? newData[key] : null;
@@ -1437,7 +1293,7 @@ function multiSetWithRetry(data: OnyxMultiSetInput, retryAttempt?: number): Prom
const keyValuePairsToStore = keyValuePairsToSet.filter((keyValuePair) => {
const [key] = keyValuePair;
// Filter out the RAM-only key value pairs, as they should not be saved to storage
- return !isRamOnlyKey(key);
+ return !keyManager.isRamOnlyKey(key);
});
return Storage.multiSet(keyValuePairsToStore)
@@ -1473,7 +1329,7 @@ function setCollectionWithRetry<TKey extends CollectionKeyBase>({collectionKey,
if (skippableCollectionMemberIDs.size) {
resultCollection = resultCollectionKeys.reduce((result: OnyxInputKeyValueMapping, key) => {
try {
- const [, collectionMemberID] = OnyxUtils.splitCollectionMemberKey(key, collectionKey);
+ const [, collectionMemberID] = keyManager.splitCollectionMemberKey(key, collectionKey);
// If the collection member key is a skippable one we set its value to null.
// eslint-disable-next-line no-param-reassign
result[key] = !skippableCollectionMemberIDs.has(collectionMemberID) ? resultCollection[key] : null;
@@ -1510,7 +1366,7 @@ function setCollectionWithRetry<TKey extends CollectionKeyBase>({collectionKey,
const updatePromise = OnyxUtils.scheduleNotifyCollectionSubscribers(collectionKey, mutableCollection, previousCollection);
// RAM-only keys are not supposed to be saved to storage
- if (isRamOnlyKey(collectionKey)) {
+ if (keyManager.isRamOnlyKey(collectionKey)) {
OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET_COLLECTION, undefined, mutableCollection);
return updatePromise;
}
@@ -1557,7 +1413,7 @@ function mergeCollectionWithPatches<TKey extends CollectionKeyBase>(
if (skippableCollectionMemberIDs.size) {
resultCollection = resultCollectionKeys.reduce((result: OnyxInputKeyValueMapping, key) => {
try {
- const [, collectionMemberID] = splitCollectionMemberKey(key, collectionKey);
+ const [, collectionMemberID] = keyManager.splitCollectionMemberKey(key, collectionKey);
// If the collection member key is a skippable one we set its value to null.
// eslint-disable-next-line no-param-reassign
result[key] = !skippableCollectionMemberIDs.has(collectionMemberID) ? resultCollection[key] : null;
@@ -1626,12 +1482,12 @@ function mergeCollectionWithPatches<TKey extends CollectionKeyBase>(
// New keys will be added via multiSet while existing keys will be updated using multiMerge
// This is because setting a key that doesn't exist yet with multiMerge will throw errors
// We can skip this step for RAM-only keys as they should never be saved to storage
- if (!isRamOnlyKey(collectionKey) && keyValuePairsForExistingCollection.length > 0) {
+ if (!keyManager.isRamOnlyKey(collectionKey) && keyValuePairsForExistingCollection.length > 0) {
promises.push(Storage.multiMerge(keyValuePairsForExistingCollection));
}
// We can skip this step for RAM-only keys as they should never be saved to storage
- if (!isRamOnlyKey(collectionKey) && keyValuePairsForNewCollection.length > 0) {
+ if (!keyManager.isRamOnlyKey(collectionKey) && keyValuePairsForNewCollection.length > 0) {
promises.push(Storage.multiSet(keyValuePairsForNewCollection));
}
@@ -1685,7 +1541,7 @@ function partialSetCollection<TKey extends CollectionKeyBase>({collectionKey, co
if (skippableCollectionMemberIDs.size) {
resultCollection = resultCollectionKeys.reduce((result: OnyxInputKeyValueMapping, key) => {
try {
- const [, collectionMemberID] = splitCollectionMemberKey(key, collectionKey);
+ const [, collectionMemberID] = keyManager.splitCollectionMemberKey(key, collectionKey);
// If the collection member key is a skippable one we set its value to null.
// eslint-disable-next-line no-param-reassign
result[key] = !skippableCollectionMemberIDs.has(collectionMemberID) ? resultCollection[key] : null;
@@ -1710,7 +1566,7 @@ function partialSetCollection<TKey extends CollectionKeyBase>({collectionKey, co
const updatePromise = scheduleNotifyCollectionSubscribers(collectionKey, mutableCollection, previousCollection);
- if (isRamOnlyKey(collectionKey)) {
+ if (keyManager.isRamOnlyKey(collectionKey)) {
sendActionToDevTools(METHOD.SET_COLLECTION, undefined, mutableCollection);
return updatePromise;
}
@@ -1753,18 +1609,11 @@ const OnyxUtils = {
sendActionToDevTools,
get,
getAllKeys,
- getCollectionKeys,
- isCollectionKey,
- isCollectionMemberKey,
- isCollectionMember,
- splitCollectionMemberKey,
- isKeyMatch,
tryGetCachedValue,
getCachedCollection,
keysChanged,
keyChanged,
sendDataToConnection,
- getCollectionKey,
getCollectionDataAndSendAsObject,
scheduleSubscriberUpdate,
scheduleNotifyCollectionSubscribers,
@@ -1800,7 +1649,6 @@ const OnyxUtils = {
setWithRetry,
multiSetWithRetry,
setCollectionWithRetry,
- isRamOnlyKey,
};
GlobalSettings.addGlobalSettingsChangeListener(({enablePerformanceMetrics}) => {
@@ -1816,8 +1664,6 @@ GlobalSettings.addGlobalSettingsChangeListener(({enablePerformanceMetrics}) => {
// @ts-expect-error Reassign
getAllKeys = decorateWithMetrics(getAllKeys, 'OnyxUtils.getAllKeys');
// @ts-expect-error Reassign
- getCollectionKeys = decorateWithMetrics(getCollectionKeys, 'OnyxUtils.getCollectionKeys');
- // @ts-expect-error Reassign
keysChanged = decorateWithMetrics(keysChanged, 'OnyxUtils.keysChanged');
// @ts-expect-error Reassign
keyChanged = decorateWithMetrics(keyChanged, 'OnyxUtils.keyChanged');
diff --git a/lib/index.ts b/lib/index.ts
index bb6df0e..efccbc9 100644
--- a/lib/index.ts
+++ b/lib/index.ts
@@ -21,10 +21,11 @@ import type {
import type {FetchStatus, ResultMetadata, UseOnyxResult, UseOnyxOptions} from './useOnyx';
import type {Connection} from './OnyxConnectionManager';
import useOnyx from './useOnyx';
+import onyxKeyManager from './OnyxKeyManager';
import type {OnyxSQLiteKeyValuePair} from './storage/providers/SQLiteProvider';
export default Onyx;
-export {useOnyx};
+export {useOnyx, onyxKeyManager};
export type {
ConnectOptions,
CustomTypeOptions,
diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts
index 0b28a45..a4ca44c 100644
--- a/lib/useOnyx.ts
+++ b/lib/useOnyx.ts
@@ -5,6 +5,7 @@ import OnyxCache, {TASK} from './OnyxCache';
import type {Connection} from './OnyxConnectionManager';
import connectionManager from './OnyxConnectionManager';
import OnyxUtils from './OnyxUtils';
+import keyManager from './OnyxKeyManager';
import * as GlobalSettings from './GlobalSettings';
import type {CollectionKeyBase, OnyxKey, OnyxValue} from './types';
import usePrevious from './usePrevious';
@@ -170,10 +171,10 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
}
try {
- const previousCollectionKey = OnyxUtils.splitCollectionMemberKey(previousKey)[0];
- const collectionKey = OnyxUtils.splitCollectionMemberKey(key)[0];
+ const previousCollectionKey = keyManager.splitCollectionMemberKey(previousKey)[0];
+ const collectionKey = keyManager.splitCollectionMemberKey(key)[0];
- if (OnyxUtils.isCollectionMemberKey(previousCollectionKey, previousKey) && OnyxUtils.isCollectionMemberKey(collectionKey, key) && previousCollectionKey === collectionKey) {
+ if (keyManager.isCollectionMemberKey(previousCollectionKey, previousKey) && keyManager.isCollectionMemberKey(collectionKey, key) && previousCollectionKey === collectionKey) {
return;
}
} catch (e) {
@@ -361,7 +362,7 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
onStoreChange();
},
initWithStoredValues: options?.initWithStoredValues,
- waitForCollectionCallback: OnyxUtils.isCollectionKey(key) as true,
+ waitForCollectionCallback: keyManager.isCollectionKey(key) as true,
reuseConnection: options?.reuseConnection,
});
diff --git a/tests/perf-test/OnyxUtils.perf-test.ts b/tests/perf-test/OnyxUtils.perf-test.ts
index 87e332b..4607f66 100644
--- a/tests/perf-test/OnyxUtils.perf-test.ts
+++ b/tests/perf-test/OnyxUtils.perf-test.ts
@@ -6,6 +6,7 @@ import Onyx from '../../lib';
import StorageMock from '../../lib/storage';
import OnyxCache from '../../lib/OnyxCache';
import OnyxUtils, {clearOnyxUtilsInternals} from '../../lib/OnyxUtils';
+import keyManager from '../../lib/OnyxKeyManager';
import type GenericCollection from '../utils/GenericCollection';
import type {OnyxUpdate} from '../../lib/Onyx';
import createDeferredTask from '../../lib/createDeferredTask';
@@ -132,15 +133,9 @@ describe('OnyxUtils', () => {
});
});
- describe('getCollectionKeys', () => {
- test('one call', async () => {
- await measureFunction(() => OnyxUtils.getCollectionKeys());
- });
- });
-
describe('isCollectionKey', () => {
test('one call', async () => {
- await measureFunction(() => OnyxUtils.isCollectionKey(collectionKey));
+ await measureFunction(() => keyManager.isCollectionKey(collectionKey));
});
});
diff --git a/tests/unit/OnyxSnapshotCacheTest.ts b/tests/unit/OnyxSnapshotCacheTest.ts
index daaf5fc..680518f 100644
--- a/tests/unit/OnyxSnapshotCacheTest.ts
+++ b/tests/unit/OnyxSnapshotCacheTest.ts
@@ -1,15 +1,17 @@
import type {OnyxKey} from '../../lib';
import {OnyxSnapshotCache} from '../../lib/OnyxSnapshotCache';
-import OnyxUtils from '../../lib/OnyxUtils';
+import keyManager from '../../lib/OnyxKeyManager';
import type {UseOnyxOptions, UseOnyxResult, UseOnyxSelector} from '../../lib/useOnyx';
-// Mock OnyxUtils for testing
-jest.mock('../../lib/OnyxUtils', () => ({
- isCollectionKey: jest.fn(),
- getCollectionKey: jest.fn(),
+// Mock OnyxKeyManager for testing
+jest.mock('../../lib/OnyxKeyManager', () => ({
+ __esModule: true,
+ default: {
+ getCollectionKey: jest.fn(),
+ },
}));
-const mockedOnyxUtils = OnyxUtils as jest.Mocked<typeof OnyxUtils>;
+const mockedKeyManager = keyManager as jest.Mocked<typeof keyManager>;
// Test types
type TestData = {
@@ -157,7 +159,8 @@ describe('OnyxSnapshotCache', () => {
});
it('should invalidate non-collection keys without affecting others', () => {
- mockedOnyxUtils.isCollectionKey.mockReturnValue(false);
+ // Non-collection keys return null from getCollectionKey
+ mockedKeyManager.getCollectionKey.mockReturnValue(null);
cache.invalidateForKey('nonCollectionKey');
@@ -173,8 +176,7 @@ describe('OnyxSnapshotCache', () => {
});
it('should invalidate collection member key and its base collection only', () => {
- mockedOnyxUtils.isCollectionKey.mockReturnValue(true);
- mockedOnyxUtils.getCollectionKey.mockReturnValue('reports_');
+ mockedKeyManager.getCollectionKey.mockReturnValue('reports_');
cache.invalidateForKey('reports_123');
@@ -192,8 +194,7 @@ describe('OnyxSnapshotCache', () => {
});
it('should invalidate collection base key without cascading to members', () => {
- mockedOnyxUtils.isCollectionKey.mockReturnValue(true);
- mockedOnyxUtils.getCollectionKey.mockReturnValue('reports_');
+ mockedKeyManager.getCollectionKey.mockReturnValue('reports_');
// When base key equals the key to invalidate, it's a collection base key
cache.invalidateForKey('reports_');
@@ -213,13 +214,11 @@ describe('OnyxSnapshotCache', () => {
it('should handle multiple different collection keys independently', () => {
// Invalidate reports collection member
- mockedOnyxUtils.isCollectionKey.mockReturnValueOnce(true);
- mockedOnyxUtils.getCollectionKey.mockReturnValueOnce('reports_');
+ mockedKeyManager.getCollectionKey.mockReturnValueOnce('reports_');
cache.invalidateForKey('reports_123');
// Invalidate users collection member
- mockedOnyxUtils.isCollectionKey.mockReturnValueOnce(true);
- mockedOnyxUtils.getCollectionKey.mockReturnValueOnce('users_');
+ mockedKeyManager.getCollectionKey.mockReturnValueOnce('users_');
cache.invalidateForKey('users_789');
// Reports: member and base should be invalidated
diff --git a/tests/unit/onyxCacheTest.tsx b/tests/unit/onyxCacheTest.tsx
index 5f21542..85cfd2b 100644
--- a/tests/unit/onyxCacheTest.tsx
+++ b/tests/unit/onyxCacheTest.tsx
@@ -423,6 +423,9 @@ describe('Onyx', () => {
/** @type OnyxCache */
let cache: typeof OnyxCache;
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ let keyManager: any;
+
const ONYX_KEYS = {
TEST_KEY: 'test',
OTHER_TEST: 'otherTest',
@@ -455,6 +458,8 @@ describe('Onyx', () => {
StorageMock = require('../../lib/storage').default;
cache = require('../../lib/OnyxCache').default;
+
+ keyManager = require('../../lib/OnyxKeyManager').default;
});
it('Should keep recently accessed items in cache', () => {
@@ -676,9 +681,9 @@ describe('Onyx', () => {
keys: testKeys,
ramOnlyKeys: [testKeys.COLLECTION.RAM_ONLY_COLLECTION, testKeys.RAM_ONLY_KEY],
}).then(() => {
- expect(cache.isRamOnlyKey(testKeys.RAM_ONLY_KEY)).toBeTruthy();
- expect(cache.isRamOnlyKey(testKeys.COLLECTION.RAM_ONLY_COLLECTION)).toBeTruthy();
- expect(cache.isRamOnlyKey(testKeys.TEST_KEY)).toBeFalsy();
+ expect(keyManager.isRamOnlyKey(testKeys.RAM_ONLY_KEY)).toBeTruthy();
+ expect(keyManager.isRamOnlyKey(testKeys.COLLECTION.RAM_ONLY_COLLECTION)).toBeTruthy();
+ expect(keyManager.isRamOnlyKey(testKeys.TEST_KEY)).toBeFalsy();
});
});
});
diff --git a/tests/unit/onyxUtilsTest.ts b/tests/unit/onyxUtilsTest.ts
index 3448c6a..2dae30a 100644
--- a/tests/unit/onyxUtilsTest.ts
+++ b/tests/unit/onyxUtilsTest.ts
@@ -1,5 +1,6 @@
import Onyx from '../../lib';
import OnyxUtils from '../../lib/OnyxUtils';
+import keyManager from '../../lib/OnyxKeyManager';
import type {GenericDeepRecord} from '../types';
import utils from '../../lib/utils';
import type {Collection, OnyxCollection} from '../../lib/types';
@@ -103,30 +104,30 @@ describe('OnyxUtils', () => {
};
it.each(Object.keys(dataResult))('%s', (key) => {
- const [collectionKey, id] = OnyxUtils.splitCollectionMemberKey(key);
+ const [collectionKey, id] = keyManager.splitCollectionMemberKey(key);
expect(collectionKey).toEqual(dataResult[key][0]);
expect(id).toEqual(dataResult[key][1]);
});
});
- it('should throw error if key does not contain underscore', () => {
+ it('should throw error if key is not a collection member', () => {
expect(() => {
- OnyxUtils.splitCollectionMemberKey(ONYXKEYS.TEST_KEY);
- }).toThrowError("Invalid 'test' key provided, only collection keys are allowed.");
+ keyManager.splitCollectionMemberKey(ONYXKEYS.TEST_KEY);
+ }).toThrowError("Invalid 'test' key provided, only collection member keys are allowed.");
expect(() => {
- OnyxUtils.splitCollectionMemberKey('');
- }).toThrowError("Invalid '' key provided, only collection keys are allowed.");
+ keyManager.splitCollectionMemberKey('');
+ }).toThrowError("Invalid '' key provided, only collection member keys are allowed.");
});
it('should allow passing the collection key beforehand for performance gains', () => {
- const [collectionKey, id] = OnyxUtils.splitCollectionMemberKey(`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, ONYXKEYS.COLLECTION.TEST_KEY);
+ const [collectionKey, id] = keyManager.splitCollectionMemberKey(`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, ONYXKEYS.COLLECTION.TEST_KEY);
expect(collectionKey).toEqual(ONYXKEYS.COLLECTION.TEST_KEY);
expect(id).toEqual('id1');
});
it("should throw error if the passed collection key isn't compatible with the key", () => {
expect(() => {
- OnyxUtils.splitCollectionMemberKey(`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, ONYXKEYS.COLLECTION.TEST_LEVEL_KEY);
+ keyManager.splitCollectionMemberKey(`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, ONYXKEYS.COLLECTION.TEST_LEVEL_KEY);
}).toThrowError("Invalid 'test_level_' collection key provided, it isn't compatible with 'test_id1' key.");
});
});
@@ -332,48 +333,14 @@ describe('OnyxUtils', () => {
};
it.each(Object.keys(dataResult))('%s', (key) => {
- const collectionKey = OnyxUtils.getCollectionKey(key);
+ const collectionKey = keyManager.getCollectionKey(key);
expect(collectionKey).toEqual(dataResult[key]);
});
});
- it('should throw error if key does not contain underscore', () => {
- expect(() => {
- OnyxUtils.getCollectionKey(ONYXKEYS.TEST_KEY);
- }).toThrowError("Invalid 'test' key provided, only collection keys are allowed.");
- expect(() => {
- OnyxUtils.getCollectionKey('');
- }).toThrowError("Invalid '' key provided, only collection keys are allowed.");
- });
- });
-
- describe('isCollectionMember', () => {
- it('should return true for collection member keys', () => {
- expect(OnyxUtils.isCollectionMember('test_123')).toBe(true);
- expect(OnyxUtils.isCollectionMember('test_level_456')).toBe(true);
- expect(OnyxUtils.isCollectionMember('test_level_last_789')).toBe(true);
- expect(OnyxUtils.isCollectionMember('test_-1_something')).toBe(true);
- expect(OnyxUtils.isCollectionMember('routes_abc')).toBe(true);
- });
-
- it('should return false for collection keys themselves', () => {
- expect(OnyxUtils.isCollectionMember('test_')).toBe(false);
- expect(OnyxUtils.isCollectionMember('test_level_')).toBe(false);
- expect(OnyxUtils.isCollectionMember('test_level_last_')).toBe(false);
- expect(OnyxUtils.isCollectionMember('routes_')).toBe(false);
- });
-
- it('should return false for non-collection keys', () => {
- expect(OnyxUtils.isCollectionMember('test')).toBe(false);
- expect(OnyxUtils.isCollectionMember('someRegularKey')).toBe(false);
- expect(OnyxUtils.isCollectionMember('notACollection')).toBe(false);
- expect(OnyxUtils.isCollectionMember('')).toBe(false);
- });
-
- it('should return false for invalid keys', () => {
- expect(OnyxUtils.isCollectionMember('invalid_key_123')).toBe(false);
- expect(OnyxUtils.isCollectionMember('notregistered_')).toBe(false);
- expect(OnyxUtils.isCollectionMember('notregistered_123')).toBe(false);
+ it('should return null if key is not a collection key or collection member', () => {
+ expect(keyManager.getCollectionKey(ONYXKEYS.TEST_KEY)).toBeNull();
+ expect(keyManager.getCollectionKey('')).toBeNull();
});
});
@@ -493,27 +460,79 @@ describe('OnyxUtils', () => {
describe('isRamOnlyKey', () => {
it('should return true for RAM-only key', () => {
- expect(OnyxUtils.isRamOnlyKey(ONYXKEYS.RAM_ONLY_KEY)).toBeTruthy();
+ expect(keyManager.isRamOnlyKey(ONYXKEYS.RAM_ONLY_KEY)).toBeTruthy();
});
it('should return true for RAM-only collection', () => {
- expect(OnyxUtils.isRamOnlyKey(ONYXKEYS.COLLECTION.RAM_ONLY_COLLECTION)).toBeTruthy();
+ expect(keyManager.isRamOnlyKey(ONYXKEYS.COLLECTION.RAM_ONLY_COLLECTION)).toBeTruthy();
});
it('should return true for RAM-only collection member', () => {
- expect(OnyxUtils.isRamOnlyKey(`${ONYXKEYS.COLLECTION.RAM_ONLY_COLLECTION}1`)).toBeTruthy();
+ expect(keyManager.isRamOnlyKey(`${ONYXKEYS.COLLECTION.RAM_ONLY_COLLECTION}1`)).toBeTruthy();
});
it('should return false for a normal key', () => {
- expect(OnyxUtils.isRamOnlyKey(ONYXKEYS.TEST_KEY)).toBeFalsy();
+ expect(keyManager.isRamOnlyKey(ONYXKEYS.TEST_KEY)).toBeFalsy();
});
it('should return false for normal collection', () => {
- expect(OnyxUtils.isRamOnlyKey(ONYXKEYS.COLLECTION.TEST_KEY)).toBeFalsy();
+ expect(keyManager.isRamOnlyKey(ONYXKEYS.COLLECTION.TEST_KEY)).toBeFalsy();
});
it('should return false for normal collection member', () => {
- expect(OnyxUtils.isRamOnlyKey(`${ONYXKEYS.COLLECTION.TEST_KEY}1`)).toBeFalsy();
+ expect(keyManager.isRamOnlyKey(`${ONYXKEYS.COLLECTION.TEST_KEY}1`)).toBeFalsy();
+ });
+ });
+
+ describe('isValidKey', () => {
+ it('should return true for a registered regular key', () => {
+ expect(keyManager.isValidKey(ONYXKEYS.TEST_KEY)).toBeTruthy();
+ });
+
+ it('should return true for a registered RAM-only key', () => {
+ expect(keyManager.isValidKey(ONYXKEYS.RAM_ONLY_KEY)).toBeTruthy();
+ });
+
+ it('should return true for a registered collection base key', () => {
+ expect(keyManager.isValidKey(ONYXKEYS.COLLECTION.TEST_KEY)).toBeTruthy();
+ });
+
+ it('should return true for a collection member key', () => {
+ expect(keyManager.isValidKey(`${ONYXKEYS.COLLECTION.TEST_KEY}123`)).toBeTruthy();
+ });
+
+ it('should return true for a nested collection member key', () => {
+ expect(keyManager.isValidKey(`${ONYXKEYS.COLLECTION.TEST_LEVEL_KEY}abc`)).toBeTruthy();
+ });
+
+ it('should return false for an unregistered key', () => {
+ expect(keyManager.isValidKey('unregisteredKey')).toBeFalsy();
+ });
+
+ it('should return false for a key that looks like a collection member but has an unregistered prefix', () => {
+ expect(keyManager.isValidKey('unregistered_123')).toBeFalsy();
+ });
+ });
+
+ describe('assertValidKey', () => {
+ it('should not throw for a valid regular key', () => {
+ expect(() => keyManager.assertValidKey(ONYXKEYS.TEST_KEY)).not.toThrow();
+ });
+
+ it('should not throw for a valid collection key', () => {
+ expect(() => keyManager.assertValidKey(ONYXKEYS.COLLECTION.TEST_KEY)).not.toThrow();
+ });
+
+ it('should not throw for a valid collection member key', () => {
+ expect(() => keyManager.assertValidKey(`${ONYXKEYS.COLLECTION.TEST_KEY}123`)).not.toThrow();
+ });
+
+ it('should throw for a non-string key', () => {
+ expect(() => keyManager.assertValidKey(123)).toThrow('Invalid number key provided in Onyx update. Onyx key must be of type string.');
+ });
+
+ it('should throw for an unregistered key', () => {
+ expect(() => keyManager.assertValidKey('unregisteredKey')).toThrow('Invalid key "unregisteredKey" provided in Onyx update. Key is not a registered OnyxKey.');
});
});
});
Edit: I spent a while iterating on this, and I'm really happy with how it turned out. But it's also a very large diff, so I can open a separate follow-up PR with these changes.
| */ | ||
| function isRamOnlyKey(key: OnyxKey): boolean { | ||
| try { | ||
| const collectionKey = getCollectionKey(key); |
There was a problem hiding this comment.
Not a fan of using try/catch for flow control. Claude put it elegantly:
Problems with the current pattern:
- Hidden contract: It's not obvious from reading this code that getCollectionKey throws for non-collection keys. You have to go read its implementation to understand the flow.
- Expensive error path: If many keys aren't collection members, you're creating exception objects frequently (though JS/TS exceptions are cheaper than some languages)
- Empty catch: The silent catch with just a comment is a code smell—what if getCollectionKey throws for a different reason? You'd swallow that error too.
Its proposed solution is to change the signature of getCollectionKey to return null instead of throw when the provided key is not a collection key or collection member.
There was a problem hiding this comment.
I like this suggestion any concerns with that? @JKobrynski @fabioh8010 ?
There was a problem hiding this comment.
I dont see immediate concerns and I agree with the comment, but I would make it return undefined instead of null – from the usage I saw it would be simpler to refactor
There was a problem hiding this comment.
Agreed. Do you think we could do that in a follow-up issue? That change would also affect some existing logic so it might be safer to do it separately.
There was a problem hiding this comment.
I think we should do it in a separate PR. I agree with the change and I have commented about this in past PRs as well.
There was a problem hiding this comment.
@JKobrynski Mind creating an issue for this and link it here?
There was a problem hiding this comment.
I created an issue to remove the try/catch block from getCollectionKey, do you want me to create and issue for this too?
|
@JKobrynski @fabioh8010 Have you been able to check the failing reassure tests? |
Reviewer Checklist
Screenshots/Videosios_native.mp4web_chrome.mp4 |
|
@JKobrynski Lets prepare a bump in App 🙌 |
|
@mountiny on it! |
Details
Related Issues
$ Expensify/App#80091
Automated Tests
Unit tests covering the newly added features were added in this PR.
Manual Tests
This is a new feature and is not used in the E/App yet, so it can't be tested there.
Author Checklist
### Related Issuessection aboveTestssectiontoggleReportand notonIconClick)myBool && <MyComponent />.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)Avataris modified, I verified thatAvataris working as expected in all cases)mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop