Merge remote-tracking branch 'upstream/main' into JKobrynski/feat/80091-prevent-storage-reads-for-ram-only-keys

Author: JKobrynski

API-INTERNAL.md

@@ -20,9 +20,15 @@
 <dt><a href="#getSkippableCollectionMemberIDs">getSkippableCollectionMemberIDs()</a></dt>
 <dd><p>Getter - returns the skippable collection member IDs.</p>
 </dd>
+<dt><a href="#getSnapshotMergeKeys">getSnapshotMergeKeys()</a></dt>
+<dd><p>Getter - returns the snapshot merge keys allowlist.</p>
+</dd>
 <dt><a href="#setSkippableCollectionMemberIDs">setSkippableCollectionMemberIDs()</a></dt>
 <dd><p>Setter - sets the skippable collection member IDs.</p>
 </dd>
+<dt><a href="#setSnapshotMergeKeys">setSnapshotMergeKeys()</a></dt>
+<dd><p>Setter - sets the snapshot merge keys allowlist.</p>
+</dd>
 <dt><a href="#initStoreValues">initStoreValues(keys, initialKeyStates, evictableKeys)</a></dt>
 <dd><p>Sets the initial values for the Onyx store</p>
 </dd>
@@ -222,12 +228,24 @@ Getter - returns the deffered init task.
 ## getSkippableCollectionMemberIDs()
 Getter - returns the skippable collection member IDs.
 
+**Kind**: global function  
+<a name="getSnapshotMergeKeys"></a>
+
+## getSnapshotMergeKeys()
+Getter - returns the snapshot merge keys allowlist.
+
 **Kind**: global function  
 <a name="setSkippableCollectionMemberIDs"></a>
 
 ## setSkippableCollectionMemberIDs()
 Setter - sets the skippable collection member IDs.
 
+**Kind**: global function  
+<a name="setSnapshotMergeKeys"></a>
+
+## setSnapshotMergeKeys()
+Setter - sets the snapshot merge keys allowlist.
+
 **Kind**: global function  
 <a name="initStoreValues"></a>
 

README.md

@@ -206,21 +206,6 @@ export default App;
 
 It's also beneficial to use a [selector](https://github.com/Expensify/react-native-onyx/blob/main/API.md#connectmapping--number) with the mapping in case you need to grab a single item in a collection (like a single report action).
 
-### useOnyx()'s `canBeMissing` option
-
-You must pass the `canBeMissing` configuration flag to `useOnyx` if you want the hook to log an alert when data is missing from Onyx store. Regarding usage in `Expensify/App` repo, if the component calling this is the one loading the data by calling an action, then you should set this to `true`. If the component calling this does not load the data then you should set it to `false`, which means that if the data is not there, it will log an alert, as it means we are using data that no one loaded and that's most probably a bug.
-
-```javascript
-const Component = ({reportID}) => {
-    // This hook will log an alert (via `Logger.logAlert()`) if `report` is `undefined`.
-    const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {canBeMissing: false});
-    
-    // rest of the component's code.
-};
-
-export default Component;
-```
-
 ## Collections
 
 Collections allow keys with similar value types to be subscribed together by subscribing to the collection key. To define one, it must be included in the `ONYXKEYS.COLLECTION` object and it must be suffixed with an underscore. Member keys should use a unique identifier or index after the collection key prefix (e.g. `report_42`).

lib/Onyx.ts

@@ -352,13 +352,7 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise<void> {
                         if (newValue !== oldValue) {
                             cache.set(key, newValue);
 
-                            let collectionKey: string | undefined;
-                            try {
-                                collectionKey = OnyxUtils.getCollectionKey(key);
-                            } catch (e) {
-                                // If getCollectionKey() throws an error it means the key is not a collection key.
-                                collectionKey = undefined;
-                            }
+                            const collectionKey = OnyxUtils.getCollectionKey(key);
 
                             if (collectionKey) {
                                 if (!keyValuesToResetAsCollection[collectionKey]) {

lib/OnyxCache.ts

@@ -458,13 +458,13 @@ class OnyxCache {
     /**
      * Get the collection key for a given member key
      */
-    getCollectionKey(key: OnyxKey): OnyxKey | null {
+    getCollectionKey(key: OnyxKey): OnyxKey | undefined {
         for (const collectionKey of this.collectionKeys) {
             if (key.startsWith(collectionKey) && key.length > collectionKey.length) {
                 return collectionKey;
             }
         }
-        return null;
+        return undefined;
     }
 
     /**

lib/OnyxSnapshotCache.ts

@@ -60,19 +60,17 @@ class OnyxSnapshotCache {
      * - `selector`: Different selectors produce different results, so each selector needs its own cache entry
      * - `initWithStoredValues`: This flag changes the initial loading behavior and affects the returned fetch status
      * - `allowStaleData`: Controls whether stale data can be returned during pending merges, affecting result timing
-     * - `canBeMissing`: Determines logging behavior for missing data, but doesn't affect the actual data returned
      *
      * Other options like `canEvict`, `reuseConnection`, and `allowDynamicKey` don't affect the data transformation
      * or timing behavior of getSnapshot, so they're excluded from the cache key for better cache hit rates.
      */
-    registerConsumer<TKey extends OnyxKey, TReturnValue>(options: Pick<UseOnyxOptions<TKey, TReturnValue>, 'selector' | 'initWithStoredValues' | 'allowStaleData' | 'canBeMissing'>): string {
+    registerConsumer<TKey extends OnyxKey, TReturnValue>(options: Pick<UseOnyxOptions<TKey, TReturnValue>, 'selector' | 'initWithStoredValues' | 'allowStaleData'>): string {
         const selectorID = options?.selector ? this.getSelectorID(options.selector) : 'no_selector';
 
         // Create options hash without expensive JSON.stringify
         const initWithStoredValues = options?.initWithStoredValues ?? true;
         const allowStaleData = options?.allowStaleData ?? false;
-        const canBeMissing = options?.canBeMissing ?? true;
-        const cacheKey = `${selectorID}_${initWithStoredValues}_${allowStaleData}_${canBeMissing}`;
+        const cacheKey = `${selectorID}_${initWithStoredValues}_${allowStaleData}`;
 
         // Increment reference count for this cache key
         const currentCount = this.cacheKeyRefCounts.get(cacheKey) || 0;
@@ -133,12 +131,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 = OnyxUtils.getCollectionKey(keyToInvalidate);
+        if (collectionBaseKey) {
             this.snapshotCache.delete(collectionBaseKey);
-        } catch (e) {
-            // do nothing - this just means the key is not a collection member
         }
     }
 

lib/OnyxUtils.ts

@@ -495,14 +495,9 @@ function isCollectionMemberKey<TCollectionKey extends CollectionKeyBase>(collect
  * @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;
-    }
+    const collectionKey = getCollectionKey(key);
+    // If the key is longer than the collection key, it's a collection member
+    return !!collectionKey && key.length > collectionKey.length;
 }
 
 /**
@@ -523,12 +518,9 @@ function isCollectionMember(key: OnyxKey): boolean {
  * @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
+    const collectionKey = getCollectionKey(key);
+    if (collectionKey) {
         return cache.isRamOnlyKey(collectionKey);
-    } catch {
-        // If getCollectionKey throws, the key is not a collection member
     }
 
     return cache.isRamOnlyKey(key);
@@ -550,8 +542,12 @@ function splitCollectionMemberKey<TKey extends CollectionKey, CollectionKeyType
     }
 
     if (!collectionKey) {
+        const resolvedKey = getCollectionKey(key);
+        if (!resolvedKey) {
+            throw new Error(`Invalid '${key}' key provided, only collection keys are allowed.`);
+        }
         // eslint-disable-next-line no-param-reassign
-        collectionKey = getCollectionKey(key);
+        collectionKey = resolvedKey;
     }
 
     return [collectionKey as CollectionKeyType, key.slice(collectionKey.length)];
@@ -575,9 +571,9 @@ function isKeyMatch(configKey: OnyxKey, key: OnyxKey): boolean {
  * - `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.
+ * @returns The plain collection key or undefined if the key is not a collection one.
  */
-function getCollectionKey(key: CollectionKey): string {
+function getCollectionKey(key: CollectionKey): string | undefined {
     // Start by finding the position of the last underscore in the string
     let lastUnderscoreIndex = key.lastIndexOf('_');
 
@@ -595,7 +591,7 @@ function getCollectionKey(key: CollectionKey): string {
         lastUnderscoreIndex = key.lastIndexOf('_', lastUnderscoreIndex - 1);
     }
 
-    throw new Error(`Invalid '${key}' key provided, only collection keys are allowed.`);
+    return undefined;
 }
 
 /**
@@ -774,13 +770,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 = getCollectionKey(key);
 
     if (collectionKey) {
         // Getting the collection key from the specific key because only collection keys were stored in the mapping.

lib/useOnyx.ts

@@ -9,7 +9,6 @@ import * as GlobalSettings from './GlobalSettings';
 import type {CollectionKeyBase, OnyxKey, OnyxValue} from './types';
 import usePrevious from './usePrevious';
 import decorateWithMetrics from './metrics';
-import * as Logger from './Logger';
 import onyxSnapshotCache from './OnyxSnapshotCache';
 import useLiveRef from './useLiveRef';
 
@@ -43,14 +42,6 @@ type UseOnyxOptions<TKey extends OnyxKey, TReturnValue> = {
      */
     allowDynamicKey?: boolean;
 
-    /**
-     * If the component calling this is the one loading the data by calling an action, then you should set this to `true`.
-     *
-     * If the component calling this does not load the data then you should set it to `false`, which means that if the data
-     * is not there, it will log an alert, as it means we are using data that no one loaded and that's most probably a bug.
-     */
-    canBeMissing?: boolean;
-
     /**
      * This will be used to subscribe to a subset of an Onyx key's data.
      * Using this setting on `useOnyx` can have very positive performance benefits because the component will only re-render
@@ -156,9 +147,8 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
                 selector: options?.selector,
                 initWithStoredValues: options?.initWithStoredValues,
                 allowStaleData: options?.allowStaleData,
-                canBeMissing: options?.canBeMissing,
             }),
-        [options?.selector, options?.initWithStoredValues, options?.allowStaleData, options?.canBeMissing],
+        [options?.selector, options?.initWithStoredValues, options?.allowStaleData],
     );
 
     useEffect(() => () => onyxSnapshotCache.deregisterConsumer(key, cacheKey), [key, cacheKey]);
@@ -242,8 +232,6 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
             }
         }
 
-        let isOnyxValueDefined = true;
-
         // We return the initial result right away during the first connection if `initWithStoredValues` is set to `false`.
         if (isFirstConnectionRef.current && options?.initWithStoredValues === false) {
             const result = resultRef.current;
@@ -262,10 +250,6 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
             const selectedValue = memoizedSelector ? memoizedSelector(value) : value;
             newValueRef.current = (selectedValue ?? undefined) as TReturnValue | undefined;
 
-            // This flag is `false` when the original Onyx value (without selector) is not defined yet.
-            // It will be used later to check if we need to log an alert that the value is missing.
-            isOnyxValueDefined = value !== null && value !== undefined;
-
             // We set this flag to `false` again since we don't want to get the newest cached value every time `getSnapshot()` is executed,
             // and only when `Onyx.connect()` callback is fired.
             shouldGetCachedValueRef.current = false;
@@ -317,21 +301,14 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
                     sourceValue: sourceValueRef.current,
                 },
             ];
-
-            // If `canBeMissing` is set to `false` and the Onyx value of that key is not defined,
-            // we log an alert so it can be acknowledged by the consumer. Additionally, we won't log alerts
-            // if there's a `Onyx.clear()` task in progress.
-            if (options?.canBeMissing === false && newFetchStatus === 'loaded' && !isOnyxValueDefined && !OnyxCache.hasPendingTask(TASK.CLEAR)) {
-                Logger.logAlert(`useOnyx returned no data for key with canBeMissing set to false for key ${key}`, {showAlert: true});
-            }
         }
 
         if (newFetchStatus !== 'loading') {
             onyxSnapshotCache.setCachedResult<UseOnyxResult<TReturnValue>>(key, cacheKey, resultRef.current);
         }
 
         return resultRef.current;
-    }, [options?.initWithStoredValues, options?.allowStaleData, options?.canBeMissing, key, memoizedSelector, cacheKey, previousKey]);
+    }, [options?.initWithStoredValues, options?.allowStaleData, key, memoizedSelector, cacheKey, previousKey]);
 
     const subscribe = useCallback(
         (onStoreChange: () => void) => {