diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index 0444de37..063ea5eb 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,26 +461,93 @@ 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]) { + 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 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.getCollectionKey(key) !== collectionKey) { continue; } - this.collectionData[collectionKey] = {}; + 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 + * 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; } } diff --git a/tests/unit/onyxCacheTest.tsx b/tests/unit/onyxCacheTest.tsx index fdaed1d5..70a227a0 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,