From c71c1db224b9b115594666336e2c5e1a022d02cc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A1bio=20Henriques?= <contact@fabiohenriques.dev>
Date: Tue, 29 Oct 2024 19:28:30 +0000
Subject: [PATCH] Improve comments

---
 lib/OnyxConnectionManager.ts | 18 +++++++++++++-----
 lib/useOnyx.ts               | 13 ++++++++-----
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts
index 6e04bae39..bf50a487d 100644
--- a/lib/OnyxConnectionManager.ts
+++ b/lib/OnyxConnectionManager.ts
@@ -74,7 +74,17 @@ class OnyxConnectionManager {
     private lastCallbackID: number;
 
     /**
-     * Stores the last generated session ID for the connection manager.
+     * Stores the last generated session ID for the connection manager. The current session ID
+     * is appended to the connection IDs and it's used to create new different connections for the same key
+     * when `refreshSessionID()` is called.
+     *
+     * When calling `Onyx.clear()` after a logout operation some connections might remain active as they
+     * aren't tied to the React's lifecycle e.g. `Onyx.connect()` usage, causing infinite loading state issues to new `useOnyx()` subscribers
+     * that are connecting to the same key as we didn't populate the cache again because we are still reusing such connections.
+     *
+     * To elimitate this problem, the session ID must be refreshed during the `Onyx.clear()` call (by using `refreshSessionID()`)
+     * in order to create fresh connections when new subscribers connect to the same keys again, allowing them
+     * to use the cache system correctly and avoid the mentioned issues in `useOnyx()`.
      */
     private sessionID: string;
 
@@ -97,7 +107,7 @@ class OnyxConnectionManager {
         const {key, initWithStoredValues, reuseConnection, waitForCollectionCallback} = connectOptions;
 
         // The current session ID is appended to the connection ID so we can have different connections
-        // after an Onyx.clear() operation.
+        // after an `Onyx.clear()` operation.
         let suffix = `,sessionID=${this.sessionID}`;
 
         // We will generate a unique ID in any of the following situations:
@@ -239,9 +249,7 @@ class OnyxConnectionManager {
     }
 
     /**
-     * Refreshes the connection manager's session ID. Used in Onyx.clear() in order to
-     * create new connections after Onyx is cleared, instead of reusing existing connections
-     * that can produce unexpected behavior in Onyx subscribers.
+     * Refreshes the connection manager's session ID.
      */
     refreshSessionID(): void {
         this.sessionID = Str.guid();
diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts
index f560802e7..fa460dd7a 100644
--- a/lib/useOnyx.ts
+++ b/lib/useOnyx.ts
@@ -236,11 +236,14 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(key: TKey
             areValuesEqual = shallowEqual(previousValueRef.current ?? undefined, newValueRef.current);
         }
 
-        // If the previously cached value is different from the new value, we update both cached value
-        // and the result to be returned by the hook.
-        // If the cache was set for the first time or we have a pending Onyx.clear() task, we also update the cached value and the result.
-        const isCacheSetFirstTime = previousValueRef.current === null && (hasCacheForKey || OnyxCache.hasPendingTask(TASK.CLEAR));
-        if (isCacheSetFirstTime || !areValuesEqual) {
+        // We updated the cached value and the result in the following conditions:
+        // We will update the cached value and the result in any of the following situations:
+        // - The previously cached value is different from the new value.
+        // - The previously cached value is `null` (not set from cache yet) and we have cache for this key
+        //   OR we have a pending `Onyx.clear()` task (if `Onyx.clear()` is running cache might not be available anymore
+        //   so we update the cached value/result right away in order to prevent infinite loading state issues).
+        const shouldUpdateResult = !areValuesEqual || (previousValueRef.current === null && (hasCacheForKey || OnyxCache.hasPendingTask(TASK.CLEAR)));
+        if (shouldUpdateResult) {
             previousValueRef.current = newValueRef.current;
 
             // If the new value is `null` we default it to `undefined` to ensure the consumer gets a consistent result from the hook.