Merge remote-tracking branch 'origin/main' into feature/onyxutils-get-synchronous

Author: fabioh8010

API-INTERNAL.md

@@ -145,6 +145,9 @@ that this internal function allows passing an additional <code>mergeReplaceNullP
 Any existing collection members not included in the new data will not be removed.
 Retries on failure.</p>
 </dd>
+<dt><a href="#getCallbackToStateMapping">getCallbackToStateMapping()</a></dt>
+<dd><p>Getter - returns the callback to state mapping, useful in test environments.</p>
+</dd>
 <dt><a href="#clearOnyxUtilsInternals">clearOnyxUtilsInternals()</a></dt>
 <dd><p>Clear internal variables used in this file, useful in test environments.</p>
 </dd>
@@ -519,6 +522,12 @@ Retries on failure.
 | params.collection | Object collection keyed by individual collection member keys and values |
 | retryAttempt | retry attempt |
 
+<a name="getCallbackToStateMapping"></a>
+
+## getCallbackToStateMapping()
+Getter - returns the callback to state mapping, useful in test environments.
+
+**Kind**: global function  
 <a name="clearOnyxUtilsInternals"></a>
 
 ## clearOnyxUtilsInternals()

lib/OnyxCache.ts

@@ -2,9 +2,19 @@ import {deepEqual} from 'fast-equals';
 import bindAll from 'lodash/bindAll';
 import type {ValueOf} from 'type-fest';
 import utils from './utils';
-import type {OnyxKey, OnyxValue} from './types';
+import type {CollectionKeyBase, KeyValueMapping, NonUndefined, OnyxCollection, OnyxKey, OnyxValue} from './types';
 import OnyxKeys from './OnyxKeys';
 
+/** Frozen object containing all collection members — safe to return by reference */
+type CollectionSnapshot = Readonly<NonUndefined<OnyxCollection<KeyValueMapping[OnyxKey]>>>;
+
+/**
+ * Stable frozen empty object used as the canonical value for empty collections.
+ * Returning the same reference avoids unnecessary re-renders in useSyncExternalStore,
+ * which relies on === equality to detect changes.
+ */
+const FROZEN_EMPTY_COLLECTION: Readonly<NonUndefined<OnyxCollection<KeyValueMapping[OnyxKey]>>> = Object.freeze({});
+
 // Task constants
 const TASK = {
     GET: 'get',
@@ -28,9 +38,6 @@ class OnyxCache {
     /** A map of cached values */
     private storageMap: Record<OnyxKey, OnyxValue<OnyxKey>>;
 
-    /** Cache of complete collection data objects for O(1) retrieval */
-    private collectionData: Record<OnyxKey, Record<OnyxKey, OnyxValue<OnyxKey>>>;
-
     /**
      * Captured pending tasks for already running storage methods
      * Using a map yields better performance on operations such a delete
@@ -43,12 +50,19 @@ class OnyxCache {
     /** List of keys that have been directly subscribed to or recently modified from least to most recent */
     private recentlyAccessedKeys = new Set<OnyxKey>();
 
+    /** Frozen collection snapshots for structural sharing */
+    private collectionSnapshots: Map<OnyxKey, CollectionSnapshot>;
+
+    /** Collections whose snapshots need rebuilding (lazy — rebuilt on next read) */
+    private dirtyCollections: Set<CollectionKeyBase>;
+
     constructor() {
         this.storageKeys = new Set();
         this.nullishStorageKeys = new Set();
         this.storageMap = {};
-        this.collectionData = {};
         this.pendingPromises = new Map();
+        this.collectionSnapshots = new Map();
+        this.dirtyCollections = new Set();
 
         // bind all public methods to prevent problems with `this`
         bindAll(
@@ -74,8 +88,8 @@ class OnyxCache {
             'addEvictableKeysToRecentlyAccessedList',
             'getKeyForEviction',
             'setCollectionKeys',
-            'getCollectionData',
             'hasValueChanged',
+            'getCollectionData',
         );
     }
 
@@ -147,24 +161,21 @@ class OnyxCache {
         this.nullishStorageKeys.delete(key);
 
         const collectionKey = OnyxKeys.getCollectionKey(key);
+        const oldValue = this.storageMap[key];
+
         if (value === null || value === undefined) {
             delete this.storageMap[key];
 
-            // Remove from collection data cache if it's a collection member
-            if (collectionKey && this.collectionData[collectionKey]) {
-                delete this.collectionData[collectionKey][key];
+            if (collectionKey && oldValue !== undefined) {
+                this.dirtyCollections.add(collectionKey);
             }
             return undefined;
         }
 
         this.storageMap[key] = value;
 
-        // Update collection data cache if this is a collection member
-        if (collectionKey) {
-            if (!this.collectionData[collectionKey]) {
-                this.collectionData[collectionKey] = {};
-            }
-            this.collectionData[collectionKey][key] = value;
+        if (collectionKey && oldValue !== value) {
+            this.dirtyCollections.add(collectionKey);
         }
 
         return value;
@@ -174,15 +185,14 @@ class OnyxCache {
     drop(key: OnyxKey): void {
         delete this.storageMap[key];
 
-        // Remove from collection data cache if this is a collection member
         const collectionKey = OnyxKeys.getCollectionKey(key);
-        if (collectionKey && this.collectionData[collectionKey]) {
-            delete this.collectionData[collectionKey][key];
+        if (collectionKey) {
+            this.dirtyCollections.add(collectionKey);
         }
 
-        // If this is a collection key, clear its data
+        // If this is a collection key, clear its snapshot
         if (OnyxKeys.isCollectionKey(key)) {
-            delete this.collectionData[key];
+            this.collectionSnapshots.delete(key);
         }
 
         this.storageKeys.delete(key);
@@ -198,37 +208,54 @@ class OnyxCache {
             throw new Error('data passed to cache.merge() must be an Object of onyx key/value pairs');
         }
 
-        this.storageMap = {
-            ...utils.fastMerge(this.storageMap, data, {
-                shouldRemoveNestedNulls: true,
-                objectRemovalMode: 'replace',
-            }).result,
-        };
+        const affectedCollections = new Set<OnyxKey>();
 
         for (const [key, value] of Object.entries(data)) {
             this.addKey(key);
 
             const collectionKey = OnyxKeys.getCollectionKey(key);
 
-            if (value === null || value === undefined) {
+            if (value === undefined) {
+                this.addNullishStorageKey(key);
+                // undefined means "no change" — skip storageMap modification
+                continue;
+            }
+
+            if (value === null) {
                 this.addNullishStorageKey(key);
+                delete this.storageMap[key];
 
-                // Remove from collection data cache if it's a collection member
-                if (collectionKey && this.collectionData[collectionKey]) {
-                    delete this.collectionData[collectionKey][key];
+                if (collectionKey) {
+                    affectedCollections.add(collectionKey);
                 }
             } else {
                 this.nullishStorageKeys.delete(key);
 
-                // Update collection data cache if this is a collection member
+                // Per-key merge instead of spreading the entire storageMap
+                const existing = this.storageMap[key];
+                const merged = utils.fastMerge(existing, value, {
+                    shouldRemoveNestedNulls: true,
+                    objectRemovalMode: 'replace',
+                }).result;
+
+                // fastMerge is reference-stable: returns the original target when
+                // nothing changed, so a simple === check detects no-ops.
+                if (merged === existing) {
+                    continue;
+                }
+
+                this.storageMap[key] = merged;
+
                 if (collectionKey) {
-                    if (!this.collectionData[collectionKey]) {
-                        this.collectionData[collectionKey] = {};
-                    }
-                    this.collectionData[collectionKey][key] = this.storageMap[key];
+                    affectedCollections.add(collectionKey);
                 }
             }
         }
+
+        // Mark affected collections as dirty — snapshots will be lazily rebuilt on next read
+        for (const collectionKey of affectedCollections) {
+            this.dirtyCollections.add(collectionKey);
+        }
     }
 
     /**
@@ -264,9 +291,12 @@ class OnyxCache {
         return returnPromise;
     }
 
-    /** Check if the value has changed */
+    /** Check if the value has changed. Uses reference equality as a fast path, falls back to deep equality. */
     hasValueChanged(key: OnyxKey, value: OnyxValue<OnyxKey>): boolean {
-        const currentValue = this.get(key);
+        const currentValue = this.storageMap[key];
+        if (currentValue === value) {
+            return false;
+        }
         return !deepEqual(currentValue, value);
     }
 
@@ -344,26 +374,103 @@ class OnyxCache {
     setCollectionKeys(collectionKeys: Set<OnyxKey>): void {
         OnyxKeys.setCollectionKeys(collectionKeys);
 
-        // Initialize collection data for existing collection keys
+        // Initialize frozen snapshots for collection keys
         for (const collectionKey of collectionKeys) {
-            if (this.collectionData[collectionKey]) {
+            if (!this.collectionSnapshots.has(collectionKey)) {
+                this.collectionSnapshots.set(collectionKey, Object.freeze({}));
+            }
+        }
+    }
+
+    /**
+     * Rebuilds the frozen collection snapshot from current storageMap references.
+     * Uses the indexed collection->members map for O(collectionMembers) instead of O(totalKeys).
+     * Returns the previous snapshot reference when all member references are identical,
+     * preventing unnecessary re-renders in useSyncExternalStore.
+     *
+     * @param collectionKey - The collection key to rebuild
+     */
+    private rebuildCollectionSnapshot(collectionKey: OnyxKey): void {
+        const previousSnapshot = this.collectionSnapshots.get(collectionKey);
+
+        const members: NonUndefined<OnyxCollection<KeyValueMapping[OnyxKey]>> = {};
+        let hasMemberChanges = false;
+
+        // Use the indexed forward lookup for O(collectionMembers) iteration.
+        // Falls back to scanning all storageKeys if the index isn't populated yet.
+        const memberKeys = OnyxKeys.getMembersOfCollection(collectionKey);
+        const keysToScan = memberKeys ?? this.storageKeys;
+        const needsPrefixCheck = !memberKeys;
+
+        for (const key of keysToScan) {
+            // When using the fallback path (scanning all storageKeys instead of the indexed
+            // forward lookup), skip keys that don't belong to this collection.
+            if (needsPrefixCheck && OnyxKeys.getCollectionKey(key) !== collectionKey) {
                 continue;
             }
-            this.collectionData[collectionKey] = {};
+            const val = this.storageMap[key];
+            // Skip null/undefined values — they represent deleted or unset keys
+            // and should not be included in the frozen collection snapshot.
+            if (val !== undefined && val !== null) {
+                members[key] = val;
+
+                // Check if this member's reference changed from the old snapshot
+                if (!hasMemberChanges && (!previousSnapshot || previousSnapshot[key] !== val)) {
+                    hasMemberChanges = true;
+                }
+            }
         }
+
+        // Check if any members were removed from the previous snapshot.
+        // We can't rely on count comparison alone — if one key is removed and another added,
+        // the counts match but the snapshot content is different.
+        if (!hasMemberChanges && previousSnapshot) {
+            // eslint-disable-next-line no-restricted-syntax
+            for (const key in previousSnapshot) {
+                if (!(key in members)) {
+                    hasMemberChanges = true;
+                    break;
+                }
+            }
+        }
+
+        // If nothing actually changed, reuse the old snapshot reference.
+        // This is critical: useSyncExternalStore uses === to detect changes,
+        // so returning the same reference prevents unnecessary re-renders.
+        if (!hasMemberChanges && previousSnapshot) {
+            return;
+        }
+
+        Object.freeze(members);
+
+        this.collectionSnapshots.set(collectionKey, members);
     }
 
     /**
-     * Get all data for a collection key
+     * Get all data for a collection key.
+     * Returns a frozen snapshot with structural sharing — safe to return by reference.
+     * Lazily rebuilds the snapshot if the collection was modified since the last read.
      */
     getCollectionData(collectionKey: OnyxKey): Record<OnyxKey, OnyxValue<OnyxKey>> | undefined {
-        const cachedCollection = this.collectionData[collectionKey];
-        if (!cachedCollection || Object.keys(cachedCollection).length === 0) {
+        if (this.dirtyCollections.has(collectionKey)) {
+            this.rebuildCollectionSnapshot(collectionKey);
+            this.dirtyCollections.delete(collectionKey);
+        }
+
+        const snapshot = this.collectionSnapshots.get(collectionKey);
+        if (utils.isEmptyObject(snapshot)) {
+            // We check storageKeys.size (not collection-specific keys) to distinguish
+            // "init complete, this collection is genuinely empty" from "init not done yet."
+            // During init, setAllKeys loads ALL keys at once — so if any key exists,
+            // the full storage picture is loaded and an empty collection is truly empty.
+            // Returning undefined before init prevents subscribers from seeing a false empty state.
+            if (this.storageKeys.size > 0) {
+                return FROZEN_EMPTY_COLLECTION;
+            }
             return undefined;
         }
 
-        // Return a shallow copy to ensure React detects changes when items are added/removed
-        return {...cachedCollection};
+        return snapshot;
     }
 }
 

lib/OnyxUtils.ts

@@ -57,8 +57,6 @@ const IDB_STORAGE_ERRORS = [
 // SQLite errors that indicate storage capacity issues where eviction can help
 const SQLITE_STORAGE_ERRORS = [
     'database or disk is full', // Device storage is full
-    'disk I/O error', // File system I/O failure, often due to insufficient space or corrupted storage
-    'out of memory', // Insufficient RAM or storage space to complete the operation
 ] as const;
 
 const STORAGE_ERRORS = [...IDB_STORAGE_ERRORS, ...SQLITE_STORAGE_ERRORS];
@@ -903,6 +901,24 @@ function subscribeToKey<TKey extends OnyxKey>(connectOptions: ConnectOptions<TKe
     callbackToStateMapping[subscriptionID] = mapping as CallbackToStateMapping<OnyxKey>;
     callbackToStateMapping[subscriptionID].subscriptionID = subscriptionID;
 
+    // If the subscriber is attempting to connect to a collection member whose ID is skippable (e.g. "undefined", "null", etc.)
+    // we suppress wiring the subscription fully to avoid unnecessary callback emissions such as for "report_undefined".
+    // We still return a valid subscriptionID so callers can disconnect safely.
+    try {
+        const skippableIDs = getSkippableCollectionMemberIDs();
+        if (skippableIDs.size) {
+            const [, collectionMemberID] = OnyxKeys.splitCollectionMemberKey(mapping.key);
+            if (skippableIDs.has(collectionMemberID)) {
+                // Clean up the provisional mapping to avoid retaining unused subscribers.
+                cache.addNullishStorageKey(mapping.key);
+                delete callbackToStateMapping[subscriptionID];
+                return subscriptionID;
+            }
+        }
+    } catch (e) {
+        // Not a collection member key, proceed as usual.
+    }
+
     // When keyChanged is called, a key is passed and the method looks through all the Subscribers in callbackToStateMapping for the matching key to get the subscriptionID
     // to avoid having to loop through all the Subscribers all the time (even when just one connection belongs to one key),
     // We create a mapping from key to lists of subscriptionIDs to access the specific list of subscriptionIDs.
@@ -1496,6 +1512,13 @@ function logKeyRemoved(onyxMethod: Extract<OnyxMethod, 'set' | 'merge'>, key: On
     Logger.logInfo(`${onyxMethod} called for key: ${key} => null passed, so key was removed`);
 }
 
+/**
+ * Getter - returns the callback to state mapping, useful in test environments.
+ */
+function getCallbackToStateMapping(): Record<number, CallbackToStateMapping<OnyxKey>> {
+    return callbackToStateMapping;
+}
+
 /**
  * Clear internal variables used in this file, useful in test environments.
  */
@@ -1555,6 +1578,7 @@ const OnyxUtils = {
     setWithRetry,
     multiSetWithRetry,
     setCollectionWithRetry,
+    getCallbackToStateMapping,
 };
 
 export type {OnyxMethod};