From aace5a95bddcdbc771a2c0c6dbccd985bc8f6c41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 19 Mar 2026 16:59:31 +0000 Subject: [PATCH 1/7] perf: frozen collection snapshots with lazy rebuild and per-key merge Replace mutable collectionData with frozen collectionSnapshots and dirty tracking. Collection snapshots are lazily rebuilt on read, returning stable frozen references for structural sharing. The merge() method now operates per-key instead of spreading the entire storageMap, and hasValueChanged() uses reference equality as a fast path. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/OnyxCache.ts | 204 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 153 insertions(+), 51 deletions(-) diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index b7526a1c6..7fdb3a64c 100644 --- a/lib/OnyxCache.ts +++ b/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>>; + +/** + * 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>> = Object.freeze({}); + // Task constants const TASK = { GET: 'get', @@ -31,9 +41,6 @@ class OnyxCache { /** A map of cached values */ private storageMap: Record>; - /** Cache of complete collection data objects for O(1) retrieval */ - private collectionData: Record>>; - /** * Captured pending tasks for already running storage methods * Using a map yields better performance on operations such a delete @@ -52,13 +59,20 @@ class OnyxCache { /** List of keys that have been directly subscribed to or recently modified from least to most recent */ private recentlyAccessedKeys = new Set(); + /** Frozen collection snapshots for structural sharing */ + private collectionSnapshots: Map; + + /** Collections whose snapshots need rebuilding (lazy — rebuilt on next read) */ + private dirtyCollections: Set; + constructor() { this.storageKeys = new Set(); this.nullishStorageKeys = new Set(); this.recentKeys = 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( @@ -88,8 +102,8 @@ class OnyxCache { 'addEvictableKeysToRecentlyAccessedList', 'getKeyForEviction', 'setCollectionKeys', - 'getCollectionData', 'hasValueChanged', + 'getCollectionData', ); } @@ -168,24 +182,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; @@ -195,15 +206,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); @@ -220,12 +230,7 @@ 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(); for (const [key, value] of Object.entries(data)) { this.addKey(key); @@ -233,25 +238,47 @@ class OnyxCache { 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); + } } /** @@ -317,16 +344,22 @@ class OnyxCache { iterResult = iterator.next(); } + const affectedCollections = new Set(); + for (const key of keysToRemove) { delete this.storageMap[key]; - // Remove from collection data cache if this is a collection member + // Track affected collections for snapshot rebuild const collectionKey = OnyxKeys.getCollectionKey(key); - if (collectionKey && this.collectionData[collectionKey]) { - delete this.collectionData[collectionKey][key]; + if (collectionKey) { + affectedCollections.add(collectionKey); } this.recentKeys.delete(key); } + + for (const collectionKey of affectedCollections) { + this.dirtyCollections.add(collectionKey); + } } /** Set the recent keys list size */ @@ -334,9 +367,12 @@ class OnyxCache { this.maxRecentKeysSize = limit; } - /** 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): boolean { - const currentValue = this.get(key, false); + const currentValue = this.storageMap[key]; + if (currentValue === value) { + return false; + } return !deepEqual(currentValue, value); } @@ -425,33 +461,99 @@ class OnyxCache { setCollectionKeys(collectionKeys: Set): 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]) { - continue; + if (!this.collectionSnapshots.has(collectionKey)) { + this.collectionSnapshots.set(collectionKey, Object.freeze({})); } - this.collectionData[collectionKey] = {}; } - // Register existing storageKeys with OnyxKeys + // Pre-populate the reverse lookup map for any existing keys for (const key of this.storageKeys) { OnyxKeys.registerMemberKey(key); } } /** - * Get all data for a collection key + * 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 oldSnapshot = this.collectionSnapshots.get(collectionKey); + + const members: NonUndefined> = {}; + let hasChanges = false; + let newMemberCount = 0; + + // 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) { + if (needsPrefixCheck && !OnyxKeys.isCollectionMemberKey(collectionKey, key)) { + continue; + } + const val = this.storageMap[key]; + if (val !== undefined && val !== null) { + members[key] = val; + newMemberCount++; + + // Check if this member's reference changed from the old snapshot + if (!hasChanges && (!oldSnapshot || oldSnapshot[key] !== val)) { + hasChanges = true; + } + } + } + + // Check if any members were removed (old snapshot had more keys) + if (!hasChanges && oldSnapshot) { + const oldMemberCount = Object.keys(oldSnapshot).length; + if (oldMemberCount !== newMemberCount) { + hasChanges = true; + } + } + + // 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 (!hasChanges && oldSnapshot) { + return; + } + + Object.freeze(members); + + this.collectionSnapshots.set(collectionKey, members); + } + + /** + * 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> | 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 (!snapshot || Object.keys(snapshot).length === 0) { + // If we know we have storage keys loaded, return a stable empty reference + // to avoid new {} allocations that break useSyncExternalStore === equality. + 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; } - } const instance = new OnyxCache(); From 7f580cee2d578c9c8707c194a0198992b00b6040 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 3 Apr 2026 16:05:33 +0100 Subject: [PATCH 2/7] Add unit tests --- tests/unit/onyxCacheTest.tsx | 153 +++++++++++++++++++++++++++++++++++ 1 file changed, 153 insertions(+) diff --git a/tests/unit/onyxCacheTest.tsx b/tests/unit/onyxCacheTest.tsx index fdaed1d51..70a227a0b 100644 --- a/tests/unit/onyxCacheTest.tsx +++ b/tests/unit/onyxCacheTest.tsx @@ -754,6 +754,159 @@ describe('Onyx', () => { }); }); + describe('getCollectionData', () => { + it('should return a frozen object', async () => { + await initOnyx(); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1}); + + const result = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + expect(result).toBeDefined(); + expect(Object.isFrozen(result)).toBe(true); + }); + + it('should return the same reference when nothing changed', async () => { + await initOnyx(); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1}); + + const first = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + const second = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + expect(first).toBe(second); + }); + + it('should return a new reference after a member is updated', async () => { + await initOnyx(); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1}); + + const before = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 2}); + const after = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + + expect(before).not.toBe(after); + expect(after).toEqual({[`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`]: {id: 2}}); + }); + + it('should return a new reference after a member is added', async () => { + await initOnyx(); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1}); + + const before = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}2`, {id: 2}); + const after = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + + expect(before).not.toBe(after); + expect(after).toEqual({ + [`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`]: {id: 1}, + [`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}2`]: {id: 2}, + }); + }); + + it('should return a stable empty reference for empty collections when keys are loaded', async () => { + await initOnyx(); + // Set a key so storageKeys is non-empty, but not a member of MOCK_COLLECTION + await Onyx.set(ONYX_KEYS.TEST_KEY, 'value'); + + const first = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + const second = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + + expect(first).toBeDefined(); + expect(first).toBe(second); + expect(Object.keys(first!)).toHaveLength(0); + }); + + it('should return undefined for empty collections when no keys are loaded', async () => { + await initOnyx(); + + const result = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + expect(result).toBeUndefined(); + }); + + it('should preserve unchanged member references when a sibling is updated', async () => { + await initOnyx(); + const member1Value = {id: 1, name: 'unchanged'}; + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, member1Value); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}2`, {id: 2}); + + const before = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}2`, {id: 3}); + const after = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + + // Snapshot reference changed (sibling updated) + expect(before).not.toBe(after); + // But unchanged member keeps the same reference + expect(after?.[`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`]).toBe(member1Value); + }); + }); + + describe('hasValueChanged', () => { + it('should return false for the same reference (fast path)', async () => { + await initOnyx(); + const value = {id: 1, name: 'test'}; + cache.set('test', value); + + expect(cache.hasValueChanged('test', value)).toBe(false); + }); + + it('should return false for deep-equal but different reference', async () => { + await initOnyx(); + cache.set('test', {id: 1, name: 'test'}); + + expect(cache.hasValueChanged('test', {id: 1, name: 'test'})).toBe(false); + }); + + it('should return true when value differs', async () => { + await initOnyx(); + cache.set('test', {id: 1}); + + expect(cache.hasValueChanged('test', {id: 2})).toBe(true); + }); + }); + + describe('merge', () => { + it('should not mark collection dirty when merged value is unchanged', async () => { + await initOnyx(); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1, name: 'test'}); + + const before = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + + // Merge with identical values — fastMerge returns same reference, so no-op + cache.merge({[`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`]: {id: 1, name: 'test'}}); + + const after = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + expect(before).toBe(after); + }); + + it('should mark collection dirty when a member value changes', async () => { + await initOnyx(); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1}); + + const before = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + + cache.merge({[`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`]: {id: 2}}); + + const after = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + expect(before).not.toBe(after); + expect(after![`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`]).toEqual({id: 2}); + }); + + it('should handle null values by removing the key from storageMap', async () => { + await initOnyx(); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1}); + + cache.merge({[`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`]: null}); + + expect(cache.get(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`)).toBeUndefined(); + }); + + it('should skip undefined values without modifying storageMap', async () => { + await initOnyx(); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1}); + + cache.merge({[`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`]: undefined}); + + expect(cache.get(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`)).toEqual({id: 1}); + }); + }); + it('should save RAM-only keys', () => { const testKeys = { ...ONYX_KEYS, From eac67740531ab1c5ebd796ef05d948db2f6d1599 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 3 Apr 2026 17:34:22 +0100 Subject: [PATCH 3/7] fix: use longest-prefix matching in rebuildCollectionSnapshot fallback scan --- lib/OnyxCache.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index 881fd91b9..063ea5eb3 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -491,7 +491,7 @@ class OnyxCache { const needsPrefixCheck = !memberKeys; for (const key of keysToScan) { - if (needsPrefixCheck && !OnyxKeys.isCollectionMemberKey(collectionKey, key)) { + if (needsPrefixCheck && OnyxKeys.getCollectionKey(key) !== collectionKey) { continue; } const val = this.storageMap[key]; From 9a6fd3029f16aa8883be1eec4353e0f62060b169 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 10 Apr 2026 20:01:37 +0100 Subject: [PATCH 4/7] Address comments --- lib/OnyxCache.ts | 29 ++++++++++++----------------- lib/utils.ts | 14 +++++++++++++- tests/unit/utilsTest.ts | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 18 deletions(-) diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index 063ea5eb3..d5f13e48c 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -344,22 +344,15 @@ class OnyxCache { iterResult = iterator.next(); } - const affectedCollections = new Set(); - for (const key of keysToRemove) { delete this.storageMap[key]; - // Track affected collections for snapshot rebuild const collectionKey = OnyxKeys.getCollectionKey(key); if (collectionKey) { - affectedCollections.add(collectionKey); + this.dirtyCollections.add(collectionKey); } this.recentKeys.delete(key); } - - for (const collectionKey of affectedCollections) { - this.dirtyCollections.add(collectionKey); - } } /** Set the recent keys list size */ @@ -478,10 +471,10 @@ class OnyxCache { * @param collectionKey - The collection key to rebuild */ private rebuildCollectionSnapshot(collectionKey: OnyxKey): void { - const oldSnapshot = this.collectionSnapshots.get(collectionKey); + const previousSnapshot = this.collectionSnapshots.get(collectionKey); const members: NonUndefined> = {}; - let hasChanges = false; + let hasMemberChanges = false; let newMemberCount = 0; // Use the indexed forward lookup for O(collectionMembers) iteration. @@ -495,29 +488,31 @@ class OnyxCache { continue; } 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; newMemberCount++; // Check if this member's reference changed from the old snapshot - if (!hasChanges && (!oldSnapshot || oldSnapshot[key] !== val)) { - hasChanges = true; + if (!hasMemberChanges && (!previousSnapshot || previousSnapshot[key] !== val)) { + hasMemberChanges = true; } } } // Check if any members were removed (old snapshot had more keys) - if (!hasChanges && oldSnapshot) { - const oldMemberCount = Object.keys(oldSnapshot).length; + if (!hasMemberChanges && previousSnapshot) { + const oldMemberCount = Object.keys(previousSnapshot).length; if (oldMemberCount !== newMemberCount) { - hasChanges = true; + hasMemberChanges = true; } } // 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 (!hasChanges && oldSnapshot) { + if (!hasMemberChanges && previousSnapshot) { return; } @@ -538,7 +533,7 @@ class OnyxCache { } const snapshot = this.collectionSnapshots.get(collectionKey); - if (!snapshot || Object.keys(snapshot).length === 0) { + if (utils.isEmptyObject(snapshot)) { // If we know we have storage keys loaded, return a stable empty reference // to avoid new {} allocations that break useSyncExternalStore === equality. if (this.storageKeys.size > 0) { diff --git a/lib/utils.ts b/lib/utils.ts index 1fca1c8a4..b469c1acd 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -172,7 +172,19 @@ function mergeObject>( /** Checks whether the given object is an object and not null/undefined. */ function isEmptyObject(obj: T | EmptyValue): obj is EmptyValue { - return typeof obj === 'object' && Object.keys(obj || {}).length === 0; + if (typeof obj !== 'object') { + return false; + } + + // Use for-in loop to avoid an unnecessary array allocation from Object.keys() + // eslint-disable-next-line no-restricted-syntax + for (const key in obj) { + if (Object.hasOwn(obj, key)) { + return false; + } + } + + return true; } /** diff --git a/tests/unit/utilsTest.ts b/tests/unit/utilsTest.ts index c76cae58b..48e51d598 100644 --- a/tests/unit/utilsTest.ts +++ b/tests/unit/utilsTest.ts @@ -389,4 +389,36 @@ describe('utils', () => { }); }); }); + + describe('isEmptyObject', () => { + it('should return true for an empty object', () => { + expect(utils.isEmptyObject({})).toBe(true); + }); + + it('should return true for null', () => { + expect(utils.isEmptyObject(null)).toBe(true); + }); + + it('should return false for undefined', () => { + expect(utils.isEmptyObject(undefined)).toBe(false); + }); + + it('should return false for an object with properties', () => { + expect(utils.isEmptyObject({a: 1})).toBe(false); + }); + + it('should return false for non-object types', () => { + expect(utils.isEmptyObject('hello')).toBe(false); + expect(utils.isEmptyObject(42)).toBe(false); + expect(utils.isEmptyObject(true)).toBe(false); + }); + + it('should return true for an empty array', () => { + expect(utils.isEmptyObject([])).toBe(true); + }); + + it('should return false for a non-empty array', () => { + expect(utils.isEmptyObject([1, 2])).toBe(false); + }); + }); }); From c1dbb9d65b62ad4961eda2fae50c8a3ab7916439 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 10 Apr 2026 20:08:46 +0100 Subject: [PATCH 5/7] fix: add comment explaining storageKeys.size check in getCollectionData --- lib/OnyxCache.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index d5f13e48c..303146172 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -534,8 +534,11 @@ class OnyxCache { const snapshot = this.collectionSnapshots.get(collectionKey); if (utils.isEmptyObject(snapshot)) { - // If we know we have storage keys loaded, return a stable empty reference - // to avoid new {} allocations that break useSyncExternalStore === equality. + // 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; } From c16605da52ea631001b95ed0a3b6ce442b20e05d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 10 Apr 2026 20:17:24 +0100 Subject: [PATCH 6/7] Detect member removals in snapshot rebuild when add and remove happen simultaneously --- lib/OnyxCache.ts | 15 +++++++++------ tests/unit/onyxCacheTest.tsx | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index 303146172..521d791a0 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -475,7 +475,6 @@ class OnyxCache { const members: NonUndefined> = {}; let hasMemberChanges = false; - let newMemberCount = 0; // Use the indexed forward lookup for O(collectionMembers) iteration. // Falls back to scanning all storageKeys if the index isn't populated yet. @@ -492,7 +491,6 @@ class OnyxCache { // and should not be included in the frozen collection snapshot. if (val !== undefined && val !== null) { members[key] = val; - newMemberCount++; // Check if this member's reference changed from the old snapshot if (!hasMemberChanges && (!previousSnapshot || previousSnapshot[key] !== val)) { @@ -501,11 +499,16 @@ class OnyxCache { } } - // Check if any members were removed (old snapshot had more keys) + // 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) { - const oldMemberCount = Object.keys(previousSnapshot).length; - if (oldMemberCount !== newMemberCount) { - hasMemberChanges = true; + // eslint-disable-next-line no-restricted-syntax + for (const key in previousSnapshot) { + if (!(key in members)) { + hasMemberChanges = true; + break; + } } } diff --git a/tests/unit/onyxCacheTest.tsx b/tests/unit/onyxCacheTest.tsx index 70a227a0b..667e70aee 100644 --- a/tests/unit/onyxCacheTest.tsx +++ b/tests/unit/onyxCacheTest.tsx @@ -820,6 +820,25 @@ describe('Onyx', () => { expect(result).toBeUndefined(); }); + it('should return a new reference when a member is removed and another added simultaneously', async () => { + await initOnyx(); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1}); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}2`, {id: 2}); + + const before = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + + // Remove member 1 and add member 3 — count stays the same (2) but content changed + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, null); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}3`, {id: 3}); + const after = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + + expect(before).not.toBe(after); + expect(after).toEqual({ + [`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}2`]: {id: 2}, + [`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}3`]: {id: 3}, + }); + }); + it('should preserve unchanged member references when a sibling is updated', async () => { await initOnyx(); const member1Value = {id: 1, name: 'unchanged'}; From 17fd83c8da9edceb17c2f78811d1e8695c90d571 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 10 Apr 2026 20:27:08 +0100 Subject: [PATCH 7/7] Address comments --- lib/OnyxCache.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index 521d791a0..0c4738b51 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -483,6 +483,8 @@ class OnyxCache { 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; }