From 11e4104f1e56cae1fe1e342d7e40f664153e43cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 11 Jun 2026 14:48:04 +0100 Subject: [PATCH 01/12] Add OnyxStore as a standalone subscription registry (inert) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce lib/OnyxStore.ts: a single listener registry (keyListeners Map>) with subscribe / notifyKey / notifyCollection / getState / hasListenersForKey / clearAll. Built on the existing structural-sharing cache (cache.getCollectionData frozen snapshots). This module is inert — nothing imports it yet. The subscription and notification paths (Onyx.connect, useOnyx, OnyxUtils.notify*) are wired onto it in a later change. Adding it alone has zero behavioral impact. Includes tests/unit/OnyxStoreTest.ts (20 tests) covering exact-key and collection-snapshot routing, ref-equality member skips, hasListenersForKey, clearAll, and listener error isolation. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/OnyxStore.ts | 186 +++++++++++++++++++++++++ tests/unit/OnyxStoreTest.ts | 270 ++++++++++++++++++++++++++++++++++++ 2 files changed, 456 insertions(+) create mode 100644 lib/OnyxStore.ts create mode 100644 tests/unit/OnyxStoreTest.ts diff --git a/lib/OnyxStore.ts b/lib/OnyxStore.ts new file mode 100644 index 000000000..449c0751f --- /dev/null +++ b/lib/OnyxStore.ts @@ -0,0 +1,186 @@ +import cache from './OnyxCache'; +import OnyxKeys from './OnyxKeys'; +import * as Logger from './Logger'; +import type {CollectionKeyBase, KeyValueMapping, OnyxCollection, OnyxKey, OnyxValue} from './types'; + +/** + * Listener fired when an exact key's value changes. For collection root keys this is the + * snapshot-mode listener: receives the frozen collection snapshot every time a member changes. + */ +type KeyListener = (value: OnyxValue, key: TKey) => void; + +/** + * `OnyxStore` is a single listener registry for Onyx reads/subscriptions. It replaces the + * connection manager's several per-subscription bookkeeping structures with one index: + * + * keyListeners — listeners on an exact key (a single key, a collection root in snapshot + * mode, or a specific collection member). + * + * Write paths call `notifyKey()` (single-key write) or `notifyCollection()` (batch collection + * update from `mergeCollection`/`setCollection`/`clear`). + * + * NOTE: This module is introduced inert — nothing calls it yet. The subscription/notification + * paths (`Onyx.connect`, `useOnyx`, `OnyxUtils.notify*`) are wired onto it in a later change. + */ +class OnyxStore { + private keyListeners: Map>; + + constructor() { + this.keyListeners = new Map(); + } + + /** + * Sync, cache-only read. Returns the frozen collection snapshot for collection + * keys, the cached value for single keys, or `undefined` if not in cache. + */ + getState(key: TKey): OnyxValue { + if (OnyxKeys.isCollectionKey(key)) { + return cache.getCollectionData(key) as OnyxValue; + } + return cache.get(key) as OnyxValue; + } + + /** + * Subscribe to an exact key. For collection root keys this is "snapshot mode" — + * the listener fires with the frozen collection snapshot whenever any member + * changes. For collection member keys or regular keys, the listener fires when + * that specific key's value changes. + * + * Returns an unsubscribe function. + */ + subscribe(key: TKey, listener: KeyListener): () => void { + let listeners = this.keyListeners.get(key); + if (!listeners) { + listeners = new Set(); + this.keyListeners.set(key, listeners); + } + listeners.add(listener as unknown as KeyListener); + return () => { + const set = this.keyListeners.get(key); + if (!set) { + return; + } + set.delete(listener as unknown as KeyListener); + if (set.size === 0) { + this.keyListeners.delete(key); + } + }; + } + + /** + * Notify of a single-key write. + * + * Dispatch: + * 1. keyListeners.get(key) — exact-key subscribers (always fires) + * 2. If key is a collection member: keyListeners.get(collectionKey) — snapshot + * subscribers for the parent collection (unless suppressed). + * + * `options.suppressCollectionSnapshot` skips step 2 — used by collection-batch + * write paths so each member-write doesn't re-trigger the collection-level + * snapshot listeners; the outer `notifyCollection()` fires those once. + */ + notifyKey(key: TKey, value: OnyxValue, options?: {suppressCollectionSnapshot?: boolean}): void { + // 1. Exact-key listeners + const exact = this.keyListeners.get(key); + if (exact && exact.size > 0) { + for (const listener of exact) { + this.safeInvoke(() => listener(value as OnyxValue, key), key); + } + } + + // 2. Collection-level snapshot routing — only fires when the write is to a member key. + // Direct writes to a collection root (e.g. `Onyx.merge(COLLECTION_KEY, ...)`) are + // an unsupported anti-pattern — treat them as opaque single-key writes. + const collectionKey = OnyxKeys.getCollectionKey(key); + const isCollectionMemberWrite = collectionKey !== undefined && collectionKey !== key; + if (isCollectionMemberWrite && !options?.suppressCollectionSnapshot) { + const snapshotListeners = this.keyListeners.get(collectionKey); + if (snapshotListeners && snapshotListeners.size > 0) { + const snapshot = cache.getCollectionData(collectionKey); + for (const listener of snapshotListeners) { + this.safeInvoke(() => listener(snapshot as OnyxValue, collectionKey), collectionKey); + } + } + } + } + + /** + * Notify of a collection-level batch update. Used by `mergeCollection`, + * `setCollection`, and `clear`'s collection path. + * + * Dispatch: + * 1. keyListeners.get(collectionKey) — fires ONCE with the new snapshot. + * 2. keyListeners.get(memberKey) — fires per changed member where the value + * differs from the previous (for ref-equality on unchanged members). + */ + notifyCollection( + collectionKey: TKey, + partialCollection: OnyxCollection, + partialPreviousCollection?: OnyxCollection, + ): void { + const changedKeys = Object.keys(partialCollection ?? {}); + if (changedKeys.length === 0) { + return; + } + const previous = partialPreviousCollection ?? {}; + + // Read the merged snapshot once. `cache.getCollectionData()` returns the post-merge + // frozen object, which is what listeners should see (not the raw `partialCollection` + // input, which is just the delta and lacks fields preserved during merge). + const snapshot = cache.getCollectionData(collectionKey); + + // 1. Snapshot subscribers fire once with the new snapshot. + const snapshotListeners = this.keyListeners.get(collectionKey); + if (snapshotListeners && snapshotListeners.size > 0) { + for (const listener of snapshotListeners) { + this.safeInvoke(() => listener(snapshot as OnyxValue, collectionKey), collectionKey); + } + } + + // 2. Exact-member subscribers fire per changed key (skip if ref unchanged vs previous). + for (const memberKey of changedKeys) { + const value = snapshot?.[memberKey]; + const prev = previous[memberKey]; + if (value === prev) { + continue; + } + const exact = this.keyListeners.get(memberKey); + if (!exact || exact.size === 0) { + continue; + } + for (const listener of exact) { + this.safeInvoke(() => listener(value as OnyxValue, memberKey), memberKey); + } + } + } + + /** Wipe all subscriptions. Used by tests and `Onyx.clear()` follow-on. */ + clearAll(): void { + this.keyListeners.clear(); + } + + /** True if there are any subscribers for the given key (exact or parent collection). */ + hasListenersForKey(key: OnyxKey): boolean { + if ((this.keyListeners.get(key)?.size ?? 0) > 0) { + return true; + } + const collectionKey = OnyxKeys.getCollectionKey(key); + if (collectionKey && collectionKey !== key && (this.keyListeners.get(collectionKey)?.size ?? 0) > 0) { + return true; + } + return false; + } + + private safeInvoke(fn: () => void, contextKey: OnyxKey): void { + try { + fn(); + } catch (error) { + Logger.logAlert(`[OnyxStore] Listener threw an error for key '${contextKey}': ${error}`); + } + } +} + +const onyxStore = new OnyxStore(); + +export default onyxStore; +export type {KeyListener}; diff --git a/tests/unit/OnyxStoreTest.ts b/tests/unit/OnyxStoreTest.ts new file mode 100644 index 000000000..e1eb3d1b3 --- /dev/null +++ b/tests/unit/OnyxStoreTest.ts @@ -0,0 +1,270 @@ +import type {OnyxKey} from '../../lib'; +import Onyx from '../../lib'; +import onyxStore from '../../lib/OnyxStore'; +import cache from '../../lib/OnyxCache'; +import * as Logger from '../../lib/Logger'; + +// We need access to some internal properties of `onyxStore` during the tests but they are private, +// so this workaround allows us to have access to them. The maps are created once in the constructor +// and only ever `.clear()`ed (never reassigned), so capturing the references here stays valid. +// eslint-disable-next-line dot-notation +const keyListeners = onyxStore['keyListeners']; + +const ONYXKEYS = { + TEST_KEY: 'test', + OTHER_TEST: 'otherTest', + COLLECTION: { + TEST_KEY: 'test_', + }, +}; + +const COLLECTION = ONYXKEYS.COLLECTION.TEST_KEY; +const MEMBER_1 = `${COLLECTION}1`; +const MEMBER_2 = `${COLLECTION}2`; + +Onyx.init({ + keys: ONYXKEYS, +}); + +beforeEach(() => Onyx.clear()); + +describe('OnyxStore', () => { + // Always start from a clean registry. + beforeEach(() => { + onyxStore.clearAll(); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe('subscribe / notifyKey', () => { + it('should fire the listener with (value, key) on notifyKey', () => { + const callback = jest.fn(); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback); + + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'hello'); + + expect(callback).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenCalledWith('hello', ONYXKEYS.TEST_KEY); + }); + + it('should fire all listeners registered on the same key', () => { + const callback1 = jest.fn(); + const callback2 = jest.fn(); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback1); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback2); + + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 1); + + expect(callback1).toHaveBeenCalledTimes(1); + expect(callback2).toHaveBeenCalledTimes(1); + }); + + it('should not fire the listener after it unsubscribes', () => { + const callback = jest.fn(); + const unsubscribe = onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback); + + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'first'); + unsubscribe(); + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'second'); + + expect(callback).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenLastCalledWith('first', ONYXKEYS.TEST_KEY); + }); + + it('should only unsubscribe the specific listener, leaving others intact', () => { + const callback1 = jest.fn(); + const callback2 = jest.fn(); + const unsubscribe1 = onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback1); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback2); + + unsubscribe1(); + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 1); + + expect(callback1).not.toHaveBeenCalled(); + expect(callback2).toHaveBeenCalledTimes(1); + }); + + it('should delete the key entry from the internal map once the last listener unsubscribes', () => { + const unsubscribe = onyxStore.subscribe(ONYXKEYS.TEST_KEY, jest.fn()); + expect(keyListeners.has(ONYXKEYS.TEST_KEY)).toBeTruthy(); + + unsubscribe(); + + expect(keyListeners.has(ONYXKEYS.TEST_KEY)).toBeFalsy(); + }); + + it('should be a no-op to notify a key with no listeners', () => { + expect(() => onyxStore.notifyKey('keyWithNoListeners' as OnyxKey, 'x')).not.toThrow(); + }); + + it('should be idempotent when unsubscribing more than once', () => { + const callback = jest.fn(); + const unsubscribe = onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback); + + unsubscribe(); + expect(() => unsubscribe()).not.toThrow(); + + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 1); + expect(callback).not.toHaveBeenCalled(); + }); + }); + + describe('collection-snapshot routing on notifyKey', () => { + it('should fire the collection-root snapshot listener with the cache snapshot when a member is written', () => { + const snapshot = {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}; + const getCollectionData = jest.spyOn(cache, 'getCollectionData').mockReturnValue(snapshot); + + const callback = jest.fn(); + onyxStore.subscribe(COLLECTION, callback); + + onyxStore.notifyKey(MEMBER_1, {id: 1}); + + expect(getCollectionData).toHaveBeenCalledWith(COLLECTION); + expect(callback).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenCalledWith(snapshot, COLLECTION); + }); + + it('should fire both the exact-member listener and the collection-root snapshot listener', () => { + const snapshot = {[MEMBER_1]: {id: 1}}; + jest.spyOn(cache, 'getCollectionData').mockReturnValue(snapshot); + + const memberCallback = jest.fn(); + const snapshotCallback = jest.fn(); + onyxStore.subscribe(MEMBER_1, memberCallback); + onyxStore.subscribe(COLLECTION, snapshotCallback); + + onyxStore.notifyKey(MEMBER_1, {id: 1}); + + expect(memberCallback).toHaveBeenCalledWith({id: 1}, MEMBER_1); + expect(snapshotCallback).toHaveBeenCalledWith(snapshot, COLLECTION); + }); + + it('should skip the collection-root listener but still fire the exact-member listener when suppressCollectionSnapshot is set', () => { + const getCollectionData = jest.spyOn(cache, 'getCollectionData').mockReturnValue({}); + + const memberCallback = jest.fn(); + const snapshotCallback = jest.fn(); + onyxStore.subscribe(MEMBER_1, memberCallback); + onyxStore.subscribe(COLLECTION, snapshotCallback); + + onyxStore.notifyKey(MEMBER_1, {id: 1}, {suppressCollectionSnapshot: true}); + + expect(memberCallback).toHaveBeenCalledTimes(1); + expect(snapshotCallback).not.toHaveBeenCalled(); + // The snapshot is never read when suppressed. + expect(getCollectionData).not.toHaveBeenCalled(); + }); + + it('should not perform collection routing for a non-member single key', () => { + const getCollectionData = jest.spyOn(cache, 'getCollectionData'); + const callback = jest.fn(); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback); + + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'x'); + + expect(callback).toHaveBeenCalledTimes(1); + expect(getCollectionData).not.toHaveBeenCalled(); + }); + }); + + describe('notifyCollection', () => { + it('should fire the snapshot listener once with the cache snapshot', () => { + const snapshot = {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}; + jest.spyOn(cache, 'getCollectionData').mockReturnValue(snapshot); + + const callback = jest.fn(); + onyxStore.subscribe(COLLECTION, callback); + + onyxStore.notifyCollection(COLLECTION, {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}); + + expect(callback).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenCalledWith(snapshot, COLLECTION); + }); + + it('should fire exact-member listeners only for members whose value reference changed', () => { + const shared = {id: 2}; // same reference in snapshot and previous → should be skipped + const snapshot = {[MEMBER_1]: {id: 1}, [MEMBER_2]: shared}; + jest.spyOn(cache, 'getCollectionData').mockReturnValue(snapshot); + + const member1Callback = jest.fn(); + const member2Callback = jest.fn(); + onyxStore.subscribe(MEMBER_1, member1Callback); + onyxStore.subscribe(MEMBER_2, member2Callback); + + onyxStore.notifyCollection( + COLLECTION, + {[MEMBER_1]: {id: 1}, [MEMBER_2]: shared}, + {[MEMBER_2]: shared}, // previous: member 2 unchanged by reference + ); + + expect(member1Callback).toHaveBeenCalledWith({id: 1}, MEMBER_1); + expect(member2Callback).not.toHaveBeenCalled(); + }); + + it('should be a no-op when the partial collection is empty', () => { + const callback = jest.fn(); + onyxStore.subscribe(COLLECTION, callback); + + onyxStore.notifyCollection(COLLECTION, {}); + + expect(callback).not.toHaveBeenCalled(); + }); + }); + + describe('hasListenersForKey', () => { + it('should return true for an exact-key subscriber', () => { + onyxStore.subscribe(ONYXKEYS.TEST_KEY, jest.fn()); + expect(onyxStore.hasListenersForKey(ONYXKEYS.TEST_KEY)).toBeTruthy(); + }); + + it('should return true for a member key when its parent collection has a subscriber', () => { + onyxStore.subscribe(COLLECTION, jest.fn()); + expect(onyxStore.hasListenersForKey(MEMBER_1)).toBeTruthy(); + }); + + it('should return false when there are no relevant subscribers', () => { + expect(onyxStore.hasListenersForKey('someUnwatchedKey')).toBeFalsy(); + }); + + it('should return false after the last listener unsubscribes', () => { + const unsubscribe = onyxStore.subscribe(ONYXKEYS.TEST_KEY, jest.fn()); + unsubscribe(); + expect(onyxStore.hasListenersForKey(ONYXKEYS.TEST_KEY)).toBeFalsy(); + }); + }); + + describe('clearAll', () => { + it('should wipe key and collection subscriptions', () => { + const keyCallback = jest.fn(); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, keyCallback); + + onyxStore.clearAll(); + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'x'); + + expect(keyCallback).not.toHaveBeenCalled(); + expect(onyxStore.hasListenersForKey(ONYXKEYS.TEST_KEY)).toBeFalsy(); + }); + }); + + describe('listener error isolation', () => { + it('should log a throwing listener and still fire the other listeners', () => { + const logAlertSpy = jest.spyOn(Logger, 'logAlert').mockImplementation(() => { + /* empty */ + }); + const throwingCallback = jest.fn(() => { + throw new Error('boom'); + }); + const healthyCallback = jest.fn(); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, throwingCallback); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, healthyCallback); + + expect(() => onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'x')).not.toThrow(); + + expect(throwingCallback).toHaveBeenCalledTimes(1); + expect(healthyCallback).toHaveBeenCalledTimes(1); + expect(logAlertSpy).toHaveBeenCalled(); + }); + }); +}); From 0af8c8cd999f583e249194af8898630cdbcfac51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 11 Jun 2026 21:19:53 +0100 Subject: [PATCH 02/12] Flip the subscription layer onto OnyxStore (engine swap) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire useOnyx, Onyx.connect/disconnect, and the OnyxUtils notify path onto the OnyxStore registry. useOnyx is rebuilt on useSyncExternalStore reading the eager, structurally-shared cache via onyxStore.getState — it renders once (no loading->loaded second render) and bails out via === on the stable cached reference. Delete OnyxConnectionManager and OnyxSnapshotCache (and their unit + perf tests). Also remove the now-defunct reuseConnection option (connection pooling is gone). The `loading` status value is retained for now (always 'loaded' in practice) and removed in a follow-up; OnyxStore stays trimmed (no subscribeState) since useOnyxState is a separate proposal. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/Onyx.ts | 215 +++++--- lib/OnyxCache.ts | 18 +- lib/OnyxConnectionManager.ts | 262 --------- lib/OnyxSnapshotCache.ts | 154 ------ lib/OnyxUtils.ts | 423 ++------------- lib/index.ts | 3 +- lib/types.ts | 34 +- lib/useOnyx.ts | 252 ++------- .../OnyxConnectionManager.perf-test.ts | 149 ------ .../perf-test/OnyxSnapshotCache.perf-test.ts | 249 --------- tests/perf-test/OnyxUtils.perf-test.ts | 193 ++----- tests/perf-test/useOnyx.perf-test.tsx | 66 --- tests/unit/OnyxConnectionManagerTest.ts | 468 ---------------- tests/unit/OnyxSnapshotCacheTest.ts | 241 --------- tests/unit/collectionHydrationTest.ts | 2 +- tests/unit/onyxCacheTest.tsx | 9 +- tests/unit/onyxClearNativeStorageTest.ts | 2 +- tests/unit/onyxClearWebStorageTest.ts | 7 +- tests/unit/onyxTest.ts | 137 ++--- tests/unit/onyxUtilsTest.ts | 235 +------- tests/unit/useOnyxTest.ts | 505 ++++++------------ 21 files changed, 564 insertions(+), 3060 deletions(-) delete mode 100644 lib/OnyxConnectionManager.ts delete mode 100644 lib/OnyxSnapshotCache.ts delete mode 100644 tests/perf-test/OnyxConnectionManager.perf-test.ts delete mode 100644 tests/perf-test/OnyxSnapshotCache.perf-test.ts delete mode 100644 tests/unit/OnyxConnectionManagerTest.ts delete mode 100644 tests/unit/OnyxSnapshotCacheTest.ts diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 0a0d8abc9..9d882e47e 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -4,8 +4,10 @@ import Storage from './storage'; import utils from './utils'; import DevTools, {initDevTools} from './DevTools'; import type { + CollectionConnectCallback, CollectionKeyBase, ConnectOptions, + DefaultConnectCallback, InitOptions, KeyValueMapping, MixedOperationsQueue, @@ -28,10 +30,28 @@ import type { import OnyxUtils from './OnyxUtils'; import OnyxKeys from './OnyxKeys'; import logMessages from './logMessages'; -import type {Connection} from './OnyxConnectionManager'; -import connectionManager from './OnyxConnectionManager'; +import onyxStore from './OnyxStore'; import OnyxMerge from './OnyxMerge'; +/** + * Opaque handle returned by `Onyx.connect()` / `Onyx.connectWithoutView()`. + * Pass it to `Onyx.disconnect()` to stop receiving callbacks for this subscription. + */ +type Connection = { + /** Unsubscribe this connection. Idempotent. */ + unsubscribe: () => void; +}; + +/** + * Shared sentinel for "nothing delivered yet" in `connect()`'s per-subscription dedup. + * A unique Symbol can't collide with any real Onyx value, so the first `Object.is` check + * never matches and the initial fire always runs — even for a key whose genuine first + * value is `undefined`. It only needs to be distinct from real values, not unique per + * subscription, so a single module-level instance is reused by every connection. + */ +// eslint-disable-next-line rulesdir/no-negated-variables +const NOT_DELIVERED = Symbol('NOT_DELIVERED'); + /** Initialize the store with actions and listening for storage events */ function init({ keys = {}, @@ -71,7 +91,7 @@ function init({ const collectionKey = OnyxKeys.getCollectionKey(key); const isCollectionMember = !!collectionKey && OnyxKeys.isCollectionMemberKey(collectionKey, key); - // Capture the previous cached value BEFORE cache.set() so keysChanged() can diff old vs new per member. + // Capture the previous cached value BEFORE cache.set() so notifyCollection() can diff old vs new per member. const previousValue = isCollectionMember ? cache.get(key) : undefined; cache.set(key, value); @@ -92,15 +112,15 @@ function init({ } } - // Non-collection keys: notify individually, matching keyChanged() semantics for exact keys. + // Non-collection keys: notify individually, matching notifyKey() semantics for exact keys. for (const [key, value] of individual) { - OnyxUtils.keyChanged(key, value); + OnyxUtils.notifyKey(key, value); } - // One keysChanged() per collection notifies the collection-root subscriber once and lets - // keysChanged() decide which individual member subscribers actually changed. + // One notifyCollection() per collection notifies the collection-root subscriber once and lets + // notifyCollection() decide which individual member subscribers actually changed. for (const [collectionKey, {partial, previous}] of collectionBatches) { - OnyxUtils.keysChanged(collectionKey, partial, previous); + OnyxUtils.notifyCollection(collectionKey, partial, previous); } }); } @@ -117,71 +137,142 @@ function init({ } /** - * Connects to an Onyx key given the options passed and listens to its changes. - * This method will be deprecated soon. Please use `Onyx.connectWithoutView()` instead. - * - * @example - * ```ts - * const connection = Onyx.connectWithoutView({ - * key: ONYXKEYS.SESSION, - * callback: onSessionChange, - * }); - * ``` + * Sync, cache-only read of an Onyx key. Returns the frozen collection snapshot for + * collection keys, the cached value for single keys, or `undefined` if the key isn't + * in cache (no storage fallback). * - * @param connectOptions The options object that will define the behavior of the connection. - * @param connectOptions.key The Onyx key to subscribe to. - * @param connectOptions.callback A function that will be called when the Onyx data we are subscribed changes. - * @param connectOptions.selector This will be used to subscribe to a subset of an Onyx key's data. **Only used inside `useOnyx()` hook.** - * Using this setting on `useOnyx()` can have very positive performance benefits because the component will only re-render - * when the subset of data changes. Otherwise, any change of data on any property would normally - * cause the component to re-render (and that can be expensive from a performance standpoint). - * @returns The connection object to use when calling `Onyx.disconnect()`. + * Use this for one-off reads outside React. Inside React, prefer `useOnyx`. */ -function connect(connectOptions: ConnectOptions): Connection { - return connectionManager.connect(connectOptions); +function getState(key: TKey): OnyxValue { + return onyxStore.getState(key); } /** - * Connects to an Onyx key given the options passed and listens to its changes. + * Defer initial-fire of `Onyx.connect` callbacks far enough that any Onyx writes + * scheduled in the same synchronous tick have applied before the callback reads cache. * - * @example - * ```ts - * const connection = Onyx.connectWithoutView({ - * key: ONYXKEYS.SESSION, - * callback: onSessionChange, - * }); - * ``` + * The legacy `subscribeToKey` chain (`deferredInitTask.then(getAllKeys).then(multiGet) + * .then(sendDataToConnection)`) reached this depth incidentally via storage I/O. The + * new store-based wrapper has no storage chain, so we have to introduce the depth + * explicitly. The three nested `.then()`s match the legacy effective depth — enough + * to outpace the longest in-flight write chain: `Onyx.update` -> `clearPromise.then` + * -> per-item `Onyx.merge` -> `OnyxUtils.get(key).then(applyMerge)` is two hops to + * apply, so the third hop guarantees initial-fire reads the post-write cache. * - * @param connectOptions The options object that will define the behavior of the connection. - * @param connectOptions.key The Onyx key to subscribe to. - * @param connectOptions.callback A function that will be called when the Onyx data we are subscribed changes. - * @param connectOptions.selector This will be used to subscribe to a subset of an Onyx key's data. **Only used inside `useOnyx()` hook.** - * Using this setting on `useOnyx()` can have very positive performance benefits because the component will only re-render - * when the subset of data changes. Otherwise, any change of data on any property would normally - * cause the component to re-render (and that can be expensive from a performance standpoint). - * @returns The connection object to use when calling `Onyx.disconnect()`. + * Microtask depth (not `setTimeout(0)`) is required because Jest test bodies run + * entirely in microtask land via chained `.then()`s; a macrotask-deferred initial + * fire would not run until the chain returns to the event loop, which can be after + * the test's assertions execute — leaving module-level Onyx subscribers stale. */ -function connectWithoutView(connectOptions: ConnectOptions): Connection { - return connectionManager.connect(connectOptions); +function scheduleInitialFire(fn: () => void): void { + Promise.resolve() + .then(() => Promise.resolve()) + .then(() => Promise.resolve()) + .then(fn); } /** - * Disconnects and removes the listener from the Onyx key. - * - * @example - * ```ts - * const connection = Onyx.connectWithoutView({ - * key: ONYXKEYS.SESSION, - * callback: onSessionChange, - * }); + * Subscribe to changes for `key`. * - * Onyx.disconnect(connection); - * ``` + * For a collection root key, the callback fires with the entire frozen collection + * snapshot whenever any member changes; signature `(collection, collectionKey)`. + * For any other key, the callback fires with the value at that key; signature + * `(value, key)`. Initial fire is deferred via `scheduleInitialFire` so it reads + * cache after any same-tick writes have applied. * - * @param connection Connection object returned by calling `Onyx.connect()` or `Onyx.connectWithoutView()`. + * Returns synchronously with a `Connection` handle. Disconnecting is idempotent. + */ +function connect(connectOptions: ConnectOptions): Connection { + const {key, callback} = connectOptions; + + let active = true; + let unsubscribeFn: (() => void) | null = null; + + const wireUp = () => { + if (!active) { + return; + } + + if (OnyxKeys.isCollectionKey(key)) { + // Collection-root snapshot mode — listener fires with the whole snapshot per + // collection change. Callback shape is `(snapshot, key)`. Dedup: skip identical + // snapshot refs. Initial fire always delivers the current snapshot (frozen `{}` + // for an empty-but-known collection, `undefined` only if the collection key has + // not been seen yet). + let lastDeliveredSnapshot: unknown = NOT_DELIVERED; + const deliverSnapshot = (rawSnapshot: OnyxValue | undefined, k: TKey) => { + if (Object.is(lastDeliveredSnapshot, rawSnapshot)) { + return; + } + lastDeliveredSnapshot = rawSnapshot; + (callback as CollectionConnectCallback | undefined)?.(rawSnapshot as NonNullable>, k); + }; + unsubscribeFn = onyxStore.subscribe(key, (value, k) => { + deliverSnapshot(value as unknown as OnyxValue, k as TKey); + }); + scheduleInitialFire(() => { + if (!active) { + return; + } + deliverSnapshot(onyxStore.getState(key) as unknown as OnyxValue, key as TKey); + }); + return; + } + + // Non-collection key (or a specific collection member) — single-value subscription. + let lastDelivered: unknown = NOT_DELIVERED; + const deliverValue = (value: OnyxValue, k: TKey | undefined) => { + if (Object.is(lastDelivered, value)) { + return; + } + lastDelivered = value; + (callback as DefaultConnectCallback | undefined)?.(value, k as TKey); + }; + unsubscribeFn = onyxStore.subscribe(key, (value, k) => { + deliverValue(value, k as TKey); + }); + scheduleInitialFire(() => { + if (!active) { + return; + } + deliverValue(onyxStore.getState(key), key); + }); + }; + + OnyxUtils.afterInit(() => { + wireUp(); + return Promise.resolve(); + }); + + return { + unsubscribe: () => { + if (!active) { + return; + } + active = false; + if (unsubscribeFn) { + unsubscribeFn(); + unsubscribeFn = null; + } + }, + }; +} + +/** + * Identical to `connect()` — kept for naming consistency with existing call sites. + */ +function connectWithoutView(connectOptions: ConnectOptions): Connection { + return connect(connectOptions); +} + +/** + * Disconnects a subscription previously returned by `connect()` / `connectWithoutView()`. */ function disconnect(connection: Connection): void { - connectionManager.disconnect(connection); + if (!connection) { + return; + } + connection.unsubscribe(); } /** @@ -424,17 +515,16 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { // Remove only the items that we want cleared from storage, and reset others to default for (const key of keysToBeClearedFromStorage) cache.drop(key); return Storage.removeItems(keysToBeClearedFromStorage) - .then(() => connectionManager.refreshSessionID()) .then(() => Storage.multiSet(defaultKeyValuePairs)) .then(() => { DevTools.clearState(keysToPreserve); // Notify the subscribers for each key/value group so they can receive the new values for (const [key, value] of Object.entries(keyValuesToResetIndividually)) { - OnyxUtils.keyChanged(key, value); + OnyxUtils.notifyKey(key, value); } for (const [key, value] of Object.entries(keyValuesToResetAsCollection)) { - OnyxUtils.keysChanged(key, value.newValues, value.oldValues); + OnyxUtils.notifyCollection(key, value.newValues, value.oldValues); } }); }) @@ -613,6 +703,7 @@ function setCollection(collectionKey: TKey, coll const Onyx = { METHOD: OnyxUtils.METHOD, + getState, connect, connectWithoutView, disconnect, @@ -628,4 +719,4 @@ const Onyx = { }; export default Onyx; -export type {OnyxUpdate, ConnectOptions, SetOptions}; +export type {OnyxUpdate, ConnectOptions, SetOptions, Connection}; diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index e105a0ab6..2d8977261 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -477,20 +477,18 @@ class OnyxCache { const snapshot = this.collectionSnapshots.get(collectionKey); - // We never stored anything for this collection key. + // No entry for this collection key means init hasn't seeded it yet (pre-load), so there's + // genuinely nothing to return. `setCollectionKeys()` (called inside `Onyx.init`) seeds every + // known collection with a frozen empty entry, so the presence of an entry is the reliable + // post-init "loaded" signal — and unlike `storageKeys.size > 0`, it doesn't flip back to + // "not loaded" after `Onyx.clear()` wipes the storage-keys index. An empty collection is + // stored as the shared `FROZEN_EMPTY_COLLECTION` reference (see `rebuildCollectionSnapshot`), + // so returning `snapshot` directly hands back that frozen empty object with a quick `===` + // check and no per-read `isEmptyObject` scan. if (snapshot === undefined) { return undefined; } - // The collection is empty (it holds our shared empty object). But "empty" is ambiguous - // during startup: we can't tell an actually-empty collection apart from one whose data - // hasn't loaded yet. Once any key exists, we know setAllKeys has run and loaded everything, - // so an empty collection really is empty. Before that, return undefined so subscribers - // don't briefly see a collection as empty when it just hasn't loaded. - if (snapshot === FROZEN_EMPTY_COLLECTION) { - return this.storageKeys.size > 0 ? FROZEN_EMPTY_COLLECTION : undefined; - } - return snapshot; } } diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts deleted file mode 100644 index 1cc245e32..000000000 --- a/lib/OnyxConnectionManager.ts +++ /dev/null @@ -1,262 +0,0 @@ -import bindAll from 'lodash/bindAll'; -import * as Logger from './Logger'; -import type {ConnectOptions} from './Onyx'; -import OnyxUtils from './OnyxUtils'; -import OnyxKeys from './OnyxKeys'; -import * as Str from './Str'; -import type {CollectionConnectCallback, DefaultConnectCallback, OnyxKey, OnyxValue} from './types'; -import onyxSnapshotCache from './OnyxSnapshotCache'; - -type ConnectCallback = DefaultConnectCallback | CollectionConnectCallback; - -/** - * Represents the connection's metadata that contains the necessary properties - * to handle that connection. - */ -type ConnectionMetadata = { - /** - * The subscription ID returned by `OnyxUtils.subscribeToKey()` that is associated to this connection. - */ - subscriptionID: number; - - /** - * The Onyx key associated to this connection. - */ - onyxKey: OnyxKey; - - /** - * Whether the first connection's callback was fired or not. - */ - isConnectionMade: boolean; - - /** - * A map of the subscriber's callbacks associated to this connection. - */ - callbacks: Map; - - /** - * The last callback value returned by `OnyxUtils.subscribeToKey()`'s callback. - */ - cachedCallbackValue?: OnyxValue; - - /** - * The last callback key returned by `OnyxUtils.subscribeToKey()`'s callback. - */ - cachedCallbackKey?: OnyxKey; -}; - -/** - * Represents the connection object returned by `Onyx.connect()`. - */ -type Connection = { - /** - * The ID used to identify this particular connection. - */ - id: string; - - /** - * The ID of the subscriber's callback that is associated to this connection. - */ - callbackID: string; -}; - -/** - * Manages Onyx connections of `Onyx.connect()` and `useOnyx()` subscribers. - */ -class OnyxConnectionManager { - /** - * A map where the key is the connection ID generated inside `connect()` and the value is the metadata of that connection. - */ - private connectionsMap: Map; - - /** - * Stores the last generated callback ID which will be incremented when making a new connection. - */ - private lastCallbackID: number; - - /** - * 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; - - constructor() { - this.connectionsMap = new Map(); - this.lastCallbackID = 0; - this.sessionID = Str.guid(); - - // Binds all public methods to prevent problems with `this`. - bindAll(this, 'generateConnectionID', 'fireCallbacks', 'connect', 'disconnect', 'disconnectAll', 'refreshSessionID'); - } - - /** - * Generates a connection ID based on the `connectOptions` object passed to the function. - * - * The properties used to generate the ID are handpicked for performance reasons and - * according to their purpose and effect they produce in the Onyx connection. - */ - private generateConnectionID(connectOptions: ConnectOptions): string { - const {key, reuseConnection} = connectOptions; - - // The current session ID is appended to the connection ID so we can have different connections - // after an `Onyx.clear()` operation. - let suffix = `,sessionID=${this.sessionID}`; - - // We will generate a unique ID when `reuseConnection` is `false`, which means the subscriber - // explicitly wants the connection to not be reused. Collection-root subscriptions are now always - // snapshot mode, so they can be reused like any other connection. - if (reuseConnection === false) { - suffix += `,uniqueID=${Str.guid()}`; - } - - return `onyxKey=${key}${suffix}`; - } - - /** - * Fires all the subscribers callbacks associated with that connection ID. - */ - private fireCallbacks(connectionID: string): void { - const connection = this.connectionsMap.get(connectionID); - if (!connection) { - return; - } - - for (const callback of connection.callbacks.values()) { - try { - if (OnyxKeys.isCollectionKey(connection.onyxKey)) { - (callback as CollectionConnectCallback)(connection.cachedCallbackValue as Record, connection.cachedCallbackKey as OnyxKey); - } else { - (callback as DefaultConnectCallback)(connection.cachedCallbackValue, connection.cachedCallbackKey as OnyxKey); - } - } catch (error) { - Logger.logAlert(`[ConnectionManager] Subscriber callback threw an error for key '${connection.onyxKey}': ${error}`); - } - } - } - - /** - * Connects to an Onyx key given the options passed and listens to its changes. - * - * @param connectOptions The options object that will define the behavior of the connection. - * @returns The connection object to use when calling `disconnect()`. - */ - connect(connectOptions: ConnectOptions): Connection { - const connectionID = this.generateConnectionID(connectOptions); - let connectionMetadata = this.connectionsMap.get(connectionID); - let subscriptionID: number | undefined; - - const callbackID = String(this.lastCallbackID++); - - // If there is no connection yet for that connection ID, we create a new one. - if (!connectionMetadata) { - const callback: ConnectCallback = (value: OnyxValue, key: OnyxKey) => { - const createdConnection = this.connectionsMap.get(connectionID); - if (createdConnection) { - // We signal that the first connection was made and now any new subscribers - // can fire their callbacks immediately with the cached value when connecting. - createdConnection.isConnectionMade = true; - createdConnection.cachedCallbackValue = value; - createdConnection.cachedCallbackKey = key; - this.fireCallbacks(connectionID); - } - }; - - subscriptionID = OnyxUtils.subscribeToKey({ - ...connectOptions, - callback, - } as ConnectOptions); - - connectionMetadata = { - subscriptionID, - onyxKey: connectOptions.key, - isConnectionMade: false, - callbacks: new Map(), - }; - - this.connectionsMap.set(connectionID, connectionMetadata); - } - - // We add the subscriber's callback to the list of callbacks associated with this connection. - if (connectOptions.callback) { - connectionMetadata.callbacks.set(callbackID, connectOptions.callback as ConnectCallback); - } - - // If the first connection is already made we want any new subscribers to receive the cached callback value immediately. - if (connectionMetadata.isConnectionMade) { - // Defer the callback execution to the next tick of the event loop. - // This ensures that the current execution flow completes and the result connection object is available when the callback fires. - Promise.resolve().then(() => { - (connectOptions.callback as DefaultConnectCallback | undefined)?.(connectionMetadata.cachedCallbackValue, connectionMetadata.cachedCallbackKey as OnyxKey); - }); - } - - return {id: connectionID, callbackID}; - } - - /** - * Disconnects and removes the listener from the Onyx key. - * - * @param connection Connection object returned by calling `connect()`. - */ - disconnect(connection: Connection): void { - if (!connection) { - Logger.logInfo(`[ConnectionManager] Attempted to disconnect passing an undefined connection object.`); - return; - } - - const connectionMetadata = this.connectionsMap.get(connection.id); - if (!connectionMetadata) { - Logger.logInfo(`[ConnectionManager] Attempted to disconnect but no connection was found.`); - return; - } - - // Removes the callback from the connection's callbacks map. - connectionMetadata.callbacks.delete(connection.callbackID); - - // If the connection's callbacks map is empty we can safely unsubscribe from the Onyx key. - if (connectionMetadata.callbacks.size === 0) { - OnyxUtils.unsubscribeFromKey(connectionMetadata.subscriptionID); - - this.connectionsMap.delete(connection.id); - } - } - - /** - * Disconnect all subscribers from Onyx. - */ - disconnectAll(): void { - for (const connectionMetadata of this.connectionsMap.values()) { - OnyxUtils.unsubscribeFromKey(connectionMetadata.subscriptionID); - } - - this.connectionsMap.clear(); - - // Clear snapshot cache when all connections are disconnected - onyxSnapshotCache.clear(); - } - - /** - * Refreshes the connection manager's session ID. - */ - refreshSessionID(): void { - this.sessionID = Str.guid(); - - // Clear snapshot cache when session refreshes to avoid stale cache issues - onyxSnapshotCache.clear(); - } -} - -const connectionManager = new OnyxConnectionManager(); - -export default connectionManager; - -export type {Connection}; diff --git a/lib/OnyxSnapshotCache.ts b/lib/OnyxSnapshotCache.ts deleted file mode 100644 index 92c7bba1e..000000000 --- a/lib/OnyxSnapshotCache.ts +++ /dev/null @@ -1,154 +0,0 @@ -import OnyxKeys from './OnyxKeys'; -import type {OnyxKey, OnyxValue} from './types'; -import type {UseOnyxOptions, UseOnyxResult, UseOnyxSelector} from './useOnyx'; - -/** - * Manages snapshot caching for useOnyx hook performance optimization. - * Handles selector function tracking and memoized getSnapshot results. - */ -class OnyxSnapshotCache { - /** - * Snapshot cache is a two-level map. The top-level keys are Onyx keys. The top-level values maps. - * The second-level keys are a custom composite string defined by this.registerConsumer. These represent a unique useOnyx config, which is not fully represented by the Onyx key alone. - * The reason we have two levels is for performance: not to make cache access faster, but to make cache invalidation faster. - * We can invalidate the snapshot cache for a given Onyx key with one map.delete operation on the top-level map, rather than having to loop through a large single-level map and delete any matching keys. - */ - private snapshotCache: Map>>>; - - /** - * Maps selector functions to unique IDs for cache key generation - */ - private selectorIDMap: WeakMap, number>; - - /** - * Counter for generating incremental selector IDs - */ - private selectorIDCounter: number; - - /** - * Reference counting for cache keys to enable automatic cleanup. - * Maps cache key (string) to number of consumers using it. - */ - private cacheKeyRefCounts: Map; - - constructor() { - this.snapshotCache = new Map(); - this.selectorIDMap = new WeakMap(); - this.selectorIDCounter = 0; - this.cacheKeyRefCounts = new Map(); - } - - /** - * Generate unique ID for selector functions using incrementing numbers - */ - getSelectorID(selector: UseOnyxSelector): number { - const typedSelector = selector as unknown as UseOnyxSelector; - if (!this.selectorIDMap.has(typedSelector)) { - const id = this.selectorIDCounter++; - this.selectorIDMap.set(typedSelector, id); - } - return this.selectorIDMap.get(typedSelector)!; - } - - /** - * Register a consumer for a cache key and return the cache key. - * Generates cache key and increments reference counter. - * - * The properties used to generate the cache key are handpicked for performance reasons and - * according to their purpose and effect they produce in the useOnyx hook behavior: - * - * - `selector`: Different selectors produce different results, so each selector needs its own cache entry - * - * Other options like `reuseConnection` 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(key: TKey, options: Pick, 'selector'>): string { - const selectorID = options?.selector ? this.getSelectorID(options.selector) : 'no_selector'; - const cacheKey = `${key}_${selectorID}`; - - // Increment reference count for this cache key - const currentCount = this.cacheKeyRefCounts.get(cacheKey) || 0; - this.cacheKeyRefCounts.set(cacheKey, currentCount + 1); - - return cacheKey; - } - - /** - * Deregister a consumer for a cache key. - * Decrements reference counter and removes cache entry if no consumers remain. - */ - deregisterConsumer(key: OnyxKey, cacheKey: string): void { - const currentCount = this.cacheKeyRefCounts.get(cacheKey) || 0; - - if (currentCount <= 1) { - // Last consumer - remove from reference counter and cache - this.cacheKeyRefCounts.delete(cacheKey); - - // Remove from snapshot cache - const keyCache = this.snapshotCache.get(key); - if (keyCache) { - keyCache.delete(cacheKey); - // If this was the last cache entry for this Onyx key, remove the key entirely - if (keyCache.size === 0) { - this.snapshotCache.delete(key); - } - } - } else { - // Still has other consumers - just decrement count - this.cacheKeyRefCounts.set(cacheKey, currentCount - 1); - } - } - - /** - * Get cached snapshot result for a key and cache key combination - */ - getCachedResult>>(key: OnyxKey, cacheKey: string): TResult | undefined { - const keyCache = this.snapshotCache.get(key); - return keyCache?.get(cacheKey) as TResult | undefined; - } - - /** - * Set cached snapshot result for a key and cache key combination - */ - setCachedResult>>(key: OnyxKey, cacheKey: string, result: TResult): void { - if (!this.snapshotCache.has(key)) { - this.snapshotCache.set(key, new Map()); - } - this.snapshotCache.get(key)!.set(cacheKey, result); - } - - /** - * Selective cache invalidation to prevent data unavailability - * Collection members invalidate upward, collections don't cascade downward - */ - invalidateForKey(keyToInvalidate: OnyxKey): void { - // Always invalidate the exact key - this.snapshotCache.delete(keyToInvalidate); - - // Check if the key is a collection member and invalidate the collection base key - const collectionBaseKey = OnyxKeys.getCollectionKey(keyToInvalidate); - if (collectionBaseKey) { - this.snapshotCache.delete(collectionBaseKey); - } - } - - /** - * Clear all snapshot cache - */ - clear(): void { - this.snapshotCache.clear(); - } - - /** - * Clear selector ID mappings (useful for testing) - */ - clearSelectorIds(): void { - this.selectorIDCounter = 0; - } -} - -// Create and export a singleton instance -const onyxSnapshotCache = new OnyxSnapshotCache(); - -export default onyxSnapshotCache; -export {OnyxSnapshotCache}; diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 400cfbb2b..fd3adfbb2 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1,4 +1,3 @@ -import {shallowEqual} from 'fast-equals'; import type {ValueOf} from 'type-fest'; import _ from 'underscore'; import DevTools from './DevTools'; @@ -7,15 +6,14 @@ import type Onyx from './Onyx'; import cache, {TASK} from './OnyxCache'; import OnyxKeys from './OnyxKeys'; import StorageCircuitBreaker from './StorageCircuitBreaker'; +import onyxStore from './OnyxStore'; +import * as Str from './Str'; import Storage from './storage'; import {StorageErrorClass} from './storage/errors'; import type { CollectionKeyBase, - ConnectOptions, DeepRecord, - DefaultConnectCallback, KeyValueMapping, - CallbackToStateMapping, MultiMergeReplaceNullPatches, OnyxCollection, OnyxEntry, @@ -74,23 +72,11 @@ type PreparedKeyValuePairs = { let mergeQueue: Record>> = {}; let mergeQueuePromise: Record> = {}; -// Holds a mapping of all the React components that want their state subscribed to a store key -let callbackToStateMapping: Record> = {}; - -// Holds a mapping of the connected key to the subscriptionID for faster lookups -let onyxKeyToSubscriptionIDs = new Map(); - // Optional user-provided key value states set when Onyx initializes or clears let defaultKeyStates: Record> = {}; -// Used for comparison with a new update to avoid invoking the Onyx.connect callback with the same data. -let lastConnectionCallbackData = new Map; matchedKey: OnyxKey | undefined}>(); - let snapshotKey: OnyxKey | null = null; -// Keeps track of the last subscriptionID that was used so we can keep incrementing it -let lastSubscriptionID = 0; - // Connections can be made before `Onyx.init`. They would wait for this task before resolving const deferredInitTask = createDeferredTask(); @@ -426,35 +412,6 @@ function tupleGet(keys: Keys): Promise<{[Index }>; } -/** - * Stores a subscription ID associated with a given key. - * - * @param subscriptionID - A subscription ID of the subscriber. - * @param key - A key that the subscriber is subscribed to. - */ -function storeKeyBySubscriptions(key: OnyxKey, subscriptionID: number) { - if (!onyxKeyToSubscriptionIDs.has(key)) { - onyxKeyToSubscriptionIDs.set(key, []); - } - onyxKeyToSubscriptionIDs.get(key).push(subscriptionID); -} - -/** - * Deletes a subscription ID associated with its corresponding key. - * - * @param subscriptionID - The subscription ID to be deleted. - */ -function deleteKeyBySubscriptions(subscriptionID: number) { - const subscriber = callbackToStateMapping[subscriptionID]; - - if (subscriber && onyxKeyToSubscriptionIDs.has(subscriber.key)) { - const updatedSubscriptionsIDs = onyxKeyToSubscriptionIDs.get(subscriber.key).filter((id: number) => id !== subscriptionID); - onyxKeyToSubscriptionIDs.set(subscriber.key, updatedSubscriptionsIDs); - } - - lastConnectionCallbackData.delete(subscriptionID); -} - /** Returns current key names stored in persisted storage */ function getAllKeys(): Promise> { // When we've already read stored keys, resolve right away @@ -553,207 +510,56 @@ function getCachedCollection(collectionKey: TKey } /** - * When a collection of keys change, search for any callbacks matching the collection key and trigger those callbacks - */ -function keysChanged( - collectionKey: TKey, - partialCollection: OnyxCollection, - partialPreviousCollection: OnyxCollection | undefined, -): void { - const cachedCollection = getCachedCollection(collectionKey); - const previousCollection = partialPreviousCollection ?? {}; - const changedMemberKeys = Object.keys(partialCollection ?? {}); - - // Add or remove the keys from the recentlyAccessedKeys list - for (const memberKey of changedMemberKeys) { - const value = partialCollection?.[memberKey]; - if (value !== null && value !== undefined) { - cache.addLastAccessedKey(memberKey, false); - } else { - cache.removeLastAccessedKey(memberKey); - } - } - - // Use indexed lookup instead of scanning all subscribers. - // We need subscribers for: (1) the collection key itself, and (2) individual changed member keys. - const collectionSubscriberIDs = onyxKeyToSubscriptionIDs.get(collectionKey) ?? []; - const memberSubscriberIDs: number[] = []; - for (const memberKey of changedMemberKeys) { - const ids = onyxKeyToSubscriptionIDs.get(memberKey); - if (ids) { - for (const id of ids) { - memberSubscriberIDs.push(id); - } - } - } - - // Notify collection-level subscribers - for (const subID of collectionSubscriberIDs) { - const subscriber = callbackToStateMapping[subID]; - if (!subscriber || typeof subscriber.callback !== 'function') { - continue; - } - - try { - lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection, matchedKey: subscriber.key}); - subscriber.callback(cachedCollection, subscriber.key); - } catch (error) { - Logger.logAlert(`[OnyxUtils.keysChanged] Subscriber callback threw an error for key '${collectionKey}': ${error}`); - } - } - - // Notify member-level subscribers (e.g. subscribed to `report_123`) - for (const subID of memberSubscriberIDs) { - const subscriber = callbackToStateMapping[subID]; - if (!subscriber || typeof subscriber.callback !== 'function') { - continue; - } - - if (cachedCollection[subscriber.key] === previousCollection[subscriber.key]) { - continue; - } - - try { - const subscriberCallback = subscriber.callback as DefaultConnectCallback; - subscriberCallback(cachedCollection[subscriber.key], subscriber.key as TKey); - lastConnectionCallbackData.set(subscriber.subscriptionID, { - value: cachedCollection[subscriber.key], - matchedKey: subscriber.key, - }); - } catch (error) { - Logger.logAlert(`[OnyxUtils.keysChanged] Subscriber callback threw an error for key '${collectionKey}': ${error}`); - } - } -} - -/** - * When a key change happens, search for any callbacks matching the key or collection key and trigger those callbacks + * Notify subscribers of a single-key write. Wrapper over `onyxStore.notifyKey()` + * that also performs LRU bookkeeping for eviction. Write paths call this instead + * of touching the subscriber registry directly. + * + * Pass `suppressCollectionSnapshot: true` when notifying within a collection-batch + * operation — the outer `notifyCollection()` fires snapshot listeners once, so + * each per-key fire shouldn't re-trigger them. */ -function keyChanged(key: TKey, value: OnyxValue, canUpdateSubscriber: (subscriber?: CallbackToStateMapping) => boolean = () => true): void { - // Add or remove this key from the recentlyAccessedKeys list +function notifyKey(key: TKey, value: OnyxValue, options?: {suppressCollectionSnapshot?: boolean}): void { if (value !== null && value !== undefined) { cache.addLastAccessedKey(key, OnyxKeys.isCollectionKey(key)); } else { cache.removeLastAccessedKey(key); } - - // We get the subscribers interested in the key that has just changed. If the subscriber's key is a collection key then we will - // notify them if the key that changed is a collection member. Or if it is a regular key notify them when there is an exact match. - // Given the amount of times this function is called we need to make sure we are not iterating over all subscribers every time. On the other hand, we don't need to - // 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) ?? []; - const collectionKey = OnyxKeys.getCollectionKey(key); - - if (collectionKey) { - // Getting the collection key from the specific key because only collection keys were stored in the mapping. - stateMappingKeys = [...stateMappingKeys, ...(onyxKeyToSubscriptionIDs.get(collectionKey) ?? [])]; - if (stateMappingKeys.length === 0) { - return; - } - } - - // Cache the collection snapshot per dispatch so all subscribers to the same collection - // see a consistent view, even if an earlier subscriber's callback synchronously writes - // to the same collection. - const cachedCollections: Record> = {}; - - for (const stateMappingKey of stateMappingKeys) { - const subscriber = callbackToStateMapping[stateMappingKey]; - if (!subscriber || !OnyxKeys.isKeyMatch(subscriber.key, key) || !canUpdateSubscriber(subscriber)) { - continue; - } - - // Subscriber is a regular call to connect() and provided a callback - if (typeof subscriber.callback === 'function') { - try { - const lastData = lastConnectionCallbackData.get(subscriber.subscriptionID); - if (lastData && lastData.matchedKey === key && lastData.value === value) { - continue; - } - - if (OnyxKeys.isCollectionKey(subscriber.key)) { - // Cache once per dispatch to ensure all subscribers see a consistent snapshot - // even if a previous callback synchronously wrote to the same collection. - let cachedCollection = cachedCollections[subscriber.key]; - if (!cachedCollection) { - cachedCollection = getCachedCollection(subscriber.key); - cachedCollections[subscriber.key] = cachedCollection; - } - lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection, matchedKey: subscriber.key}); - subscriber.callback(cachedCollection, subscriber.key); - continue; - } - - const subscriberCallback = subscriber.callback as DefaultConnectCallback; - subscriberCallback(value, key); - - lastConnectionCallbackData.set(subscriber.subscriptionID, { - value, - matchedKey: key, - }); - continue; - } catch (error) { - Logger.logAlert(`[OnyxUtils.keyChanged] Subscriber callback threw an error for key '${key}': ${error}`); - } - - continue; - } - - console.error('Warning: Found a matching subscriber to a key that changed, but no callback could be found.'); - } + onyxStore.notifyKey(key, value, options); } /** - * Sends the data obtained from the keys to the connection. + * Notify subscribers of a batch collection update. Wrapper over + * `onyxStore.notifyCollection()` that also performs LRU bookkeeping per + * changed member. */ -function sendDataToConnection(mapping: CallbackToStateMapping, matchedKey: TKey | undefined): void { - // If the mapping no longer exists then we should not send any data. - // This means our subscriber was disconnected. - if (!callbackToStateMapping[mapping.subscriptionID]) { - return; - } - - // Always read the latest value from cache to avoid stale or duplicate data. - // For collection-root subscribers, read the full collection. - // For individual key subscribers, read just that key's value. - let value: OnyxValue | undefined; - if (OnyxKeys.isCollectionKey(mapping.key)) { - const collection = getCachedCollection(mapping.key); - value = Object.keys(collection).length > 0 ? (collection as OnyxValue) : undefined; - } else { - value = cache.get(matchedKey ?? mapping.key) as OnyxValue; - } - - // For regular callbacks, we never want to pass null values, but always just undefined if a value is not set in cache or storage. - value = value === null ? undefined : value; - const lastData = lastConnectionCallbackData.get(mapping.subscriptionID); - - // If the value has not changed for the same key we do not need to trigger the callback. - // We compare matchedKey to avoid suppressing callbacks for different collection members - // that happen to have shallow-equal values (e.g. during hydration racing with set()). - if (lastData && lastData.matchedKey === matchedKey && shallowEqual(lastData.value, value)) { - return; +function notifyCollection( + collectionKey: TKey, + partialCollection: OnyxCollection, + partialPreviousCollection?: OnyxCollection, +): void { + const changedKeys = Object.keys(partialCollection ?? {}); + for (const memberKey of changedKeys) { + const value = partialCollection?.[memberKey]; + if (value !== null && value !== undefined) { + cache.addLastAccessedKey(memberKey, false); + } else { + cache.removeLastAccessedKey(memberKey); + } } - - (mapping.callback as DefaultConnectCallback | undefined)?.(value, matchedKey as TKey); -} - -/** - * Gets the data for a given an array of matching keys, combines them into an object, and sends the result back to the subscriber. - */ -function getCollectionDataAndSendAsObject(matchingKeys: CollectionKeyBase[], mapping: CallbackToStateMapping): void { - multiGet(matchingKeys).then(() => { - sendDataToConnection(mapping, mapping.key); - }); + onyxStore.notifyCollection(collectionKey, partialCollection, partialPreviousCollection); } /** - * Remove a key from Onyx and update the subscribers + * Remove a key from Onyx and update the subscribers. + * + * `suppressCollectionSnapshot` skips the collection-level snapshot fire — used by + * `prepareKeyValuePairsForStorage()` when called inside a collection-batch operation + * (setCollection/mergeCollection/partialSetCollection/multiSet's collection batch), + * because the outer `notifyCollection()` fires snapshot listeners once. */ -function remove(key: TKey): Promise { +function remove(key: TKey, options?: {suppressCollectionSnapshot?: boolean}): Promise { cache.drop(key); - keyChanged(key, undefined as OnyxValue); + notifyKey(key, undefined as OnyxValue, options); if (OnyxKeys.isRamOnlyKey(key)) { return Promise.resolve(); @@ -905,7 +711,7 @@ function broadcastUpdate(key: TKey, value: OnyxValue } cache.set(key, value); - keyChanged(key, value); + notifyKey(key, value); } function hasPendingMergeForKey(key: OnyxKey): boolean { @@ -1062,7 +868,7 @@ function initializeWithDefaultKeyStates(): Promise { // Notify subscribers about default key states so that any subscriber that connected // before init (e.g. during module load) receives the merged default values immediately for (const [key, value] of Object.entries(merged ?? {})) { - keyChanged(key, value); + notifyKey(key, value); } }) .catch((error) => { @@ -1080,7 +886,7 @@ function initializeWithDefaultKeyStates(): Promise { // Notify subscribers about default key states so that any subscriber that connected // before init (e.g. during module load) receives the merged default values immediately for (const [key, value] of Object.entries(defaultKeyStates)) { - keyChanged(key, value); + notifyKey(key, value); } }); } @@ -1113,108 +919,6 @@ function doAllCollectionItemsBelongToSameParent( return !hasCollectionKeyCheckFailed; } -/** - * Subscribes to an Onyx key and listens to its changes. - * - * @param connectOptions The options object that will define the behavior of the connection. - * @returns The subscription ID to use when calling `OnyxUtils.unsubscribeFromKey()`. - */ -function subscribeToKey(connectOptions: ConnectOptions): number { - const mapping = connectOptions as CallbackToStateMapping; - const subscriptionID = lastSubscriptionID++; - callbackToStateMapping[subscriptionID] = mapping as CallbackToStateMapping; - callbackToStateMapping[subscriptionID].subscriptionID = subscriptionID; - - // 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. - storeKeyBySubscriptions(mapping.key, callbackToStateMapping[subscriptionID].subscriptionID); - - // Commit connection only after init passes - deferredInitTask.promise - // This first .then() adds a microtask tick for compatibility reasons and - // to ensure subscribers don't receive an extra initial callback before Onyx.update() data arrives. - .then(() => undefined) - .then(() => { - // Performance improvement - // If the mapping is connected to an onyx key that is not a collection - // we can skip the call to getAllKeys() and return an array with a single item - if (!!mapping.key && typeof mapping.key === 'string' && !OnyxKeys.isCollectionKey(mapping.key) && cache.getAllKeys().has(mapping.key)) { - return new Set([mapping.key]); - } - return getAllKeys(); - }) - .then((keys) => { - // We search all the keys in storage to see if any are a "match" for the subscriber we are connecting so that we - // can send data back to the subscriber. Note that multiple keys can match as a subscriber could either be - // subscribed to a "collection key" or a single key. - const matchingKeys: string[] = []; - - // Performance optimization: For single key subscriptions, avoid O(n) iteration - if (!OnyxKeys.isCollectionKey(mapping.key)) { - if (keys.has(mapping.key)) { - matchingKeys.push(mapping.key); - } - } else { - // Collection case - need to iterate through all keys to find matches (O(n)) - for (const key of keys) { - if (!OnyxKeys.isKeyMatch(mapping.key, key)) { - continue; - } - matchingKeys.push(key); - } - } - // If the key being connected to does not exist we initialize the value with null. For subscribers that connected - // directly via connect() they will simply get a null value sent to them without any information about which key matched - // since there are none matched. - if (matchingKeys.length === 0) { - if (mapping.key) { - cache.addNullishStorageKey(mapping.key); - } - - const matchedKey = OnyxKeys.isCollectionKey(mapping.key) ? mapping.key : undefined; - - // Here we cannot use batching because the nullish value is expected to be set immediately for default props - // or they will be undefined. - sendDataToConnection(mapping, matchedKey); - return; - } - - // When using a callback subscriber, a subscription to a collection key combines all matching - // member values into a single object and makes one call with the whole collection object. - if (typeof mapping.callback === 'function') { - if (OnyxKeys.isCollectionKey(mapping.key)) { - getCollectionDataAndSendAsObject(matchingKeys, mapping); - return; - } - - // If we are not subscribed to a collection key then there's only a single key to send an update for. - get(mapping.key).then(() => sendDataToConnection(mapping, mapping.key)); - return; - } - - console.error('Warning: Onyx.connect() was found without a callback'); - }); - - // The subscriptionID is returned back to the caller so that it can be used to clean up the connection when it's no longer needed - // by calling OnyxUtils.unsubscribeFromKey(subscriptionID). - return subscriptionID; -} - -/** - * Disconnects and removes the listener from the Onyx key. - * - * @param subscriptionID Subscription ID returned by calling `OnyxUtils.subscribeToKey()`. - */ -function unsubscribeFromKey(subscriptionID: number): void { - if (!callbackToStateMapping[subscriptionID]) { - return; - } - - deleteKeyBySubscriptions(subscriptionID); - delete callbackToStateMapping[subscriptionID]; -} - function updateSnapshots(data: Array>, mergeFn: typeof Onyx.merge): Array<() => Promise> { const snapshotCollectionKey = getSnapshotKey(); if (!snapshotCollectionKey) return []; @@ -1411,9 +1115,9 @@ function multiSetWithRetry(data: OnyxMultiSetInput, retryAttempt?: number): Prom const keysToRemove = removalCandidates.filter((key) => cache.get(key) !== undefined || persistedKeys.size === 0 || persistedKeys.has(key)); // Group collection members by their parent collection key so each collection can be notified - // via a single batched keysChanged() call instead of one keyChanged() per member. For each + // via a single batched notifyCollection() call instead of one notifyKey() per member. For each // collection, `partial` holds the new values being set and `previous` holds the cached values - // from before the set, which keysChanged() uses to skip subscribers whose value didn't change. + // from before the set, which notifyCollection() uses to skip subscribers whose value didn't change. const collectionBatches = new Map< string, { @@ -1431,7 +1135,7 @@ function multiSetWithRetry(data: OnyxMultiSetInput, retryAttempt?: number): Prom const collectionKey = OnyxKeys.getCollectionKey(key); if (collectionKey && OnyxKeys.isCollectionMemberKey(collectionKey, key)) { - // Capture the previous cached value BEFORE calling cache.set() so keysChanged() + // Capture the previous cached value BEFORE calling cache.set() so notifyCollection() // can diff old vs new per-member. const previousValue = cache.get(key); cache.set(key, value); @@ -1444,14 +1148,13 @@ function multiSetWithRetry(data: OnyxMultiSetInput, retryAttempt?: number): Prom batch.partial[key] = value; batch.previous[key] = previousValue; } else { - // Non-collection keys are notified inline (cache.set + keyChanged in iteration order) + // Non-collection keys are notified inline (cache.set + notifyKey in iteration order) // so re-entrant callbacks (e.g. Onyx.set inside a callback) see consistent cache // and subscriber state, matching the original per-key notification semantics. cache.set(key, value); // Skip subscriber notification on retry — already notified on attempt 0. - // Collection-root subscribers re-fire on every keyChanged by contract. if (!retryAttempt) { - keyChanged(key, value); + notifyKey(key, value); } } } @@ -1473,16 +1176,16 @@ function multiSetWithRetry(data: OnyxMultiSetInput, retryAttempt?: number): Prom batch.previous[key] = previousValue; } else if (!retryAttempt) { // Skip subscriber notification on retry — already notified on attempt 0. - keyChanged(key, undefined); + notifyKey(key, undefined); } } - // One keysChanged() per collection — fires each collection-level subscriber once and lets - // keysChanged() internally decide which individual member subscribers need notification. + // One notifyCollection() per collection — fires each collection-level subscriber once and lets + // notifyCollection() internally decide which individual member subscribers need notification. // Skip on retry — already notified on attempt 0 (see same-reason comment above). if (!retryAttempt) { for (const [collectionKey, batch] of collectionBatches) { - keysChanged(collectionKey as CollectionKeyBase, batch.partial, batch.previous); + notifyCollection(collectionKey as CollectionKeyBase, batch.partial, batch.previous); } } @@ -1565,18 +1268,17 @@ function setCollectionWithRetry({collectionKey, const {pairs: keyValuePairs, keysToRemove: removalCandidates} = OnyxUtils.prepareKeyValuePairsForStorage(mutableCollection, true); // Removals of keys that are neither cached nor persisted are no-ops and skipped. const keysToRemove = removalCandidates.filter((key) => cache.get(key) !== undefined || persistedKeys.has(key)); - // Snapshot before cache mutations so keysChanged() can diff removed members. + // Snapshot before cache mutations so notifyCollection() can diff removed members. const previousCollection = OnyxUtils.getCachedCollection(collectionKey); for (const [key, value] of keyValuePairs) cache.set(key, value); for (const key of keysToRemove) cache.drop(key); // Skip subscriber notification on retry — already notified on attempt 0. - // Collection-root subscribers re-fire on every keysChanged by contract. if (!retryAttempt) { // Removed members are notified as undefined, matching mergeCollection/multiSet. const partialForNotify = Object.fromEntries(Object.entries(mutableCollection).map(([key, value]) => [key, value ?? undefined])); - keysChanged(collectionKey, partialForNotify, previousCollection); + notifyCollection(collectionKey, partialForNotify, previousCollection); } // RAM-only keys are not supposed to be saved to storage @@ -1734,13 +1436,13 @@ function mergeCollectionWithPatches( // promise-chain depth; slow path batches the misses into one Storage.multiGet. const hasColdExistingKey = existingKeys.some((key) => !cache.hasCacheForKey(key)); // Swallow pre-warm read failures so a transient Storage.multiGet rejection doesn't - // skip the cache.merge() + keysChanged() below. Subscribers still see the merge even + // skip the cache.merge() + notifyCollection() below. Subscribers still see the merge even // when storage reads fail. const prewarmPromise = hasColdExistingKey ? multiGet(existingKeys).catch((err) => Logger.logInfo(`mergeCollectionWithPatches pre-warm failed; proceeding with cache-only merge. Error: ${err}`)) : Promise.resolve(); return prewarmPromise.then(() => { - // Snapshot previous values from the (now-warm) cache for keysChanged's diff, then update + // Snapshot previous values from the (now-warm) cache for the subscriber diff, then update // cache and notify subscribers synchronously BEFORE issuing storage writes. This matches // the cache-first / storage-second invariant followed by every other Onyx write method // (setWithRetry, applyMerge, setCollectionWithRetry, partialSetCollection, clear), @@ -1750,12 +1452,11 @@ function mergeCollectionWithPatches( cache.merge(finalMergedCollection); // Skip subscriber notification on retry — already notified on attempt 0. - // Collection-root subscribers re-fire on every keysChanged by contract. if (!retryAttempt) { const partialForNotify = keysToRemove.length > 0 ? {...finalMergedCollection, ...Object.fromEntries(keysToRemove.map((key) => [key, undefined]))} : finalMergedCollection; const previousForNotify = keysToRemove.length > 0 ? {...previousCollection, ...removedPreviousValues} : previousCollection; if (Object.keys(partialForNotify).length > 0) { - keysChanged(collectionKey, partialForNotify, previousForNotify); + notifyCollection(collectionKey, partialForNotify, previousForNotify); } } @@ -1847,18 +1548,17 @@ function partialSetCollection({collectionKey, co const {pairs: keyValuePairs, keysToRemove: removalCandidates} = prepareKeyValuePairsForStorage(mutableCollection, true); // Removals of keys that are neither cached nor persisted are no-ops and skipped. const keysToRemove = removalCandidates.filter((key) => cache.get(key) !== undefined || persistedKeys.has(key)); - // Snapshot before cache mutations so keysChanged() can diff removed members. + // Snapshot before cache mutations so notifyCollection() can diff removed members. const previousCollection = getCachedCollection(collectionKey, existingKeys); for (const [key, value] of keyValuePairs) cache.set(key, value); for (const key of keysToRemove) cache.drop(key); // Skip subscriber notification on retry — already notified on attempt 0. - // Collection-root subscribers re-fire on every keysChanged by contract. if (!retryAttempt) { // Removed members are notified as undefined, matching mergeCollection/multiSet. const partialForNotify = Object.fromEntries(Object.entries(mutableCollection).map(([key, value]) => [key, value ?? undefined])); - keysChanged(collectionKey, partialForNotify, previousCollection); + notifyCollection(collectionKey, partialForNotify, previousCollection); } if (OnyxKeys.isRamOnlyKey(collectionKey)) { @@ -1898,9 +1598,6 @@ function logKeyRemoved(onyxMethod: Extract, key: On function clearOnyxUtilsInternals() { mergeQueue = {}; mergeQueuePromise = {}; - callbackToStateMapping = {}; - onyxKeyToSubscriptionIDs = new Map(); - lastConnectionCallbackData = new Map(); } const OnyxUtils = { @@ -1916,10 +1613,8 @@ const OnyxUtils = { getAllKeys, tryGetCachedValue, getCachedCollection, - keysChanged, - keyChanged, - sendDataToConnection, - getCollectionDataAndSendAsObject, + notifyKey, + notifyCollection, remove, reportStorageQuota, resetDiskPressureLogThrottle, @@ -1935,14 +1630,10 @@ const OnyxUtils = { tupleGet, isValidNonEmptyCollectionForMerge, doAllCollectionItemsBelongToSameParent, - subscribeToKey, - unsubscribeFromKey, getSkippableCollectionMemberIDs, setSkippableCollectionMemberIDs, getSnapshotMergeKeys, setSnapshotMergeKeys, - storeKeyBySubscriptions, - deleteKeyBySubscriptions, reduceCollectionWithSelector, updateSnapshots, mergeCollectionWithPatches, diff --git a/lib/index.ts b/lib/index.ts index bb6df0e0c..671b28a70 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -1,4 +1,4 @@ -import type {ConnectOptions, OnyxUpdate} from './Onyx'; +import type {Connection, ConnectOptions, OnyxUpdate} from './Onyx'; import Onyx from './Onyx'; import type { CustomTypeOptions, @@ -19,7 +19,6 @@ import type { OnyxSetCollectionInput, } from './types'; import type {FetchStatus, ResultMetadata, UseOnyxResult, UseOnyxOptions} from './useOnyx'; -import type {Connection} from './OnyxConnectionManager'; import useOnyx from './useOnyx'; import type {OnyxSQLiteKeyValuePair} from './storage/providers/SQLiteProvider'; diff --git a/lib/types.ts b/lib/types.ts index 96f130813..928376b7c 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -209,16 +209,6 @@ type NullishObjectDeep = { */ type Collection = Record<`${TKey}${string}`, TValue>; -/** Represents the base options used in `Onyx.connect()` method. */ -// NOTE: Any changes to this type like adding or removing options must be accounted in OnyxConnectionManager's `generateConnectionID()` method! -type BaseConnectOptions = { - /** - * If set to `false`, the connection won't be reused between other subscribers that are listening to the same Onyx key - * with the same connect configurations. - */ - reuseConnection?: boolean; -}; - /** Represents the callback function used in `Onyx.connect()` method with a regular key. */ type DefaultConnectCallback = (value: OnyxEntry, key: TKey) => void; @@ -232,20 +222,28 @@ type CollectionConnectCallback = (value: NonUndefined = BaseConnectOptions & { +type ConnectOptions = { /** The Onyx key to subscribe to. */ key: TKey; - /** A function that will be called when the Onyx data we are subscribed changes. */ + /** + * A function that will be called when the Onyx data we are subscribed changes. + * + * The value is a conditional *parameter* (collection snapshot vs. entry) inside a single + * function type — rather than a union of two distinct callback types — so that callers using a + * generic or union `TKey` still get an assignable, non-`any` callback. Collection snapshots stay + * `NonUndefined`. + */ callback?: (value: TKey extends CollectionKeyBase ? NonUndefined> : OnyxEntry, key: TKey) => void; }; -type CallbackToStateMapping = ConnectOptions & { - subscriptionID: number; -}; - /** * Represents a single Onyx input value, that can be either `TOnyxValue` or `null` if the key should be deleted. * This type is used for data passed to Onyx e.g. in `Onyx.merge` and `Onyx.set`. @@ -421,7 +419,6 @@ type MixedOperationsQueue = { }; export type { - BaseConnectOptions, Collection, CollectionConnectCallback, CollectionKey, @@ -435,7 +432,6 @@ export type { InitOptions, Key, KeyValueMapping, - CallbackToStateMapping, NonNull, NonUndefined, OnyxInputKeyValueMapping, diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index c906040f6..859f3160f 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -1,32 +1,25 @@ -import {useCallback, useEffect, useMemo, useRef, useSyncExternalStore} from 'react'; +import {useCallback, useMemo, useRef, useSyncExternalStore} from 'react'; import createMemoizedSelector from './createMemoizedSelector'; -import OnyxCache, {TASK} from './OnyxCache'; -import type {Connection} from './OnyxConnectionManager'; -import connectionManager from './OnyxConnectionManager'; -import OnyxUtils from './OnyxUtils'; -import type {CollectionKeyBase, OnyxKey, OnyxValue} from './types'; -import onyxSnapshotCache from './OnyxSnapshotCache'; -import memoizedShallowEqual from './memoizedShallowEqual'; +import onyxStore from './OnyxStore'; +import type {OnyxKey, OnyxValue} from './types'; type UseOnyxSelector> = (data: OnyxValue | undefined) => TReturnValue; type UseOnyxOptions = { /** - * If set to `false`, the connection won't be reused between other subscribers that are listening to the same Onyx key - * with the same connect configurations. - */ - reuseConnection?: 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 - * when the subset of data changes. Otherwise, any change of data on any property would normally - * cause the component to re-render (and that can be expensive from a performance standpoint). - * @see `useOnyx` cannot return `null` and so selector will replace `null` with `undefined` to maintain compatibility. + * Subscribe to a subset of an Onyx key's data. The component re-renders only when + * the selector's output reference changes; selectors that allocate fresh objects + * (e.g. `(e) => ({id: e?.id})`) are handled by an internal input-cache + deepEqual + * fallback so they don't cause `useSyncExternalStore` to loop. */ selector?: UseOnyxSelector; }; +/** + * Always `'loaded'` in the store-based design. The type is preserved so existing + * destructures like `const [val, {status}] = useOnyx(KEY)` keep compiling. Will be + * removed in a future cleanup once consumers stop reading it. + */ type FetchStatus = 'loading' | 'loaded'; type ResultMetadata = { @@ -35,209 +28,44 @@ type ResultMetadata = { type UseOnyxResult = [NonNullable | undefined, ResultMetadata]; +const LOADED_METADATA: ResultMetadata = {status: 'loaded'}; + +/** + * Subscribes a React component to an Onyx key. The component re-renders when the value + * at `key` changes (for collection keys, when any member changes — the returned value is + * the frozen collection snapshot). + * + * Returns `[value, {status: 'loaded'}]`. With eager-load + the structural-sharing cache, + * there's no loading phase — the cache always has an answer (a value or "absent"). The + * `status` field is retained for API compatibility and is always `'loaded'`. + */ function useOnyx>(key: TKey, options?: UseOnyxOptions): UseOnyxResult { - const connectionRef = useRef(null); const selector = options?.selector; - // Create memoized version of selector for performance. It caches by input reference - // with a deepEqual fallback on the output to keep the returned reference stable. - const memoizedSelector = useMemo((): UseOnyxSelector | null => { - if (!selector) { - return null; - } - - return createMemoizedSelector(selector); - }, [selector]); - - // Stores the previous cached value as it's necessary to compare with the new value in `getSnapshot()`. - // We initialize it to `null` to simulate that we don't have any value from cache yet. - const previousValueRef = useRef(null); - - // Stores the newest cached value in order to compare with the previous one and optimize `getSnapshot()` execution. - const newValueRef = useRef(null); - - // Stores the previously result returned by the hook, containing the data from cache and the fetch status. - // We initialize it to `undefined` and `loading` fetch status to simulate the initial result when the hook is loading from the cache. - const resultRef = useRef>([ - undefined, - { - status: 'loading', - }, - ]); - - // Tracks which key has completed its first Onyx connection callback. When this doesn't match the - // current key, getSnapshot() treats the hook as being in its "first connection" state for that key. - // This is key-aware by design: when the key changes, connectedKeyRef still holds the old key (or null - // after cleanup), so the hook automatically enters first-connection mode for the new key without any - // explicit reset logic — eliminating the race condition where cleanup could clobber a boolean flag. - const connectedKeyRef = useRef(null); - - // Tracks whether the hook has completed its initial mount subscription. - // Unlike connectedKeyRef (which gets nulled by cleanup), this persists across re-subscriptions. - const hasMountedRef = useRef(false); + // The memoized selector is recreated only when the selector function identity changes. + // Inside, it caches by input reference; that's what keeps useSyncExternalStore from + // looping when consumers pass inline-allocating selectors. + const memoizedSelector = useMemo(() => (selector ? createMemoizedSelector(selector) : null), [selector]); - // Indicates if the hook is connecting to an Onyx key. - const isConnectingRef = useRef(false); + const subscribe = useCallback((onStoreChange: () => void) => onyxStore.subscribe(key, onStoreChange), [key]); - // Stores the `onStoreChange()` function, which can be used to trigger a `getSnapshot()` update when desired. - const onStoreChangeFnRef = useRef<(() => void) | null>(null); + // resultRef holds the last tuple returned to React. We return the same tuple reference + // when value hasn't changed so React skips the re-render. + const resultRef = useRef>([undefined, LOADED_METADATA]); - // Indicates if we should get the newest cached value from Onyx during `getSnapshot()` execution. - const shouldGetCachedValueRef = useRef(true); + const getSnapshot = useCallback((): UseOnyxResult => { + const raw = onyxStore.getState(key); + const selected = memoizedSelector ? memoizedSelector(raw as OnyxValue) : (raw as TReturnValue | undefined); + const nextValue = (selected ?? undefined) as NonNullable | undefined; - // Cache the options key to avoid regenerating it every getSnapshot call - const cacheKey = useMemo( - () => - onyxSnapshotCache.registerConsumer(key, { - selector: options?.selector, - }), - [key, options?.selector], - ); - - useEffect(() => () => onyxSnapshotCache.deregisterConsumer(key, cacheKey), [key, cacheKey]); - - // Tracks the last memoizedSelector reference that getSnapshot() has computed with. - // When the selector changes, this mismatch forces getSnapshot() to re-evaluate - // even if all other conditions (isFirstConnection, shouldGetCachedValue, key) are false. - const lastComputedSelectorRef = useRef(memoizedSelector); - - const getSnapshot = useCallback(() => { - // Check if we have any cache for this Onyx key - // Don't use cache during active data updates (when shouldGetCachedValueRef is true) - const isFirstConnection = connectedKeyRef.current !== key; - if (!shouldGetCachedValueRef.current) { - const cachedResult = onyxSnapshotCache.getCachedResult>(key, cacheKey); - if (cachedResult !== undefined) { - // The slot is shared by all subscribers of the same (key, selector) pair, so it can hold a content-equal - // result computed by another subscriber. Keep our own result then, otherwise we would needlessly change - // this hook's result identity and re-render its consumer. - if (cachedResult !== resultRef.current && memoizedShallowEqual(cachedResult[0], resultRef.current[0]) && cachedResult[1].status === resultRef.current[1].status) { - return resultRef.current; - } - resultRef.current = cachedResult; - return cachedResult; - } + if (resultRef.current[0] === nextValue) { + return resultRef.current; } - - // We get the value from cache while the first connection to Onyx is being made or if the key has changed, - // so we can return any cached value right away. For the case where the key has changed, If we don't return the cached value right away, then the UI will show the incorrect (previous) value for a brief period which looks like a UI glitch to the user. After the connection is made, we only - // update `newValueRef` when `Onyx.connect()` callback is fired. - const hasSelectorChanged = lastComputedSelectorRef.current !== memoizedSelector; - if (isFirstConnection || shouldGetCachedValueRef.current || hasSelectorChanged) { - // Gets the value from cache and maps it with selector. It changes `null` to `undefined` for `useOnyx` compatibility. - const value = OnyxUtils.tryGetCachedValue(key) as OnyxValue; - const selectedValue = memoizedSelector ? memoizedSelector(value) : value; - lastComputedSelectorRef.current = memoizedSelector; - newValueRef.current = (selectedValue ?? undefined) as TReturnValue | 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; - } - - const hasCacheForKey = OnyxCache.hasCacheForKey(key); - - // Since the fetch status can be different given the use cases below, we define the variable right away. - let newFetchStatus: FetchStatus | undefined; - - // If we have pending merge operations for the key during the first connection, we set the new value to `undefined` - // and fetch status to `loading` to simulate that it is still being loaded until we have the most updated data. - if (isFirstConnection && OnyxUtils.hasPendingMergeForKey(key)) { - newValueRef.current = undefined; - newFetchStatus = 'loading'; - } - - // shallowEqual checks === first (O(1) for frozen snapshots and stable selector references), - // then falls back to comparing top-level properties for individual keys that may have - // new references with equivalent content. The comparison is memoized by object identity - // (see `memoizedShallowEqual`) so N hooks comparing the same two cache objects pay for - // one walk in total instead of one walk each. - // Normalize null to undefined to ensure consistent comparison (both represent "no value"). - const areValuesEqual = memoizedShallowEqual(previousValueRef.current ?? undefined, newValueRef.current ?? undefined); - - // We update 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 - // OR the subscriber is triggered (the value is gotten from the storage) - // 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) || !isFirstConnection)); - 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. - newFetchStatus = newFetchStatus ?? 'loaded'; - resultRef.current = [ - previousValueRef.current ?? undefined, - { - status: newFetchStatus, - }, - ]; - } - - if (newFetchStatus !== 'loading') { - onyxSnapshotCache.setCachedResult>(key, cacheKey, resultRef.current); - } - + resultRef.current = [nextValue, LOADED_METADATA]; return resultRef.current; - }, [key, memoizedSelector, cacheKey]); - - const subscribe = useCallback( - (onStoreChange: () => void) => { - // Reset internal state so the hook properly transitions through loading - // for the new key instead of preserving stale state from the previous one. - // Only reset when the key has actually changed (not on initial mount). - if (hasMountedRef.current) { - previousValueRef.current = null; - newValueRef.current = null; - resultRef.current = [undefined, {status: 'loading'}]; - shouldGetCachedValueRef.current = true; - } - - hasMountedRef.current = true; - isConnectingRef.current = true; - onStoreChangeFnRef.current = onStoreChange; - - connectionRef.current = connectionManager.connect({ - key, - callback: () => { - isConnectingRef.current = false; - onStoreChangeFnRef.current = onStoreChange; - - // Signals that the first connection was made for this key, so some logics - // in `getSnapshot()` won't be executed anymore. - connectedKeyRef.current = key; - - // Signals that we want to get the newest cached value again in `getSnapshot()`. - shouldGetCachedValueRef.current = true; - - // Invalidate snapshot cache for this key when data changes - onyxSnapshotCache.invalidateForKey(key); - - // Finally, we signal that the store changed, making `getSnapshot()` be called again. - onStoreChange(); - }, - reuseConnection: options?.reuseConnection, - }); - - return () => { - if (!connectionRef.current) { - return; - } - - connectionManager.disconnect(connectionRef.current); - connectedKeyRef.current = null; - isConnectingRef.current = false; - onStoreChangeFnRef.current = null; - }; - }, - [key, options?.reuseConnection], - ); - - const result = useSyncExternalStore>(subscribe, getSnapshot); + }, [key, memoizedSelector]); - return result; + return useSyncExternalStore(subscribe, getSnapshot); } export default useOnyx; diff --git a/tests/perf-test/OnyxConnectionManager.perf-test.ts b/tests/perf-test/OnyxConnectionManager.perf-test.ts deleted file mode 100644 index fc3e5c519..000000000 --- a/tests/perf-test/OnyxConnectionManager.perf-test.ts +++ /dev/null @@ -1,149 +0,0 @@ -import {measureAsyncFunction, measureFunction} from 'reassure'; -import Onyx from '../../lib'; -import type {Connection} from '../../lib/OnyxConnectionManager'; -import connectionManager from '../../lib/OnyxConnectionManager'; -import createDeferredTask from '../../lib/createDeferredTask'; -import {getRandomReportActions} from '../utils/collections/reportActions'; - -const ONYXKEYS = { - TEST_KEY: 'test', - TEST_KEY_2: 'test2', - RAM_ONLY_TEST_KEY: 'ramOnlyTestKey', - COLLECTION: { - TEST_KEY: 'test_', - TEST_NESTED_KEY: 'test_nested_', - TEST_NESTED_NESTED_KEY: 'test_nested_nested_', - TEST_KEY_2: 'test2_', - TEST_KEY_3: 'test3_', - TEST_KEY_4: 'test4_', - TEST_KEY_5: 'test5_', - EVICTABLE_TEST_KEY: 'evictable_test_', - SNAPSHOT: 'snapshot_', - RAM_ONLY_TEST_COLLECTION: 'ramOnlyTestCollection_', - }, -}; - -const collectionKey = ONYXKEYS.COLLECTION.TEST_KEY; -const mockedReportActionsMap = getRandomReportActions(collectionKey); -const mockedReportActionsKeys = Object.keys(mockedReportActionsMap); - -// We need access to some internal properties of `connectionManager` during the tests but they are private, -// so this workaround allows us to have access to them. -// eslint-disable-next-line dot-notation -const generateConnectionID = connectionManager['generateConnectionID']; -// eslint-disable-next-line dot-notation -const fireCallbacks = connectionManager['fireCallbacks']; - -const resetConectionManagerAfterEachMeasure = () => { - connectionManager.disconnectAll(); -}; - -const clearOnyxAfterEachMeasure = async () => { - await Onyx.clear(); -}; - -describe('OnyxConnectionManager', () => { - beforeAll(async () => { - Onyx.init({ - keys: ONYXKEYS, - evictableKeys: [ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY], - skippableCollectionMemberIDs: ['skippable-id'], - ramOnlyKeys: [ONYXKEYS.RAM_ONLY_TEST_KEY, ONYXKEYS.COLLECTION.RAM_ONLY_TEST_COLLECTION], - }); - }); - - describe('generateConnectionID', () => { - test('one call', async () => { - await measureFunction(() => generateConnectionID({key: mockedReportActionsKeys[0]}), { - afterEach: resetConectionManagerAfterEachMeasure, - }); - }); - }); - - describe('fireCallbacks', () => { - test('one call firing 10k callbacks', async () => { - let connectionID = ''; - - await measureFunction(() => fireCallbacks(connectionID), { - beforeEach: async () => { - connectionID = connectionManager.connect({key: mockedReportActionsKeys[0], callback: jest.fn()}).id; - for (let i = 0; i < 9999; i++) { - connectionManager.connect({key: mockedReportActionsKeys[0], callback: jest.fn()}); - } - }, - afterEach: async () => { - resetConectionManagerAfterEachMeasure(); - await clearOnyxAfterEachMeasure(); - }, - }); - }); - }); - - describe('connect', () => { - test('one call', async () => { - await measureAsyncFunction( - async () => { - const callback = createDeferredTask(); - connectionManager.connect({ - key: mockedReportActionsKeys[0], - callback: () => { - callback.resolve?.(); - }, - }); - return callback.promise; - }, - { - afterEach: async () => { - resetConectionManagerAfterEachMeasure(); - await clearOnyxAfterEachMeasure(); - }, - }, - ); - }); - }); - - describe('disconnect', () => { - test('one call', async () => { - let connection: Connection | undefined; - - await measureFunction( - () => { - connectionManager.disconnect(connection as Connection); - }, - { - beforeEach: async () => { - connection = connectionManager.connect({key: mockedReportActionsKeys[0], callback: jest.fn()}); - }, - afterEach: async () => { - resetConectionManagerAfterEachMeasure(); - await clearOnyxAfterEachMeasure(); - }, - }, - ); - }); - }); - - describe('disconnectAll', () => { - test('one call disconnecting 10k connections', async () => { - await measureFunction(() => connectionManager.disconnectAll(), { - beforeEach: async () => { - for (let i = 0; i < 10000; i++) { - connectionManager.connect({key: mockedReportActionsKeys[0], callback: jest.fn()}); - } - }, - afterEach: async () => { - resetConectionManagerAfterEachMeasure(); - await clearOnyxAfterEachMeasure(); - }, - }); - }); - }); - - describe('refreshSessionID', () => { - test('one call', async () => { - await measureFunction(() => connectionManager.refreshSessionID(), { - afterEach: resetConectionManagerAfterEachMeasure, - }); - }); - }); -}); diff --git a/tests/perf-test/OnyxSnapshotCache.perf-test.ts b/tests/perf-test/OnyxSnapshotCache.perf-test.ts deleted file mode 100644 index 02f8ed65d..000000000 --- a/tests/perf-test/OnyxSnapshotCache.perf-test.ts +++ /dev/null @@ -1,249 +0,0 @@ -import {measureFunction} from 'reassure'; -import {OnyxSnapshotCache} from '../../lib/OnyxSnapshotCache'; -import type {UseOnyxOptions, UseOnyxResult, UseOnyxSelector} from '../../lib/useOnyx'; -import type {OnyxKey} from '../../lib'; - -// Define types for test data -type MockData = { - id: number; - name: string; - value: number; - field?: string; -}; - -const ONYXKEYS = { - TEST_KEY: 'test', - TEST_KEY_2: 'test2', - COLLECTION: { - TEST_KEY: 'test_', - TEST_KEY_2: 'test2_', - REPORTS: 'reports_', - }, -}; - -// Mock selector functions -const simpleSelector: UseOnyxSelector = (data) => (data as MockData | undefined)?.value; - -type ComplexSelectorResult = {id?: number; name?: string; computed: number; formatted: string}; -const complexSelector: UseOnyxSelector = (data) => { - const mockData = data as MockData | undefined; - return { - id: mockData?.id, - name: mockData?.name, - computed: mockData?.value ? mockData.value * 2 : 0, - formatted: `${mockData?.name}: ${mockData?.value}`, - }; -}; - -const selectorOptions: UseOnyxOptions = { - selector: simpleSelector, -}; - -const complexSelectorOptions: UseOnyxOptions = { - selector: complexSelector, -}; - -// Mock results -const mockResult: UseOnyxResult = [{id: 1, name: 'Test', value: 42}, {status: 'loaded'}]; - -const mockResults = Array.from({length: 1000}, (_, i): UseOnyxResult => [{id: i, name: `Test${i}`, value: i * 10}, {status: 'loaded'}]); - -describe('OnyxSnapshotCache', () => { - let cache: OnyxSnapshotCache; - - const resetCacheBeforeEachMeasure = () => { - cache = new OnyxSnapshotCache(); - }; - - describe('getSelectorId', () => { - test('getting ID for new selector', async () => { - await measureFunction( - () => { - cache.getSelectorID(simpleSelector); - }, - { - beforeEach: resetCacheBeforeEachMeasure, - }, - ); - }); - - test('getting ID for cached selector (1000 existing selectors)', async () => { - await measureFunction( - () => { - cache.getSelectorID(simpleSelector); - }, - { - beforeEach: () => { - resetCacheBeforeEachMeasure(); - // Pre-populate with 1000 selectors - for (let i = 0; i < 1000; i++) { - const selector: UseOnyxSelector = (data) => ((data as MockData | undefined)?.field ?? '') + i; - cache.getSelectorID(selector); - } - }, - }, - ); - }); - }); - - describe('registerConsumer', () => { - test('generating key for selector options', async () => { - await measureFunction( - () => { - cache.registerConsumer(ONYXKEYS.TEST_KEY, selectorOptions); - }, - { - beforeEach: resetCacheBeforeEachMeasure, - }, - ); - }); - - test('generating key for complex selector options', async () => { - await measureFunction( - () => { - cache.registerConsumer(ONYXKEYS.TEST_KEY, complexSelectorOptions); - }, - { - beforeEach: resetCacheBeforeEachMeasure, - }, - ); - }); - - test('generating 1000 cache keys with different selectors', async () => { - await measureFunction( - () => { - for (let i = 0; i < 1000; i++) { - const selector: UseOnyxSelector = (data) => ((data as MockData | undefined)?.field ?? '') + i; - const options: UseOnyxOptions = {...selectorOptions, selector}; - cache.registerConsumer(ONYXKEYS.TEST_KEY, options); - } - }, - { - beforeEach: resetCacheBeforeEachMeasure, - }, - ); - }); - }); - - describe('getCachedResult', () => { - test('getting cached result (cache hit)', async () => { - const cacheKey = cache.registerConsumer(ONYXKEYS.TEST_KEY, selectorOptions); - await measureFunction( - () => { - cache.getCachedResult(ONYXKEYS.TEST_KEY, cacheKey); - }, - { - beforeEach: () => { - resetCacheBeforeEachMeasure(); - const key = cache.registerConsumer(ONYXKEYS.TEST_KEY, selectorOptions); - cache.setCachedResult(ONYXKEYS.TEST_KEY, key, mockResult); - }, - }, - ); - }); - - test('getting cached result with complex selector (cache hit)', async () => { - const cacheKey = cache.registerConsumer(ONYXKEYS.TEST_KEY, complexSelectorOptions); - const complexResult: UseOnyxResult = [{id: 1, name: 'Test', computed: 84, formatted: 'Test: 42'}, {status: 'loaded'}]; - await measureFunction( - () => { - cache.getCachedResult(ONYXKEYS.TEST_KEY, cacheKey); - }, - { - beforeEach: () => { - resetCacheBeforeEachMeasure(); - const key = cache.registerConsumer(ONYXKEYS.TEST_KEY, complexSelectorOptions); - cache.setCachedResult(ONYXKEYS.TEST_KEY, key, complexResult); - }, - }, - ); - }); - - test('getting cached result with 1000 keys in cache', async () => { - const cacheKey = cache.registerConsumer(ONYXKEYS.TEST_KEY, selectorOptions); - await measureFunction( - () => { - cache.getCachedResult(ONYXKEYS.TEST_KEY, cacheKey); - }, - { - beforeEach: () => { - resetCacheBeforeEachMeasure(); - // Pre-populate cache with 1000 entries - for (let i = 0; i < 1000; i++) { - const key = `test_key_${i}`; - const result = mockResults[i]; - cache.setCachedResult(key, `cache_key_${i}`, result); - } - // Set our target entry - cache.setCachedResult(ONYXKEYS.TEST_KEY, cacheKey, mockResult); - }, - }, - ); - }); - }); - - describe('setCachedResult', () => { - test('setting cached result for new key', async () => { - const cacheKey = cache.registerConsumer(ONYXKEYS.TEST_KEY, selectorOptions); - await measureFunction( - () => { - cache.setCachedResult(ONYXKEYS.TEST_KEY, cacheKey, mockResult); - }, - { - beforeEach: resetCacheBeforeEachMeasure, - }, - ); - }); - - test('setting cached result for existing key', async () => { - const cacheKey = cache.registerConsumer(ONYXKEYS.TEST_KEY, selectorOptions); - await measureFunction( - () => { - cache.setCachedResult(ONYXKEYS.TEST_KEY, cacheKey, mockResult); - }, - { - beforeEach: () => { - resetCacheBeforeEachMeasure(); - // Pre-create the key cache - cache.setCachedResult(ONYXKEYS.TEST_KEY, 'other_cache_key', mockResult); - }, - }, - ); - }); - }); - - describe('invalidateForKey', () => { - test('invalidating single key (cache hit)', async () => { - await measureFunction( - () => { - cache.invalidateForKey(ONYXKEYS.TEST_KEY); - }, - { - beforeEach: () => { - resetCacheBeforeEachMeasure(); - const cacheKey = cache.registerConsumer(ONYXKEYS.TEST_KEY, selectorOptions); - cache.setCachedResult(ONYXKEYS.TEST_KEY, cacheKey, mockResult); - }, - }, - ); - }); - - test('invalidating collection member key', async () => { - const collectionMemberKey = `${ONYXKEYS.COLLECTION.REPORTS}123`; - await measureFunction( - () => { - cache.invalidateForKey(collectionMemberKey); - }, - { - beforeEach: () => { - resetCacheBeforeEachMeasure(); - const cacheKey = cache.registerConsumer(ONYXKEYS.TEST_KEY, selectorOptions); - // Cache both collection and member - cache.setCachedResult(ONYXKEYS.COLLECTION.REPORTS, cacheKey, mockResult); - cache.setCachedResult(collectionMemberKey, cacheKey, mockResult); - }, - }, - ); - }); - }); -}); diff --git a/tests/perf-test/OnyxUtils.perf-test.ts b/tests/perf-test/OnyxUtils.perf-test.ts index 87c7dd7cb..c3a1fa89f 100644 --- a/tests/perf-test/OnyxUtils.perf-test.ts +++ b/tests/perf-test/OnyxUtils.perf-test.ts @@ -7,9 +7,9 @@ import StorageMock from '../../lib/storage'; import OnyxCache from '../../lib/OnyxCache'; import OnyxKeys from '../../lib/OnyxKeys'; import OnyxUtils, {clearOnyxUtilsInternals} from '../../lib/OnyxUtils'; +import onyxStore from '../../lib/OnyxStore'; import type GenericCollection from '../utils/GenericCollection'; import type {OnyxUpdate} from '../../lib/Onyx'; -import createDeferredTask from '../../lib/createDeferredTask'; import type {OnyxEntry, OnyxInputKeyValueMapping, OnyxKey, RetriableOnyxOperation} from '../../lib/types'; const ONYXKEYS = { @@ -298,137 +298,64 @@ describe('OnyxUtils', () => { }); }); - describe('keysChanged', () => { + describe('notifyCollection', () => { test('one call with 10k heavy objects to update 10k subscribers', async () => { - const subscriptionMap = new Map(); + const unsubscribes: Array<() => void> = []; const changedReportActions = Object.fromEntries( Object.entries(mockedReportActionsMap).map(([k, v]) => [k, createRandomReportAction(Number(v.reportActionID))] as const), ) as GenericCollection; - await measureFunction(() => OnyxUtils.keysChanged(collectionKey, changedReportActions, mockedReportActionsMap), { + await measureFunction(() => OnyxUtils.notifyCollection(collectionKey, changedReportActions, mockedReportActionsMap), { beforeEach: async () => { await Onyx.multiSet(mockedReportActionsMap); for (const key of mockedReportActionsKeys) { - const id = OnyxUtils.subscribeToKey({key, callback: jest.fn()}); - subscriptionMap.set(key, id); + unsubscribes.push(onyxStore.subscribe(key, jest.fn())); } }, afterEach: async () => { - for (const key of mockedReportActionsKeys) { - const id = subscriptionMap.get(key); - if (id) { - OnyxUtils.unsubscribeFromKey(id); - } + for (const unsubscribe of unsubscribes) { + unsubscribe(); } - subscriptionMap.clear(); + unsubscribes.length = 0; await clearOnyxAfterEachMeasure(); }, }); }); }); - describe('keyChanged', () => { + describe('notifyKey', () => { test('one call with one heavy object to update 10k subscribers', async () => { - const subscriptionIDs = new Set(); + const unsubscribes: Array<() => void> = []; const key = `${collectionKey}0`; const previousReportAction = mockedReportActionsMap[`${collectionKey}0`]; const changedReportAction = createRandomReportAction(Number(previousReportAction.reportActionID)); - await measureFunction(() => OnyxUtils.keyChanged(key, changedReportAction), { + await measureFunction(() => OnyxUtils.notifyKey(key, changedReportAction), { beforeEach: async () => { await Onyx.set(key, previousReportAction); for (let i = 0; i < 10000; i++) { - const id = OnyxUtils.subscribeToKey({key, callback: jest.fn()}); - subscriptionIDs.add(id); + unsubscribes.push(onyxStore.subscribe(key, jest.fn())); } }, afterEach: async () => { - for (const id of subscriptionIDs) { - OnyxUtils.unsubscribeFromKey(id); + for (const unsubscribe of unsubscribes) { + unsubscribe(); } - subscriptionIDs.clear(); + unsubscribes.length = 0; await clearOnyxAfterEachMeasure(); }, }); }); }); - describe('sendDataToConnection', () => { - test('one call with 10k heavy objects', async () => { - let subscriptionID = -1; - - await measureFunction( - () => - OnyxUtils.sendDataToConnection( - { - key: collectionKey, - subscriptionID, - callback: jest.fn(), - }, - undefined, - ), - { - beforeEach: async () => { - await Onyx.multiSet(mockedReportActionsMap); - subscriptionID = OnyxUtils.subscribeToKey({key: collectionKey, callback: jest.fn()}); - }, - afterEach: async () => { - if (subscriptionID) { - OnyxUtils.unsubscribeFromKey(subscriptionID); - } - await clearOnyxAfterEachMeasure(); - }, - }, - ); - }); - }); - describe('getCollectionKey', () => { test('one call', async () => { await measureFunction(() => OnyxKeys.getCollectionKey(`${ONYXKEYS.COLLECTION.TEST_NESTED_NESTED_KEY}entry1`)); }); }); - describe('getCollectionDataAndSendAsObject', () => { - test('one call with 10k heavy objects', async () => { - let subscriptionID = -1; - - await measureAsyncFunction( - async () => { - const callback = createDeferredTask(); - - subscriptionID = OnyxUtils.subscribeToKey({ - key: collectionKey, - callback: jest.fn(), - }); - - OnyxUtils.getCollectionDataAndSendAsObject(mockedReportActionsKeys, { - key: collectionKey, - subscriptionID, - callback: () => { - callback.resolve?.(); - }, - }); - - return callback.promise; - }, - { - beforeEach: async () => { - await Onyx.multiSet(mockedReportActionsMap); - }, - afterEach: async () => { - if (subscriptionID) { - OnyxUtils.unsubscribeFromKey(subscriptionID); - } - await clearOnyxAfterEachMeasure(); - }, - }, - ); - }); - }); - describe('remove', () => { test('10k calls', async () => { await measureAsyncFunction(() => Promise.all(mockedReportActionsKeys.map((key) => OnyxUtils.remove(key))), { @@ -585,27 +512,20 @@ describe('OnyxUtils', () => { }); }); - describe('subscribeToKey', () => { + describe('onyxStore.subscribe', () => { test('one call subscribing to a single key', async () => { - let subscriptionID = -1; - - await measureAsyncFunction( - async () => { - const callback = createDeferredTask(); - subscriptionID = OnyxUtils.subscribeToKey({ - key: `${collectionKey}0`, - callback: () => { - callback.resolve?.(); - }, - }); - return callback.promise; + let unsubscribe: (() => void) | undefined; + + await measureFunction( + () => { + unsubscribe = onyxStore.subscribe(`${collectionKey}0`, jest.fn()); }, { beforeEach: async () => { await StorageMock.multiSet(Object.entries(mockedReportActionsMap).map(([k, v]) => [k, v])); }, afterEach: async () => { - OnyxUtils.unsubscribeFromKey(subscriptionID); + unsubscribe?.(); await clearOnyxAfterEachMeasure(); }, }, @@ -613,25 +533,18 @@ describe('OnyxUtils', () => { }); test('one call subscribing to a whole collection of 10k heavy objects', async () => { - let subscriptionID = -1; - - await measureAsyncFunction( - async () => { - const callback = createDeferredTask(); - subscriptionID = OnyxUtils.subscribeToKey({ - key: collectionKey, - callback: () => { - callback.resolve?.(); - }, - }); - return callback.promise; + let unsubscribe: (() => void) | undefined; + + await measureFunction( + () => { + unsubscribe = onyxStore.subscribe(collectionKey, jest.fn()); }, { beforeEach: async () => { await StorageMock.multiSet(Object.entries(mockedReportActionsMap).map(([k, v]) => [k, v])); }, afterEach: async () => { - OnyxUtils.unsubscribeFromKey(subscriptionID); + unsubscribe?.(); await clearOnyxAfterEachMeasure(); }, }, @@ -639,16 +552,14 @@ describe('OnyxUtils', () => { }); }); - describe('unsubscribeFromKey', () => { + describe('onyxStore.subscribe unsubscribe', () => { test('one call', async () => { const key = `${collectionKey}0`; - let subscriptionID = -1; + let unsubscribe: (() => void) | undefined; - await measureFunction(() => OnyxUtils.unsubscribeFromKey(subscriptionID), { + await measureFunction(() => unsubscribe?.(), { beforeEach: async () => { - subscriptionID = OnyxUtils.subscribeToKey({ - key, - }); + unsubscribe = onyxStore.subscribe(key, jest.fn()); }, afterEach: clearOnyxAfterEachMeasure, }); @@ -680,46 +591,6 @@ describe('OnyxUtils', () => { }); }); - describe('storeKeyBySubscriptions', () => { - test('one call', async () => { - const key = `${collectionKey}0`; - let subscriptionID = -1; - - await measureFunction(() => OnyxUtils.storeKeyBySubscriptions(key, subscriptionID), { - beforeEach: async () => { - subscriptionID = OnyxUtils.subscribeToKey({ - key, - }); - }, - afterEach: async () => { - OnyxUtils.deleteKeyBySubscriptions(subscriptionID); - OnyxUtils.unsubscribeFromKey(subscriptionID); - await clearOnyxAfterEachMeasure(); - }, - }); - }); - }); - - describe('deleteKeyBySubscriptions', () => { - test('one call', async () => { - const key = `${collectionKey}0`; - let subscriptionID = -1; - - await measureFunction(() => OnyxUtils.deleteKeyBySubscriptions(subscriptionID), { - beforeEach: async () => { - subscriptionID = OnyxUtils.subscribeToKey({ - key, - }); - OnyxUtils.storeKeyBySubscriptions(key, subscriptionID); - }, - afterEach: async () => { - OnyxUtils.unsubscribeFromKey(subscriptionID); - await clearOnyxAfterEachMeasure(); - }, - }); - }); - }); - describe('reduceCollectionWithSelector', () => { test('one call with 10k heavy objects', async () => { const selector = generateTestSelector(); diff --git a/tests/perf-test/useOnyx.perf-test.tsx b/tests/perf-test/useOnyx.perf-test.tsx index ce5488567..6e98c97aa 100644 --- a/tests/perf-test/useOnyx.perf-test.tsx +++ b/tests/perf-test/useOnyx.perf-test.tsx @@ -4,7 +4,6 @@ import {Text, View} from 'react-native'; import {measureRenders} from 'reassure'; import type {FetchStatus, OnyxEntry, OnyxKey, OnyxValue, ResultMetadata, UseOnyxOptions} from '../../lib'; import Onyx, {useOnyx} from '../../lib'; -import StorageMock from '../../lib/storage'; import type {UseOnyxSelector} from '../../lib/useOnyx'; const ONYXKEYS = { @@ -80,23 +79,6 @@ describe('useOnyx', () => { }); }); - /** - * Expected renders: 2. - */ - test('data in storage but not yet in cache', async () => { - const key = ONYXKEYS.TEST_KEY; - await measureRenders(, { - beforeEach: async () => { - await StorageMock.setItem(key, 'test'); - }, - scenario: async () => { - await screen.findByText(dataMatcher(key, 'test')); - await screen.findByText(metadataStatusMatcher(key, 'loaded')); - }, - afterEach: clearOnyxAfterEachMeasure, - }); - }); - /** * Expected renders: 1. */ @@ -197,54 +179,6 @@ describe('useOnyx', () => { }); describe('multiple calls', () => { - /** - * Expected renders: 2. - */ - test('3 calls loading from storage', async () => { - function TestComponent() { - const [testKeyData, testKeyMetadata] = useOnyx(ONYXKEYS.TEST_KEY); - const [testKey2Data, testKey2Metadata] = useOnyx(ONYXKEYS.TEST_KEY_2); - const [testKey3Data, testKey3Metadata] = useOnyx(ONYXKEYS.TEST_KEY_3); - - return ( - - - - - - ); - } - - await measureRenders(, { - beforeEach: async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); - await StorageMock.setItem(ONYXKEYS.TEST_KEY_2, 'test2'); - await StorageMock.setItem(ONYXKEYS.TEST_KEY_3, 'test3'); - }, - scenario: async () => { - await screen.findByText(dataMatcher(ONYXKEYS.TEST_KEY, 'test')); - await screen.findByText(metadataStatusMatcher(ONYXKEYS.TEST_KEY, 'loaded')); - await screen.findByText(dataMatcher(ONYXKEYS.TEST_KEY_2, 'test2')); - await screen.findByText(metadataStatusMatcher(ONYXKEYS.TEST_KEY_2, 'loaded')); - await screen.findByText(dataMatcher(ONYXKEYS.TEST_KEY_3, 'test3')); - await screen.findByText(metadataStatusMatcher(ONYXKEYS.TEST_KEY_3, 'loaded')); - }, - afterEach: clearOnyxAfterEachMeasure, - }); - }); - /** * Expected renders: 1. */ diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts deleted file mode 100644 index 664c96f28..000000000 --- a/tests/unit/OnyxConnectionManagerTest.ts +++ /dev/null @@ -1,468 +0,0 @@ -import {act} from '@testing-library/react-native'; -import Onyx from '../../lib'; -import type {Connection} from '../../lib/OnyxConnectionManager'; -import connectionManager from '../../lib/OnyxConnectionManager'; -import StorageMock from '../../lib/storage'; -import type GenericCollection from '../utils/GenericCollection'; -import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; - -// We need access to some internal properties of `connectionManager` during the tests but they are private, -// so this workaround allows us to have access to them. -// eslint-disable-next-line dot-notation -const connectionsMap = connectionManager['connectionsMap']; -// eslint-disable-next-line dot-notation -const generateConnectionID = connectionManager['generateConnectionID']; -// eslint-disable-next-line dot-notation -const getSessionID = () => connectionManager['sessionID']; - -const ONYXKEYS = { - TEST_KEY: 'test', - TEST_KEY_2: 'test2', - COLLECTION: { - TEST_KEY: 'test_', - TEST_KEY_2: 'test2_', - }, -}; - -Onyx.init({ - keys: ONYXKEYS, -}); - -beforeEach(() => Onyx.clear()); - -describe('OnyxConnectionManager', () => { - // Always use a "fresh" instance - beforeEach(() => { - connectionManager.disconnectAll(); - }); - - describe('generateConnectionID', () => { - it('should generate a stable connection ID', async () => { - const connectionID = generateConnectionID({key: ONYXKEYS.TEST_KEY}); - expect(connectionID).toEqual(`onyxKey=${ONYXKEYS.TEST_KEY},sessionID=${getSessionID()}`); - }); - - it('should generate a stable, reusable connection ID for collection keys', async () => { - const connectionID = generateConnectionID({key: ONYXKEYS.COLLECTION.TEST_KEY}); - expect(connectionID).toEqual(`onyxKey=${ONYXKEYS.COLLECTION.TEST_KEY},sessionID=${getSessionID()}`); - }); - - it('should generate unique connection IDs if certain options are passed', async () => { - const connectionID1 = generateConnectionID({key: ONYXKEYS.TEST_KEY, reuseConnection: false}); - const connectionID2 = generateConnectionID({key: ONYXKEYS.TEST_KEY, reuseConnection: false}); - expect(connectionID1.startsWith(`onyxKey=${ONYXKEYS.TEST_KEY},sessionID=${getSessionID()},uniqueID=`)).toBeTruthy(); - expect(connectionID2.startsWith(`onyxKey=${ONYXKEYS.TEST_KEY},sessionID=${getSessionID()},uniqueID=`)).toBeTruthy(); - expect(connectionID1).not.toEqual(connectionID2); - }); - - it('should generate an unique connection ID if the session ID is changed', async () => { - const connectionID1 = generateConnectionID({key: ONYXKEYS.TEST_KEY}); - connectionManager.refreshSessionID(); - const connectionID2 = generateConnectionID({key: ONYXKEYS.TEST_KEY}); - - expect(connectionID1).not.toEqual(connectionID2); - }); - }); - - describe('connect / disconnect', () => { - it('should connect to a key and fire the callback with its value', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); - - const callback1 = jest.fn(); - const connection = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); - - expect(connectionsMap.has(connection.id)).toBeTruthy(); - - await act(async () => waitForPromisesToResolve()); - - expect(callback1).toHaveBeenCalledTimes(1); - expect(callback1).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); - - connectionManager.disconnect(connection); - - expect(connectionsMap.size).toEqual(0); - }); - - it('should connect two times to the same key and fire both callbacks with its value', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); - - const callback1 = jest.fn(); - const connection1 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); - - const callback2 = jest.fn(); - const connection2 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback2}); - - expect(connection1.id).toEqual(connection2.id); - expect(connectionsMap.size).toEqual(1); - expect(connectionsMap.has(connection1.id)).toBeTruthy(); - - await act(async () => waitForPromisesToResolve()); - - expect(callback1).toHaveBeenCalledTimes(1); - expect(callback1).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); - expect(callback2).toHaveBeenCalledTimes(1); - expect(callback2).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); - - connectionManager.disconnect(connection1); - connectionManager.disconnect(connection2); - - expect(connectionsMap.size).toEqual(0); - }); - - it('should connect two times to the same collection key, reuse the connection, and fire both callbacks with the whole collection object', async () => { - const obj1 = {id: 'entry1_id', name: 'entry1_name'}; - const obj2 = {id: 'entry2_id', name: 'entry2_name'}; - const collection = { - [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: obj1, - [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: obj2, - } as GenericCollection; - await StorageMock.multiSet([ - [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`, obj1], - [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`, obj2], - ]); - - const callback1 = jest.fn(); - const connection1 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback1}); - - const callback2 = jest.fn(); - const connection2 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback2}); - - // Collection-root connections are now always snapshot mode and are reused. - expect(connection1.id).toEqual(connection2.id); - expect(connectionsMap.size).toEqual(1); - expect(connectionsMap.has(connection1.id)).toBeTruthy(); - - await act(async () => waitForPromisesToResolve()); - - // Both subscribers share the connection and receive the whole collection object. - expect(callback1).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY); - expect(callback2).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY); - - connectionManager.disconnect(connection1); - connectionManager.disconnect(connection2); - - expect(connectionsMap.size).toEqual(0); - }); - - it('should connect to a key, connect some times more after first connection is made, and fire all subsequent callbacks immediately with its value', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); - - const callback1 = jest.fn(); - connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); - - await act(async () => waitForPromisesToResolve()); - - expect(callback1).toHaveBeenCalledTimes(1); - expect(callback1).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); - - const callback2 = jest.fn(); - connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback2}); - - const callback3 = jest.fn(); - connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback3}); - - const callback4 = jest.fn(); - connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback4}); - - await act(async () => waitForPromisesToResolve()); - - expect(callback2).toHaveBeenCalledTimes(1); - expect(callback2).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); - expect(callback3).toHaveBeenCalledTimes(1); - expect(callback3).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); - expect(callback4).toHaveBeenCalledTimes(1); - expect(callback4).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); - }); - - it('should have the connection object already defined when triggering the callback of the second connection to the same key', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); - - const callback1 = jest.fn(); - connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); - - await act(async () => waitForPromisesToResolve()); - - expect(callback1).toHaveBeenCalledTimes(1); - expect(callback1).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); - - const callback2 = jest.fn(); - const connection2 = connectionManager.connect({ - key: ONYXKEYS.TEST_KEY, - callback: (...params) => { - callback2(...params); - connectionManager.disconnect(connection2); - }, - }); - - await act(async () => waitForPromisesToResolve()); - - expect(callback2).toHaveBeenCalledTimes(1); - expect(callback2).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); - expect(connectionsMap.size).toEqual(1); - }); - - it('should create a separate connection to the same key when setting reuseConnection to false', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); - - const callback1 = jest.fn(); - const connection1 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); - - const callback2 = jest.fn(); - const connection2 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, reuseConnection: false, callback: callback2}); - - expect(connection1.id).not.toEqual(connection2.id); - expect(connectionsMap.size).toEqual(2); - expect(connectionsMap.has(connection1.id)).toBeTruthy(); - expect(connectionsMap.has(connection2.id)).toBeTruthy(); - }); - - it('should reuse the connection to the same collection key and deliver the whole collection object to all subscribers', async () => { - const collection = { - [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'}, - [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'}, - [`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`]: {id: 'entry3_id', name: 'entry3_name'}, - }; - - Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, collection as GenericCollection); - - await act(async () => waitForPromisesToResolve()); - - const callback1 = jest.fn(); - const connection1 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback1}); - - await act(async () => waitForPromisesToResolve()); - - expect(callback1).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY); - - const callback2 = jest.fn(); - const connection2 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback2}); - - expect(connection1.id).toEqual(connection2.id); - expect(connectionsMap.size).toEqual(1); - expect(connectionsMap.has(connection1.id)).toBeTruthy(); - - await act(async () => waitForPromisesToResolve()); - - expect(callback2).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY); - }); - - it('should not throw any errors when passing an undefined connection or trying to access an inexistent one inside disconnect()', () => { - expect(connectionsMap.size).toEqual(0); - - expect(() => { - connectionManager.disconnect(undefined as unknown as Connection); - }).not.toThrow(); - - expect(() => { - connectionManager.disconnect({id: 'connectionID1', callbackID: 'callbackID1'}); - }).not.toThrow(); - }); - - it('should create a separate connection for the same key after a Onyx.clear() call', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); - - const callback1 = jest.fn(); - connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); - expect(connectionsMap.size).toEqual(1); - - await act(async () => waitForPromisesToResolve()); - - expect(callback1).toHaveBeenCalledTimes(1); - expect(callback1).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); - callback1.mockReset(); - - await act(async () => Onyx.clear()); - - expect(callback1).toHaveBeenCalledTimes(1); - expect(callback1).toHaveBeenCalledWith(undefined, ONYXKEYS.TEST_KEY); - callback1.mockReset(); - - const callback2 = jest.fn(); - connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback2}); - - const callback3 = jest.fn(); - connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback3}); - - // We expect to have two connections for ONYXKEYS.TEST_KEY, one for the first subscription before Onyx.clear(), - // and the other for the two subscriptions with the same key after Onyx.clear(). - expect(connectionsMap.size).toEqual(2); - - await act(async () => waitForPromisesToResolve()); - - expect(callback2).toHaveBeenCalledTimes(1); - expect(callback2).toHaveBeenCalledWith(undefined, undefined); - expect(callback3).toHaveBeenCalledTimes(1); - expect(callback3).toHaveBeenCalledWith(undefined, undefined); - callback1.mockReset(); - callback2.mockReset(); - callback3.mockReset(); - - Onyx.merge(ONYXKEYS.TEST_KEY, 'test2'); - await act(async () => waitForPromisesToResolve()); - - expect(callback1).toHaveBeenCalledTimes(1); - expect(callback1).toHaveBeenCalledWith('test2', ONYXKEYS.TEST_KEY); - expect(callback2).toHaveBeenCalledTimes(1); - expect(callback2).toHaveBeenCalledWith('test2', ONYXKEYS.TEST_KEY); - expect(callback3).toHaveBeenCalledTimes(1); - expect(callback3).toHaveBeenCalledWith('test2', ONYXKEYS.TEST_KEY); - }); - }); - - describe('unsubscribeFromKey', () => { - it('should clean up the correct subscription ID from lastConnectionCallbackData on disconnect', async () => { - const deleteSpy = jest.spyOn(Map.prototype, 'delete'); - - const connectionA = Onyx.connect({key: ONYXKEYS.TEST_KEY, callback: jest.fn(), reuseConnection: false}); - Onyx.connect({key: ONYXKEYS.TEST_KEY, callback: jest.fn(), reuseConnection: false}); - await act(async () => waitForPromisesToResolve()); - - const subscriptionIdA = connectionsMap.get(connectionA.id)?.subscriptionID; - - await Onyx.set(ONYXKEYS.TEST_KEY, 'value1'); - await act(async () => waitForPromisesToResolve()); - - deleteSpy.mockClear(); - Onyx.disconnect(connectionA); - - const numericDeleteArgs = deleteSpy.mock.calls.map((call) => call[0]).filter((arg): arg is number => typeof arg === 'number'); - expect(numericDeleteArgs).toContain(subscriptionIdA); - - deleteSpy.mockRestore(); - }); - - it('should remove the subscription ID from onyxKeyToSubscriptionIDs on disconnect', async () => { - const setSpy = jest.spyOn(Map.prototype, 'set'); - - const connectionA = Onyx.connect({key: ONYXKEYS.TEST_KEY, callback: jest.fn(), reuseConnection: false}); - const connectionB = Onyx.connect({key: ONYXKEYS.TEST_KEY, callback: jest.fn(), reuseConnection: false}); - await act(async () => waitForPromisesToResolve()); - - const subscriptionIdA = connectionsMap.get(connectionA.id)?.subscriptionID; - const subscriptionIdB = connectionsMap.get(connectionB.id)?.subscriptionID; - - setSpy.mockClear(); - Onyx.disconnect(connectionA); - - const setCallsForKey = setSpy.mock.calls.filter((call) => call[0] === ONYXKEYS.TEST_KEY); - expect(setCallsForKey.length).toBeGreaterThan(0); - - const updatedIDs = setCallsForKey[setCallsForKey.length - 1][1] as number[]; - expect(updatedIDs).not.toContain(subscriptionIdA); - expect(updatedIDs).toContain(subscriptionIdB); - - setSpy.mockRestore(); - Onyx.disconnect(connectionB); - }); - }); - - describe('disconnectAll', () => { - it('should disconnect all connections', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); - await StorageMock.setItem(ONYXKEYS.TEST_KEY_2, 'test2'); - - const callback1 = jest.fn(); - const connection1 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); - - const callback2 = jest.fn(); - const connection2 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback2}); - - const callback3 = jest.fn(); - const connection3 = connectionManager.connect({key: ONYXKEYS.TEST_KEY_2, callback: callback3}); - - expect(connection1.id).toEqual(connection2.id); - expect(connectionsMap.size).toEqual(2); - expect(connectionsMap.has(connection1.id)).toBeTruthy(); - expect(connectionsMap.has(connection3.id)).toBeTruthy(); - - await act(async () => waitForPromisesToResolve()); - - connectionManager.disconnectAll(); - - expect(connectionsMap.size).toEqual(0); - }); - }); - - describe('refreshSessionID', () => { - it('should create a separate connection for the same key if the session ID changes', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); - await StorageMock.setItem(ONYXKEYS.TEST_KEY_2, 'test2'); - - const connection1 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: jest.fn()}); - - expect(connectionsMap.size).toEqual(1); - - connectionManager.refreshSessionID(); - - const connection2 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: jest.fn()}); - - expect(connectionsMap.size).toEqual(2); - expect(connectionsMap.has(connection1.id)).toBeTruthy(); - expect(connectionsMap.has(connection2.id)).toBeTruthy(); - }); - }); - - describe('collection callback arguments', () => { - it('should call collection-root callbacks with only the value and key', async () => { - const obj1 = {id: 'entry1_id', name: 'entry1_name'}; - const obj2 = {id: 'entry2_id', name: 'entry2_name'}; - - const callback = jest.fn(); - const connection = connectionManager.connect({ - key: ONYXKEYS.COLLECTION.TEST_KEY, - callback, - }); - - await act(async () => waitForPromisesToResolve()); - - // Initial callback with undefined values - expect(callback).toHaveBeenCalledTimes(1); - expect(callback).toHaveBeenCalledWith(undefined, ONYXKEYS.COLLECTION.TEST_KEY); - - // Reset mock to test the next update - callback.mockReset(); - - // Update with first object - await Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`, obj1); - - expect(callback).toHaveBeenCalledTimes(1); - expect(callback).toHaveBeenCalledWith({[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: obj1}, ONYXKEYS.COLLECTION.TEST_KEY); - - // Reset mock to test the next update - callback.mockReset(); - - // Update with second object - await Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`, obj2); - - expect(callback).toHaveBeenCalledTimes(1); - expect(callback).toHaveBeenCalledWith( - { - [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: obj1, - [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: obj2, - }, - ONYXKEYS.COLLECTION.TEST_KEY, - ); - - connectionManager.disconnect(connection); - }); - - it('should call regular (non-collection) key callbacks with only the value and key', async () => { - const obj1 = {id: 'entry1_id', name: 'entry1_name'}; - - const callback = jest.fn(); - const connection = connectionManager.connect({ - key: ONYXKEYS.TEST_KEY, - callback, - }); - - await act(async () => waitForPromisesToResolve()); - - // Update with object - await Onyx.merge(ONYXKEYS.TEST_KEY, obj1); - - expect(callback).toHaveBeenCalledWith(obj1, ONYXKEYS.TEST_KEY); - - connectionManager.disconnect(connection); - }); - }); -}); diff --git a/tests/unit/OnyxSnapshotCacheTest.ts b/tests/unit/OnyxSnapshotCacheTest.ts deleted file mode 100644 index aa1126462..000000000 --- a/tests/unit/OnyxSnapshotCacheTest.ts +++ /dev/null @@ -1,241 +0,0 @@ -import type {OnyxKey} from '../../lib'; -import {OnyxSnapshotCache} from '../../lib/OnyxSnapshotCache'; -import OnyxKeys from '../../lib/OnyxKeys'; -import type {UseOnyxOptions, UseOnyxResult, UseOnyxSelector} from '../../lib/useOnyx'; - -// Mock OnyxKeys for testing -jest.mock('../../lib/OnyxKeys', () => ({ - isCollectionKey: jest.fn(), - getCollectionKey: jest.fn(), -})); - -const mockedOnyxKeys = OnyxKeys as jest.Mocked; - -// Test types -type TestData = { - data: string; - id?: string; - name?: string; -}; - -type TestResult = UseOnyxResult<{data: string}>; - -type TestSelector = UseOnyxSelector; - -describe('OnyxSnapshotCache', () => { - let cache: OnyxSnapshotCache; - - beforeEach(() => { - cache = new OnyxSnapshotCache(); - jest.clearAllMocks(); - }); - - describe('basic cache operations', () => { - it('should generate unique cache keys for different options', () => { - const selector: TestSelector = (data) => { - const testData = data as TestData | undefined; - return testData?.name ?? ''; - }; - const optionsWithSelector: UseOnyxOptions = { - selector, - }; - const optionsWithoutSelector: UseOnyxOptions = {}; - const keyWithSelector = cache.registerConsumer('testKey', optionsWithSelector); - const keyWithoutSelector = cache.registerConsumer('testKey', optionsWithoutSelector); - const keyWithUndefined = cache.registerConsumer('testKey', {}); - - // Selector cache keys are the selector ID as a string; no-selector consumers share the same key - expect(keyWithSelector).toBe('testKey_0'); - expect(keyWithoutSelector).toBe('testKey_no_selector'); - expect(keyWithUndefined).toBe('testKey_no_selector'); - }); - - it('should generate unique cache keys for different keys', () => { - const key1 = 'testKey1'; - const key2 = 'testKey2'; - const options: UseOnyxOptions = {}; - const key1WithSelector = cache.registerConsumer(key1, options); - const key2WithSelector = cache.registerConsumer(key2, options); - expect(key1WithSelector).toBe(`${key1}_no_selector`); - expect(key2WithSelector).toBe(`${key2}_no_selector`); - }); - - it('should store and retrieve cached results', () => { - const key = 'testKey'; - const cacheKey = 'testCacheKey'; - const result: TestResult = [{data: 'test'}, {status: 'loaded'}]; - - cache.setCachedResult(key, cacheKey, result); - const retrieved = cache.getCachedResult(key, cacheKey); - - expect(retrieved).toEqual(result); - }); - - it('should return undefined for non-existent cache entries', () => { - const result = cache.getCachedResult('nonExistentKey', 'nonExistentCacheKey'); - expect(result).toBeUndefined(); - }); - - it('should clear all caches', () => { - const result1: TestResult = [{data: 'test1'}, {status: 'loaded'}]; - const result2: TestResult = [{data: 'test2'}, {status: 'loaded'}]; - - cache.setCachedResult('key1', 'cacheKey1', result1); - cache.setCachedResult('key2', 'cacheKey2', result2); - - cache.clear(); - - expect(cache.getCachedResult('key1', 'cacheKey1')).toBeUndefined(); - expect(cache.getCachedResult('key2', 'cacheKey2')).toBeUndefined(); - }); - }); - - describe('selector ID management', () => { - it('should generate unique IDs for different selectors', () => { - const nameSelector: TestSelector = (data) => { - const testData = data as TestData | undefined; - return testData?.name ?? ''; - }; - const idSelector: TestSelector = (data) => { - const testData = data as TestData | undefined; - return testData?.id ?? ''; - }; - - const nameId = cache.getSelectorID(nameSelector); - const idSelectorId = cache.getSelectorID(idSelector); - - // Different selectors should get different IDs - expect(nameId).not.toBe(idSelectorId); - }); - - it('should return the same ID for the same selector function', () => { - const selector: TestSelector = (data) => { - const testData = data as TestData | undefined; - return testData?.name ?? ''; - }; - - const firstCall = cache.getSelectorID(selector); - const secondCall = cache.getSelectorID(selector); - const thirdCall = cache.getSelectorID(selector); - - // Multiple calls with same selector should return identical ID - expect(firstCall).toBe(secondCall); - expect(secondCall).toBe(thirdCall); - }); - - it('should clear selector IDs and reset counter', () => { - const selector1: TestSelector = (data) => { - const testData = data as TestData | undefined; - return testData?.name ?? ''; - }; - const selector2: TestSelector = (data) => { - const testData = data as TestData | undefined; - return testData?.id ?? ''; - }; - - // Clear the selector IDs - cache.clearSelectorIds(); - - // After clearing, selectors should get new IDs starting from 0 - const id1After = cache.getSelectorID(selector1); - const id2After = cache.getSelectorID(selector2); - - expect(id1After).toBe(0); // First selector after clear should get ID 0 - expect(id2After).toBe(1); // Second selector should get ID 1 - }); - }); - - describe('cache invalidation', () => { - beforeEach(() => { - // Set up cache with multiple entries - cache.setCachedResult('reports_', 'cache1', [{data: 'collection'}, {status: 'loaded'}]); - cache.setCachedResult('reports_123', 'cache2', [{data: 'member1'}, {status: 'loaded'}]); - cache.setCachedResult('reports_456', 'cache3', [{data: 'member2'}, {status: 'loaded'}]); - cache.setCachedResult('users_', 'cache4', [{data: 'users collection'}, {status: 'loaded'}]); - cache.setCachedResult('users_789', 'cache5', [{data: 'user member'}, {status: 'loaded'}]); - cache.setCachedResult('nonCollectionKey', 'cache6', [{data: 'regular key'}, {status: 'loaded'}]); - }); - - it('should invalidate non-collection keys without affecting others', () => { - mockedOnyxKeys.isCollectionKey.mockReturnValue(false); - mockedOnyxKeys.getCollectionKey.mockReturnValue(undefined); - - cache.invalidateForKey('nonCollectionKey'); - - // Non-collection key should be invalidated - expect(cache.getCachedResult('nonCollectionKey', 'cache6')).toBeUndefined(); - - // All other keys should remain - expect(cache.getCachedResult('reports_', 'cache1')).toBeDefined(); - expect(cache.getCachedResult('reports_123', 'cache2')).toBeDefined(); - expect(cache.getCachedResult('reports_456', 'cache3')).toBeDefined(); - expect(cache.getCachedResult('users_', 'cache4')).toBeDefined(); - expect(cache.getCachedResult('users_789', 'cache5')).toBeDefined(); - }); - - it('should invalidate collection member key and its base collection only', () => { - mockedOnyxKeys.isCollectionKey.mockReturnValue(true); - mockedOnyxKeys.getCollectionKey.mockReturnValue('reports_'); - - cache.invalidateForKey('reports_123'); - - // Collection member and base should be invalidated - expect(cache.getCachedResult('reports_123', 'cache2')).toBeUndefined(); - expect(cache.getCachedResult('reports_', 'cache1')).toBeUndefined(); - - // Other collection members should remain (selective invalidation) - expect(cache.getCachedResult('reports_456', 'cache3')).toBeDefined(); - - // Unrelated keys should remain - expect(cache.getCachedResult('users_', 'cache4')).toBeDefined(); - expect(cache.getCachedResult('users_789', 'cache5')).toBeDefined(); - expect(cache.getCachedResult('nonCollectionKey', 'cache6')).toBeDefined(); - }); - - it('should invalidate collection base key without cascading to members', () => { - mockedOnyxKeys.isCollectionKey.mockReturnValue(true); - mockedOnyxKeys.getCollectionKey.mockReturnValue('reports_'); - - // When base key equals the key to invalidate, it's a collection base key - cache.invalidateForKey('reports_'); - - // Only the base collection should be invalidated - expect(cache.getCachedResult('reports_', 'cache1')).toBeUndefined(); - - // Collection members should remain (no cascade deletion) - expect(cache.getCachedResult('reports_123', 'cache2')).toBeDefined(); - expect(cache.getCachedResult('reports_456', 'cache3')).toBeDefined(); - - // Unrelated keys should remain - expect(cache.getCachedResult('users_', 'cache4')).toBeDefined(); - expect(cache.getCachedResult('users_789', 'cache5')).toBeDefined(); - expect(cache.getCachedResult('nonCollectionKey', 'cache6')).toBeDefined(); - }); - - it('should handle multiple different collection keys independently', () => { - // Invalidate reports collection member - mockedOnyxKeys.isCollectionKey.mockReturnValueOnce(true); - mockedOnyxKeys.getCollectionKey.mockReturnValueOnce('reports_'); - cache.invalidateForKey('reports_123'); - - // Invalidate users collection member - mockedOnyxKeys.isCollectionKey.mockReturnValueOnce(true); - mockedOnyxKeys.getCollectionKey.mockReturnValueOnce('users_'); - cache.invalidateForKey('users_789'); - - // Reports: member and base should be invalidated - expect(cache.getCachedResult('reports_123', 'cache2')).toBeUndefined(); - expect(cache.getCachedResult('reports_', 'cache1')).toBeUndefined(); - - // Users: member and base should be invalidated - expect(cache.getCachedResult('users_789', 'cache5')).toBeUndefined(); - expect(cache.getCachedResult('users_', 'cache4')).toBeUndefined(); - - // Other collection members should remain - expect(cache.getCachedResult('reports_456', 'cache3')).toBeDefined(); - - // Non-collection keys should remain - expect(cache.getCachedResult('nonCollectionKey', 'cache6')).toBeDefined(); - }); - }); -}); diff --git a/tests/unit/collectionHydrationTest.ts b/tests/unit/collectionHydrationTest.ts index 9a77cc9db..1713c1fdc 100644 --- a/tests/unit/collectionHydrationTest.ts +++ b/tests/unit/collectionHydrationTest.ts @@ -26,7 +26,7 @@ describe('Collection hydration with connect() followed by immediate set()', () = afterEach(() => Onyx.clear()); - test('collection connect should deliver full collection from storage', async () => { + test('waitForCollectionCallback=true should deliver full collection from storage', async () => { const mockCallback = jest.fn(); // A component connects to the collection (starts async hydration via multiGet). diff --git a/tests/unit/onyxCacheTest.tsx b/tests/unit/onyxCacheTest.tsx index e7a3bc7af..e95816aa4 100644 --- a/tests/unit/onyxCacheTest.tsx +++ b/tests/unit/onyxCacheTest.tsx @@ -599,11 +599,16 @@ describe('Onyx', () => { expect(Object.keys(first!)).toHaveLength(0); }); - it('should return undefined for empty collections when no keys are loaded', async () => { + it('should return the frozen-empty snapshot for empty collections once init has registered the collection key', async () => { await initOnyx(); + // Post-init, a known collection key with no members resolves to the frozen + // empty snapshot — not `undefined`. Returning `{}` reliably across init, + // writes, and `Onyx.clear()` keeps `Onyx.connect({waitForCollectionCallback: true})` + // subscribers seeing a consistent "collection is empty" signal instead of + // mistakenly skipping the update. const result = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); - expect(result).toBeUndefined(); + expect(result).toEqual({}); }); it('should return a new reference when a member is removed and another added simultaneously', async () => { diff --git a/tests/unit/onyxClearNativeStorageTest.ts b/tests/unit/onyxClearNativeStorageTest.ts index 902445870..a069d406d 100644 --- a/tests/unit/onyxClearNativeStorageTest.ts +++ b/tests/unit/onyxClearNativeStorageTest.ts @@ -2,7 +2,7 @@ import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; import StorageMock from '../../lib/storage'; import Onyx from '../../lib/Onyx'; import type OnyxCache from '../../lib/OnyxCache'; -import type {Connection} from '../../lib/OnyxConnectionManager'; +import type {Connection} from '../../lib/Onyx'; const ONYX_KEYS = { DEFAULT_KEY: 'defaultKey', diff --git a/tests/unit/onyxClearWebStorageTest.ts b/tests/unit/onyxClearWebStorageTest.ts index bd699b2df..b1a9cf8d7 100644 --- a/tests/unit/onyxClearWebStorageTest.ts +++ b/tests/unit/onyxClearWebStorageTest.ts @@ -3,7 +3,7 @@ import StorageMock from '../../lib/storage'; import Onyx from '../../lib/Onyx'; import type OnyxCache from '../../lib/OnyxCache'; import type GenericCollection from '../utils/GenericCollection'; -import type {Connection} from '../../lib/OnyxConnectionManager'; +import type {Connection} from '../../lib/Onyx'; const ONYX_KEYS = { DEFAULT_KEY: 'defaultKey', @@ -238,8 +238,9 @@ describe('Set data while storage is clearing', () => { // 3. clear() expect(collectionCallback).toHaveBeenCalledTimes(3); - // And it should be called with the expected parameters each time - expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST); + // And it should be called with the expected parameters each time. Initial fire + // delivers `{}` (legacy `undefined`-for-empty-initial shim was removed). + expect(collectionCallback).toHaveBeenNthCalledWith(1, {}, ONYX_KEYS.COLLECTION.TEST); expect(collectionCallback).toHaveBeenNthCalledWith( 2, { diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index a36c79ec2..824658854 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -4,12 +4,11 @@ import lodashClone from 'lodash/clone'; import lodashCloneDeep from 'lodash/cloneDeep'; import type OnyxCache from '../../lib/OnyxCache'; -import type {Connection} from '../../lib/OnyxConnectionManager'; import type {OnyxCollection, OnyxKey, OnyxUpdate} from '../../lib/types'; import type {GenericDeepRecord} from '../types'; import type GenericCollection from '../utils/GenericCollection'; - import Onyx from '../../lib'; +import type {Connection} from '../../lib/Onyx'; import createDeferredTask from '../../lib/createDeferredTask'; import * as Logger from '../../lib/Logger'; import OnyxUtils from '../../lib/OnyxUtils'; @@ -644,21 +643,6 @@ describe('Onyx', () => { }); }); - it('should overwrite an array key nested inside an object', () => { - let testKeyValue: unknown; - connection = Onyx.connect({ - key: ONYX_KEYS.TEST_KEY, - callback: (value) => { - testKeyValue = value; - }, - }); - - Onyx.merge(ONYX_KEYS.TEST_KEY, {something: [1, 2, 3]}); - return Onyx.merge(ONYX_KEYS.TEST_KEY, {something: [4]}).then(() => { - expect(testKeyValue).toEqual({something: [4]}); - }); - }); - it('should properly set and merge when using mergeCollection', async () => { const mockCallback = jest.fn(); connection = Onyx.connect({ @@ -936,8 +920,8 @@ describe('Onyx', () => { return waitForPromisesToResolve(); }) .then(() => { - // The collection callback receives the whole collection object. - expect(mockCallback.mock.calls[mockCallback.mock.calls.length - 1][0]).toEqual({test_1: {existingData: 'test'}, test_2: {existingData: 'test'}}); + // Snapshot mode: multiSet fires the collection callback per write. + expect(mockCallback).toHaveBeenLastCalledWith({test_1: {existingData: 'test'}, test_2: {existingData: 'test'}}, ONYX_KEYS.COLLECTION.TEST_KEY); mockCallback.mockReset(); // When we pass a mergeCollection data object to Onyx.update @@ -965,12 +949,14 @@ describe('Onyx', () => { .then(() => { // mergeCollection fires the collection object once with all 3 merged members. expect(mockCallback).toHaveBeenCalledTimes(1); - expect(mockCallback.mock.calls[0][0]).toEqual({ - test_1: {ID: 123, value: 'one', existingData: 'test'}, - test_2: {ID: 234, value: 'two', existingData: 'test'}, - test_3: {ID: 345, value: 'three'}, - }); - expect(mockCallback.mock.calls[0][1]).toBe(ONYX_KEYS.COLLECTION.TEST_KEY); + expect(mockCallback).toHaveBeenCalledWith( + { + test_1: {ID: 123, value: 'one', existingData: 'test'}, + test_2: {ID: 234, value: 'two', existingData: 'test'}, + test_3: {ID: 345, value: 'three'}, + }, + ONYX_KEYS.COLLECTION.TEST_KEY, + ); }); }); @@ -1020,7 +1006,7 @@ describe('Onyx', () => { }); }); - it('should return all collection keys as a single object', () => { + it('should return all collection keys as a single object when waitForCollectionCallback = true', () => { const mockCallback = jest.fn(); // Given some initial collection data @@ -1041,7 +1027,7 @@ describe('Onyx', () => { return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_CONNECT_COLLECTION, initialCollectionData as GenericCollection) .then(() => { - // When we connect to that collection + // When we connect to that collection with waitForCollectionCallback = true connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_CONNECT_COLLECTION, callback: mockCallback, @@ -1055,14 +1041,14 @@ describe('Onyx', () => { }); }); - it('should return all collection keys as a single object when updating a collection key', () => { + it('should return all collection keys as a single object when updating a collection key with waitForCollectionCallback = true', () => { const mockCallback = jest.fn(); const collectionUpdate = { testPolicy_1: {ID: 234, value: 'one'}, testPolicy_2: {ID: 123, value: 'two'}, }; - // Given an Onyx.connect call to a collection key + // Given an Onyx.connect call with waitForCollectionCallback=true connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_POLICY, callback: mockCallback, @@ -1075,8 +1061,10 @@ describe('Onyx', () => { // Then we expect the callback to have called twice, once for the initial connect call + once for the collection update expect(mockCallback).toHaveBeenCalledTimes(2); - // AND the value for the first call should be null since the collection was not initialized at that point - expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST_POLICY); + // Initial fire delivers the post-init frozen empty collection `{}` (the legacy + // "undefined for empty-on-initial-fire" shim was removed; callers that needed + // that behavior now guard at the consumer level). + expect(mockCallback).toHaveBeenNthCalledWith(1, {}, ONYX_KEYS.COLLECTION.TEST_POLICY); // AND the value for the second call should be collectionUpdate since the collection was updated expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY); @@ -1091,7 +1079,7 @@ describe('Onyx', () => { testPolicy_2: {ID: 123, value: 'two'}, }; - // Given an Onyx.connect call subscribing to a single collection member key + // Given an Onyx.connect call with waitForCollectionCallback=false connection = Onyx.connect({ key: `${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, callback: mockCallback, @@ -1104,8 +1092,10 @@ describe('Onyx', () => { // Then we expect the callback to have called twice, once for the initial connect call + once for the collection update expect(mockCallback).toHaveBeenCalledTimes(2); - // AND the value for the first call should be null since the collection was not initialized at that point - expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, undefined); + // Initial fire delivers `(undefined, key)` — the cache has no entry for + // `testPolicy_1` yet, but we still pass the key. (Legacy `(undefined, undefined)` + // no-match shim was removed.) + expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, 'testPolicy_1'); // AND the value for the second call should be collectionUpdate since the collection was updated expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate.testPolicy_1, 'testPolicy_1'); @@ -1113,13 +1103,13 @@ describe('Onyx', () => { ); }); - it('should return all collection keys as a single object when a single collection member key is updated', () => { + it('should return all collection keys as a single object for subscriber using waitForCollectionCallback when a single collection member key is updated', () => { const mockCallback = jest.fn(); const collectionUpdate = { testPolicy_1: {ID: 234, value: 'one'}, }; - // Given an Onyx.connect call to a collection key + // Given an Onyx.connect call with waitForCollectionCallback=true connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_POLICY, callback: mockCallback, @@ -1132,8 +1122,8 @@ describe('Onyx', () => { // Then we expect the callback to have called twice, once for the initial connect call + once for the collection update expect(mockCallback).toHaveBeenCalledTimes(2); - // AND the value for the second call should be collectionUpdate - expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST_POLICY); + // Initial fire delivers `{}` (legacy `undefined`-for-empty-initial shim was removed). + expect(mockCallback).toHaveBeenNthCalledWith(1, {}, ONYX_KEYS.COLLECTION.TEST_POLICY); expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY); }) ); @@ -1154,7 +1144,7 @@ describe('Onyx', () => { testPolicy_1: {ID: 234, value: 'one'}, }; - // Given an Onyx.connect call to a collection key + // Given an Onyx.connect call with waitForCollectionCallback=true connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_POLICY, callback: mockCallback, @@ -1205,11 +1195,13 @@ describe('Onyx', () => { {onyxMethod: Onyx.METHOD.MERGE_COLLECTION, key: ONYX_KEYS.COLLECTION.TEST_UPDATE, value: {[itemKey]: {a: 'a'}} as GenericCollection}, ]).then(() => { expect(collectionCallback).toHaveBeenCalledTimes(2); - expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST_UPDATE); + // Initial fire delivers `{}` (legacy `undefined`-for-empty-initial shim was removed). + expect(collectionCallback).toHaveBeenNthCalledWith(1, {}, ONYX_KEYS.COLLECTION.TEST_UPDATE); expect(collectionCallback).toHaveBeenNthCalledWith(2, {[itemKey]: {a: 'a'}}, ONYX_KEYS.COLLECTION.TEST_UPDATE); expect(testCallback).toHaveBeenCalledTimes(2); - expect(testCallback).toHaveBeenNthCalledWith(1, undefined, undefined); + // Initial fire delivers `(undefined, key)` — cache has no entry yet, but we still pass the key. + expect(testCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.TEST_KEY); expect(testCallback).toHaveBeenNthCalledWith(2, 'taco', ONYX_KEYS.TEST_KEY); expect(otherTestCallback).toHaveBeenCalledTimes(2); @@ -1468,8 +1460,12 @@ describe('Onyx', () => { // Cat hasn't changed from its original value, expect only the initial connect callback expect(catCallback).toHaveBeenCalledTimes(1); - // Dog was modified, expect the initial connect callback and the mergeCollection callback - expect(dogCallback).toHaveBeenCalledTimes(2); + // Dog was created by the merge. Onyx writes cache-first/storage-second, so the + // mergeCollection notification reaches the subscriber before the initial connect + // fire; the initial fire then reads the already-merged value and is deduped. The + // subscriber therefore receives the final value once, never the transient undefined. + expect(dogCallback).toHaveBeenCalledTimes(1); + expect(dogCallback).toHaveBeenLastCalledWith({name: 'Rex'}, dog); connections.map((id) => Onyx.disconnect(id)); }); @@ -1496,12 +1492,10 @@ describe('Onyx', () => { await Onyx.update([{key: cat, value: finalValue, onyxMethod: Onyx.METHOD.MERGE}]); - // The SNAPSHOT collection-root subscriber receives the whole collection. + // Snapshot mode: callback fires with the whole SNAPSHOT-collection snapshot. expect(callback).toBeCalledTimes(2); - expect(callback.mock.calls[0][0]).toEqual({[snapshot1]: {data: {[cat]: initialValue}}}); - expect(callback.mock.calls[0][1]).toBe(ONYX_KEYS.COLLECTION.SNAPSHOT); - expect(callback.mock.calls[1][0]).toEqual({[snapshot1]: {data: {[cat]: finalValue}}}); - expect(callback.mock.calls[1][1]).toBe(ONYX_KEYS.COLLECTION.SNAPSHOT); + expect(callback).toHaveBeenNthCalledWith(1, {[snapshot1]: {data: {[cat]: initialValue}}}, ONYX_KEYS.COLLECTION.SNAPSHOT); + expect(callback).toHaveBeenNthCalledWith(2, {[snapshot1]: {data: {[cat]: finalValue}}}, ONYX_KEYS.COLLECTION.SNAPSHOT); }); it('should merge allowlisted keys into Snapshot even if they were missing', async () => { @@ -1530,12 +1524,14 @@ describe('Onyx', () => { await Onyx.update([{key: cat, value: finalValue, onyxMethod: Onyx.METHOD.MERGE}]); - // The SNAPSHOT collection-root subscriber receives the whole collection. + // Snapshot mode: callback fires with the whole SNAPSHOT-collection snapshot. expect(callback).toBeCalledTimes(2); - expect(callback.mock.calls[0][0]).toEqual({[snapshot1]: {data: {[cat]: initialValue}}}); - expect(callback.mock.calls[0][1]).toBe(ONYX_KEYS.COLLECTION.SNAPSHOT); - expect(callback.mock.calls[1][0]).toEqual({[snapshot1]: {data: {[cat]: {name: 'Kitty', pendingAction: 'delete', pendingFields: {preview: 'delete'}}}}}); - expect(callback.mock.calls[1][1]).toBe(ONYX_KEYS.COLLECTION.SNAPSHOT); + expect(callback).toHaveBeenNthCalledWith(1, {[snapshot1]: {data: {[cat]: initialValue}}}, ONYX_KEYS.COLLECTION.SNAPSHOT); + expect(callback).toHaveBeenNthCalledWith( + 2, + {[snapshot1]: {data: {[cat]: {name: 'Kitty', pendingAction: 'delete', pendingFields: {preview: 'delete'}}}}}, + ONYX_KEYS.COLLECTION.SNAPSHOT, + ); }); describe('update', () => { @@ -1622,6 +1618,11 @@ describe('Onyx', () => { }, }, ]).then(() => { + // Initial fire is deferred past in-flight writes via `scheduleInitialFire`, + // so it reads the post-update snapshot. The write-driven fire already + // delivered the same snapshot, so the dedup in `deliverSnapshot` suppresses + // the initial fire — matching legacy timing. + expect(routesCollectionCallback).toHaveBeenCalledTimes(1); expect(routesCollectionCallback).toHaveBeenNthCalledWith( 1, { @@ -1702,19 +1703,16 @@ describe('Onyx', () => { {onyxMethod: Onyx.METHOD.MERGE, key: lisa, value: {car: 'SUV', age: 21}}, {onyxMethod: Onyx.METHOD.MERGE, key: bob, value: {age: 25}}, ]).then(() => { - expect(testCallback).toHaveBeenNthCalledWith(1, {food: 'taco', drink: 'wine'}, ONYX_KEYS.TEST_KEY); + // The store-based wrapper always fires an initial callback before the + // post-update callback (the legacy ConnectionManager's deep-promise chain + // suppressed it accidentally). We assert on the final post-update call + // via `toHaveBeenLastCalledWith` instead of pinning specific indices. + // The `sourceValue` 3rd argument was also dropped. + expect(testCallback).toHaveBeenLastCalledWith({food: 'taco', drink: 'wine'}, ONYX_KEYS.TEST_KEY); - expect(otherTestCallback).toHaveBeenNthCalledWith(1, {food: 'pizza', drink: 'water'}, ONYX_KEYS.OTHER_TEST); + expect(otherTestCallback).toHaveBeenLastCalledWith({food: 'pizza', drink: 'water'}, ONYX_KEYS.OTHER_TEST); - expect(animalsCollectionCallback).toHaveBeenNthCalledWith( - 1, - { - [cat]: {age: 3, sound: 'meow'}, - }, - ONYX_KEYS.COLLECTION.ANIMALS, - ); - expect(animalsCollectionCallback).toHaveBeenNthCalledWith( - 2, + expect(animalsCollectionCallback).toHaveBeenLastCalledWith( { [cat]: {age: 3, sound: 'meow'}, [dog]: {size: 'M', sound: 'woof'}, @@ -1722,10 +1720,9 @@ describe('Onyx', () => { ONYX_KEYS.COLLECTION.ANIMALS, ); - expect(catCallback).toHaveBeenNthCalledWith(1, {age: 3, sound: 'meow'}, cat); + expect(catCallback).toHaveBeenLastCalledWith({age: 3, sound: 'meow'}, cat); - expect(peopleCollectionCallback).toHaveBeenNthCalledWith( - 1, + expect(peopleCollectionCallback).toHaveBeenLastCalledWith( { [bob]: {age: 25, car: 'sedan'}, [lisa]: {age: 21, car: 'SUV'}, @@ -3355,7 +3352,11 @@ describe('RAM-only keys should not read from storage', () => { }); await act(async () => waitForPromisesToResolve()); - expect(receivedCollection).toBeUndefined(); + // Initial fire delivers the post-init frozen `{}` snapshot for a known-but-empty + // collection (legacy `undefined`-for-empty-initial shim was removed). What matters + // for this test is that the RAM-only members have NOT been hydrated from storage — + // the snapshot has no entries, and `cache.get(member)` returns `undefined`. + expect(receivedCollection).toEqual({}); expect(cache.get(collectionMember1)).toBeUndefined(); expect(cache.get(collectionMember2)).toBeUndefined(); diff --git a/tests/unit/onyxUtilsTest.ts b/tests/unit/onyxUtilsTest.ts index 0a20a7d21..dc1931dd6 100644 --- a/tests/unit/onyxUtilsTest.ts +++ b/tests/unit/onyxUtilsTest.ts @@ -3,7 +3,7 @@ import Onyx from '../../lib'; import OnyxUtils from '../../lib/OnyxUtils'; import type {GenericDeepRecord} from '../types'; import utils from '../../lib/utils'; -import type {Collection, OnyxCollection} from '../../lib/types'; +import type {OnyxCollection} from '../../lib/types'; import type GenericCollection from '../utils/GenericCollection'; import OnyxCache from '../../lib/OnyxCache'; import * as Logger from '../../lib/Logger'; @@ -371,9 +371,10 @@ describe('OnyxUtils', () => { Onyx.disconnect(conn2); }); - it('should not fire again for a collection subscriber that disconnects itself in its callback', async () => { - // A collection-root subscriber disconnects itself when it receives a - // collection object. A subsequent collection change must NOT trigger another callback. + it('should stop firing callbacks for a collection subscriber that disconnects itself mid-batch', async () => { + // A collection subscriber (waitForCollectionCallback=false) disconnects itself when + // it receives the first member. Subsequent changed members in the same batch must NOT + // trigger further callbacks for this subscriber. const callback = jest.fn(); const connection = Onyx.connect({ key: ONYXKEYS.COLLECTION.TEST_KEY, @@ -385,18 +386,13 @@ describe('OnyxUtils', () => { Onyx.disconnect(connection); }); - // First batch fires the collection callback once, which disconnects the subscriber. await Onyx.multiSet({ [`${ONYXKEYS.COLLECTION.TEST_KEY}1`]: {id: 1}, [`${ONYXKEYS.COLLECTION.TEST_KEY}2`]: {id: 2}, [`${ONYXKEYS.COLLECTION.TEST_KEY}3`]: {id: 3}, }); - expect(callback).toHaveBeenCalledTimes(1); - - // A subsequent change must not fire the now-disconnected subscriber again. - await Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}1`, {id: 11}); - + // Despite 3 changed members, callback should fire at most once before disconnect stops it expect(callback).toHaveBeenCalledTimes(1); }); @@ -450,221 +446,6 @@ describe('OnyxUtils', () => { }); }); - describe('keysChanged', () => { - beforeEach(() => { - Onyx.clear(); - }); - - afterEach(() => { - Onyx.clear(); - }); - - it('should call callback when data actually changes for collection member key subscribers', async () => { - const callbackSpy = jest.fn(); - const entryKey = `${ONYXKEYS.COLLECTION.TEST_KEY}123`; - const connection = Onyx.connect({ - key: entryKey, - callback: callbackSpy, - }); - - const entryData = {value: 'updated_data'}; - - // Create partial collection data that includes our member key - const collection = { - [entryKey]: entryData, - } as Collection; - - // Clear the callback spy to focus on the keysChanged behavior - callbackSpy.mockClear(); - - await Onyx.setCollection(ONYXKEYS.COLLECTION.TEST_KEY, collection); - - // Verify the subscriber callback was called - expect(callbackSpy).toHaveBeenCalledTimes(1); - expect(callbackSpy).toHaveBeenCalledWith(entryData, entryKey); - - await Onyx.disconnect(connection); - }); - - it('should set lastConnectionCallbackData for collection member key subscribers', async () => { - const entryKey = `${ONYXKEYS.COLLECTION.TEST_KEY}456`; - const initialEntryData = {value: 'initial_data'}; - const updatedEntryData = {value: 'updated_data'}; - const newEntryData = {value: 'new_data'}; - const callbackSpy = jest.fn(); - - const connection = await Onyx.connect({ - key: entryKey, - callback: callbackSpy, - }); - - // Create partial collection data that includes our member key - const initialCollection = { - [entryKey]: initialEntryData, - } as Collection; - - // Clear the callback spy to focus on the keysChanged behavior - callbackSpy.mockClear(); - - OnyxUtils.keysChanged( - ONYXKEYS.COLLECTION.TEST_KEY, - {[entryKey]: updatedEntryData}, // new collection - initialCollection, // previous collection - ); - - // Should be called again because data changed - expect(callbackSpy).toHaveBeenCalledTimes(1); - expect(callbackSpy).toHaveBeenCalledWith(undefined, entryKey); - - // Clear the callback spy to focus on the keyChanged behavior - callbackSpy.mockClear(); - - OnyxUtils.keyChanged( - entryKey, - newEntryData, // Second update with different data - () => true, // notify connect subscribers - ); - - // Should be called again because data changed - expect(callbackSpy).toHaveBeenCalledTimes(1); - expect(callbackSpy).toHaveBeenCalledWith(newEntryData, entryKey); - - await Onyx.disconnect(connection); - }); - - it('should notify collection-level subscribers with the whole collection object', async () => { - const entryKey = `${ONYXKEYS.COLLECTION.TEST_KEY}789`; - const entryData = {value: 'data'}; - - const collectionCallback = jest.fn(); - const connection = Onyx.connect({ - key: ONYXKEYS.COLLECTION.TEST_KEY, - callback: collectionCallback, - }); - - await Onyx.set(entryKey, entryData); - collectionCallback.mockClear(); - - // Trigger keysChanged directly with a partial collection - OnyxUtils.keysChanged(ONYXKEYS.COLLECTION.TEST_KEY, {[entryKey]: entryData}, {}); - - expect(collectionCallback).toHaveBeenCalledTimes(1); - // Collection subscriber receives the full cached collection and subscriber.key - const [receivedCollection, receivedKey] = collectionCallback.mock.calls[0]; - expect(receivedKey).toBe(ONYXKEYS.COLLECTION.TEST_KEY); - expect(receivedCollection[entryKey]).toEqual(entryData); - - Onyx.disconnect(connection); - }); - - it('should skip notification when member value has same reference in previous and current collection', async () => { - const entryKey = `${ONYXKEYS.COLLECTION.TEST_KEY}same`; - const sameValue = {value: 'unchanged'}; - - await Onyx.set(entryKey, sameValue); - - const callbackSpy = jest.fn(); - const connection = Onyx.connect({ - key: entryKey, - callback: callbackSpy, - }); - await waitForPromisesToResolve(); - callbackSpy.mockClear(); - - // Simulate keysChanged where the previous and current value are the SAME reference - // (which happens with frozen snapshots when nothing changed). === should skip notification. - OnyxUtils.keysChanged(ONYXKEYS.COLLECTION.TEST_KEY, {[entryKey]: sameValue}, {[entryKey]: sameValue}); - - expect(callbackSpy).not.toHaveBeenCalled(); - - Onyx.disconnect(connection); - }); - - it('should notify member subscribers only for changed keys in a batched update', async () => { - const keyA = `${ONYXKEYS.COLLECTION.TEST_KEY}A`; - const keyB = `${ONYXKEYS.COLLECTION.TEST_KEY}B`; - const keyC = `${ONYXKEYS.COLLECTION.TEST_KEY}C`; - - const dataA = {value: 'A'}; - const dataB = {value: 'B'}; - const dataC = {value: 'C'}; - - await Onyx.multiSet({[keyA]: dataA, [keyB]: dataB, [keyC]: dataC}); - - const spyA = jest.fn(); - const spyB = jest.fn(); - const spyC = jest.fn(); - const connA = Onyx.connect({key: keyA, callback: spyA}); - const connB = Onyx.connect({key: keyB, callback: spyB}); - const connC = Onyx.connect({key: keyC, callback: spyC}); - await waitForPromisesToResolve(); - spyA.mockClear(); - spyB.mockClear(); - spyC.mockClear(); - - // Update cache so keysChanged reads the new values via getCachedCollection - const newA = {value: 'A-updated'}; - const newC = {value: 'C-updated'}; - OnyxCache.set(keyA, newA); - OnyxCache.set(keyC, newC); - // keyB stays the same reference - - OnyxUtils.keysChanged(ONYXKEYS.COLLECTION.TEST_KEY, {[keyA]: newA, [keyB]: dataB, [keyC]: newC}, {[keyA]: dataA, [keyB]: dataB, [keyC]: dataC}); - - expect(spyA).toHaveBeenCalledTimes(1); - expect(spyB).not.toHaveBeenCalled(); - expect(spyC).toHaveBeenCalledTimes(1); - - Onyx.disconnect(connA); - Onyx.disconnect(connB); - Onyx.disconnect(connC); - }); - - it('should catch errors thrown by subscriber callbacks and continue notifying others', async () => { - const entryKey = `${ONYXKEYS.COLLECTION.TEST_KEY}errorTest`; - const entryData = {value: 'data'}; - - await Onyx.set(entryKey, entryData); - - const failingCallback = jest.fn(); - const workingCallback = jest.fn(); - - const connFailing = Onyx.connect({ - key: entryKey, - callback: failingCallback, - reuseConnection: false, - }); - const connWorking = Onyx.connect({ - key: entryKey, - callback: workingCallback, - reuseConnection: false, - }); - await waitForPromisesToResolve(); - failingCallback.mockReset(); - failingCallback.mockImplementation(() => { - throw new Error('subscriber failure'); - }); - workingCallback.mockClear(); - - // Spy on Logger to verify the error is logged - const logSpy = jest.spyOn(Logger, 'logAlert').mockImplementation(() => undefined); - - const newData = {value: 'new'}; - // Update the cache so keysChanged sees the new value as different from previous - OnyxCache.set(entryKey, newData); - OnyxUtils.keysChanged(ONYXKEYS.COLLECTION.TEST_KEY, {[entryKey]: newData}, {[entryKey]: entryData}); - - // Both callbacks should have been attempted; error should be logged - expect(failingCallback).toHaveBeenCalled(); - expect(workingCallback).toHaveBeenCalled(); - expect(logSpy).toHaveBeenCalled(); - - logSpy.mockRestore(); - Onyx.disconnect(connFailing); - Onyx.disconnect(connWorking); - }); - }); - describe('mergeChanges', () => { it("should return the last change if it's an array", () => { const {result} = OnyxUtils.mergeChanges([...testMergeChanges, [0, 1, 2]], testObject); @@ -1085,7 +866,7 @@ describe('OnyxUtils', () => { // re-enters the failing method on the next attempt. const transientError = new Error('Transient storage error'); - it('mergeCollection — collection-root subscriber fires once across retries', async () => { + it('mergeCollection — waitForCollectionCallback subscriber fires once across retries', async () => { const collectionKey = ONYXKEYS.COLLECTION.TEST_KEY; const existingMemberKey = `${collectionKey}1`; const newMemberKey = `${collectionKey}2`; @@ -1108,7 +889,7 @@ describe('OnyxUtils', () => { } as GenericCollection); // Before this fix, every retry attempt re-fired keysChanged() — and - // Collection-root subscribers fire on every keysChanged() call by contract. + // waitForCollectionCallback subscribers fire on every keysChanged() call by contract. // After the fix, retries skip the keysChanged re-fire, so subscribers are notified // exactly once per logical operation. expect(collectionCallback).toHaveBeenCalledTimes(1); diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 837dcbdee..9d6c0246c 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -4,7 +4,6 @@ import Onyx, {useOnyx} from '../../lib'; import StorageMock from '../../lib/storage'; import type GenericCollection from '../utils/GenericCollection'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; -import onyxSnapshotCache from '../../lib/OnyxSnapshotCache'; import type {UseOnyxSelector} from '../../lib/useOnyx'; const ONYXKEYS = { @@ -27,8 +26,6 @@ Onyx.init({ beforeEach(async () => { await Onyx.clear(); - onyxSnapshotCache.clear(); - onyxSnapshotCache.clearSelectorIds(); }); describe('useOnyx', () => { @@ -53,27 +50,6 @@ describe('useOnyx', () => { } }); - it('should transition through loading when switching between collection member keys that both resolve to undefined', async () => { - const {result, rerender} = renderHook((key: string) => useOnyx(key), {initialProps: `${ONYXKEYS.COLLECTION.TEST_KEY}1` as string}); - - // Wait for initial key to fully load - await act(async () => waitForPromisesToResolve()); - - expect(result.current[0]).toBeUndefined(); - expect(result.current[1].status).toEqual('loaded'); - - // Switch to another collection member key that also has no data - rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}2`); - - expect(result.current[0]).toBeUndefined(); - expect(result.current[1].status).toEqual('loading'); - - await act(async () => waitForPromisesToResolve()); - - expect(result.current[0]).toBeUndefined(); - expect(result.current[1].status).toEqual('loaded'); - }); - it('should return cached value immediately with loaded status when switching to a key that has data', async () => { Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}2`, 'test_value'); @@ -97,28 +73,6 @@ describe('useOnyx', () => { expect(result.current[1].status).toEqual('loaded'); }); - it('should clear previous data and transition through loading when switching from a key with data to one without', async () => { - Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}1`, 'initial_value'); - - const {result, rerender} = renderHook((key: string) => useOnyx(key), {initialProps: `${ONYXKEYS.COLLECTION.TEST_KEY}1` as string}); - - await act(async () => waitForPromisesToResolve()); - - expect(result.current[0]).toEqual('initial_value'); - expect(result.current[1].status).toEqual('loaded'); - - // Switch to a key that has no data - rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}2`); - - expect(result.current[0]).toBeUndefined(); - expect(result.current[1].status).toEqual('loading'); - - await act(async () => waitForPromisesToResolve()); - - expect(result.current[0]).toBeUndefined(); - expect(result.current[1].status).toEqual('loaded'); - }); - it('should return the new value when switching from a key with data to another key with different data', async () => { Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}1`, 'value_one'); Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}2`, 'value_two'); @@ -239,30 +193,6 @@ describe('useOnyx', () => { }); describe('misc', () => { - it('should initially return loading state while loading non-existent key, and then return `undefined` and loaded state', async () => { - const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); - - expect(result.current[0]).toBeUndefined(); - expect(result.current[1].status).toEqual('loading'); - - await act(async () => waitForPromisesToResolve()); - - expect(result.current[0]).toBeUndefined(); - expect(result.current[1].status).toEqual('loaded'); - }); - - it('should initially return loading state while loading non-existent collection key, and then return `undefined` and loaded state', async () => { - const {result} = renderHook(() => useOnyx(ONYXKEYS.COLLECTION.TEST_KEY)); - - expect(result.current[0]).toBeUndefined(); - expect(result.current[1].status).toEqual('loading'); - - await act(async () => waitForPromisesToResolve()); - - expect(result.current[0]).toBeUndefined(); - expect(result.current[1].status).toEqual('loaded'); - }); - it('should return value and loaded state when loading cached key', async () => { Onyx.set(ONYXKEYS.TEST_KEY, 'test'); @@ -272,36 +202,6 @@ describe('useOnyx', () => { expect(result.current[1].status).toEqual('loaded'); }); - it('should initially return `undefined` while loading non-cached key, and then return value and loaded state', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); - - const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); - - expect(result.current[0]).toBeUndefined(); - expect(result.current[1].status).toEqual('loading'); - - await act(async () => waitForPromisesToResolve()); - - expect(result.current[0]).toEqual('test'); - expect(result.current[1].status).toEqual('loaded'); - }); - - it('should initially return undefined and then return cached value after multiple merge operations', async () => { - Onyx.merge(ONYXKEYS.TEST_KEY, 'test1'); - Onyx.merge(ONYXKEYS.TEST_KEY, 'test2'); - Onyx.merge(ONYXKEYS.TEST_KEY, 'test3'); - - const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); - - expect(result.current[0]).toBeUndefined(); - expect(result.current[1].status).toEqual('loading'); - - await act(async () => waitForPromisesToResolve()); - - expect(result.current[0]).toEqual('test3'); - expect(result.current[1].status).toEqual('loaded'); - }); - it('should return value from cache, and return updated value after a merge operation', async () => { Onyx.set(ONYXKEYS.TEST_KEY, 'test1'); @@ -338,76 +238,6 @@ describe('useOnyx', () => { expect(result2.current[1].status).toEqual('loaded'); }); - it('should return updated state when connecting to the same regular key after an Onyx.clear() call', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); - - const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); - - await act(async () => waitForPromisesToResolve()); - - expect(result1.current[0]).toEqual('test'); - expect(result1.current[1].status).toEqual('loaded'); - - await act(async () => Onyx.clear()); - - const {result: result2} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); - const {result: result3} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); - - await act(async () => waitForPromisesToResolve()); - - expect(result1.current[0]).toBeUndefined(); - expect(result1.current[1].status).toEqual('loaded'); - expect(result2.current[0]).toBeUndefined(); - expect(result2.current[1].status).toEqual('loaded'); - expect(result3.current[0]).toBeUndefined(); - expect(result3.current[1].status).toEqual('loaded'); - - Onyx.merge(ONYXKEYS.TEST_KEY, 'test2'); - await act(async () => waitForPromisesToResolve()); - - expect(result1.current[0]).toEqual('test2'); - expect(result1.current[1].status).toEqual('loaded'); - expect(result2.current[0]).toEqual('test2'); - expect(result2.current[1].status).toEqual('loaded'); - expect(result3.current[0]).toEqual('test2'); - expect(result3.current[1].status).toEqual('loaded'); - }); - - it('should return updated state when connecting to the same colection member key after an Onyx.clear() call', async () => { - await StorageMock.setItem(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`, 'test'); - - const {result: result1} = renderHook(() => useOnyx(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`)); - - await act(async () => waitForPromisesToResolve()); - - expect(result1.current[0]).toEqual('test'); - expect(result1.current[1].status).toEqual('loaded'); - - await act(async () => Onyx.clear()); - - const {result: result2} = renderHook(() => useOnyx(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`)); - const {result: result3} = renderHook(() => useOnyx(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`)); - - await act(async () => waitForPromisesToResolve()); - - expect(result1.current[0]).toBeUndefined(); - expect(result1.current[1].status).toEqual('loaded'); - expect(result2.current[0]).toBeUndefined(); - expect(result2.current[1].status).toEqual('loaded'); - expect(result3.current[0]).toBeUndefined(); - expect(result3.current[1].status).toEqual('loaded'); - - Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`, 'test2'); - await act(async () => waitForPromisesToResolve()); - - expect(result1.current[0]).toEqual('test2'); - expect(result1.current[1].status).toEqual('loaded'); - expect(result2.current[0]).toEqual('test2'); - expect(result2.current[1].status).toEqual('loaded'); - expect(result3.current[0]).toEqual('test2'); - expect(result3.current[1].status).toEqual('loaded'); - }); - it('should not update the result when a new object with shallow-equal content is set', async () => { Onyx.set(ONYXKEYS.TEST_KEY, {id: 'test_id', name: 'test_name'}); @@ -735,89 +565,207 @@ describe('useOnyx', () => { expect(result.current[0]).not.toBe(firstResult); expect(result.current[0]).toBe(10); }); - }); - describe('pending merges', () => { - it('should return undefined and loading state while we have pending merges for the key, and then return updated value and loaded state', async () => { - Onyx.set(ONYXKEYS.TEST_KEY, 'test1'); + it('should recompute selector when dependencies change even if input data stays the same', async () => { + const testCollection = { + [`${ONYXKEYS.COLLECTION.TEST_KEY}1`]: {id: '1', value: 'item1'}, + [`${ONYXKEYS.COLLECTION.TEST_KEY}2`]: {id: '2', value: 'item2'}, + [`${ONYXKEYS.COLLECTION.TEST_KEY}3`]: {id: '3', value: 'item3'}, + }; - Onyx.merge(ONYXKEYS.TEST_KEY, 'test2'); - Onyx.merge(ONYXKEYS.TEST_KEY, 'test3'); - Onyx.merge(ONYXKEYS.TEST_KEY, 'test4'); + await act(async () => Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, testCollection as GenericCollection)); - const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + let filterIds = ['1']; + let selectorCallCount = 0; - expect(result.current[0]).toBeUndefined(); - expect(result.current[1].status).toEqual('loading'); + const {result, rerender} = renderHook(() => + useOnyx(ONYXKEYS.COLLECTION.TEST_KEY, { + selector: (collection) => { + selectorCallCount++; + return filterIds.map((id) => (collection as OnyxCollection)?.[`${ONYXKEYS.COLLECTION.TEST_KEY}${id}`]).filter(Boolean); + }, + }), + ); await act(async () => waitForPromisesToResolve()); - expect(result.current[0]).toEqual('test4'); - expect(result.current[1].status).toEqual('loaded'); + // Record count after initial stabilization + const initialCallCount = selectorCallCount; + const initialResult = result.current[0]; + + // Should return item with id '1' + expect(initialResult).toEqual([{id: '1', value: 'item1'}]); + + // Change dependencies without changing underlying data + await act(async () => { + filterIds = ['1', '2']; + rerender(ONYXKEYS.COLLECTION.TEST_KEY); + }); + + // Selector should recompute and return items with id '1' and '2' + expect(result.current[0]).toEqual([ + {id: '1', value: 'item1'}, + {id: '2', value: 'item2'}, + ]); + expect(selectorCallCount).toBeGreaterThan(initialCallCount); + + // Record count after first dependency change + const firstChangeCallCount = selectorCallCount; + + // Change dependencies again + await act(async () => { + filterIds = ['2', '3']; + rerender(ONYXKEYS.COLLECTION.TEST_KEY); + }); + + // Selector should recompute and return items with id '2' and '3' + expect(result.current[0]).toEqual([ + {id: '2', value: 'item2'}, + {id: '3', value: 'item3'}, + ]); + expect(selectorCallCount).toBeGreaterThan(firstChangeCallCount); }); - it('should return undefined and loading state while we have pending merges for the key, and then return selected data and loaded state', async () => { - Onyx.set(ONYXKEYS.TEST_KEY, 'test1'); + it('should handle complex dependency scenarios with multiple values', async () => { + type TestItem = {id: string; category: string; priority: number}; + const testData = { + [`${ONYXKEYS.COLLECTION.TEST_KEY}item1`]: {id: 'item1', category: 'A', priority: 1}, + [`${ONYXKEYS.COLLECTION.TEST_KEY}item2`]: {id: 'item2', category: 'B', priority: 2}, + [`${ONYXKEYS.COLLECTION.TEST_KEY}item3`]: {id: 'item3', category: 'A', priority: 3}, + [`${ONYXKEYS.COLLECTION.TEST_KEY}item4`]: {id: 'item4', category: 'B', priority: 4}, + }; - Onyx.merge(ONYXKEYS.TEST_KEY, 'test2'); - Onyx.merge(ONYXKEYS.TEST_KEY, 'test3'); - Onyx.merge(ONYXKEYS.TEST_KEY, 'test4'); + await act(async () => Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, testData as GenericCollection)); - const {result} = renderHook(() => - useOnyx(ONYXKEYS.TEST_KEY, { - selector: ((entry: OnyxEntry) => `${entry}_changed`) as UseOnyxSelector, + let categoryFilter = 'A'; + let sortAscending = true; + + const {result, rerender} = renderHook(() => + useOnyx(ONYXKEYS.COLLECTION.TEST_KEY, { + selector: (collection) => { + const typedCollection = collection as OnyxCollection; + if (!typedCollection) return []; + + const filtered = Object.values(typedCollection).filter((item) => item?.category === categoryFilter); + + return filtered.sort((a, b) => (sortAscending ? (a?.priority ?? 0) - (b?.priority ?? 0) : (b?.priority ?? 0) - (a?.priority ?? 0))); + }, }), ); - expect(result.current[0]).toBeUndefined(); - expect(result.current[1].status).toEqual('loading'); - await act(async () => waitForPromisesToResolve()); - expect(result.current[0]).toEqual('test4_changed'); - expect(result.current[1].status).toEqual('loaded'); + // Should return category A items sorted ascending + expect(result.current[0]).toEqual([ + {id: 'item1', category: 'A', priority: 1}, + {id: 'item3', category: 'A', priority: 3}, + ]); + + // Change sort order only + await act(async () => { + sortAscending = false; + rerender(ONYXKEYS.COLLECTION.TEST_KEY); + }); + + // Should return category A items sorted descending + expect(result.current[0]).toEqual([ + {id: 'item3', category: 'A', priority: 3}, + {id: 'item1', category: 'A', priority: 1}, + ]); + + // Change category filter + await act(async () => { + categoryFilter = 'B'; + rerender(ONYXKEYS.COLLECTION.TEST_KEY); + }); + + // Should return category B items sorted descending + expect(result.current[0]).toEqual([ + {id: 'item4', category: 'B', priority: 4}, + {id: 'item2', category: 'B', priority: 2}, + ]); }); - }); - describe('multiple usage', () => { - it('should connect to a key and load the value into cache, and return the value loaded in the next hook call', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + it('should not trigger unnecessary recomputations when dependencies remain the same', async () => { + await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, {value: 'test'})); - const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + const dependencies = ['constant']; + let selectorCallCount = 0; + const selector = ((data) => { + selectorCallCount++; + return `${dependencies.join(',')}:${(data as {value?: string})?.value}`; + }) as UseOnyxSelector; - expect(result1.current[0]).toBeUndefined(); - expect(result1.current[1].status).toEqual('loading'); + const {result, rerender} = renderHook(() => + useOnyx(ONYXKEYS.TEST_KEY, { + selector, + }), + ); await act(async () => waitForPromisesToResolve()); - expect(result1.current[0]).toEqual('test'); - expect(result1.current[1].status).toEqual('loaded'); + expect(result.current[0]).toBe('constant:test'); + expect(selectorCallCount).toBe(1); - const {result: result2} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + // Force rerender without changing dependencies + await act(async () => { + rerender(ONYXKEYS.COLLECTION.TEST_KEY); + }); - expect(result2.current[0]).toEqual('test'); - expect(result2.current[1].status).toEqual('loaded'); - }); + // Selector should not recompute since dependencies haven't changed + expect(result.current[0]).toBe('constant:test'); + expect(selectorCallCount).toBe(1); - it('should connect to a key two times while data is loading from the cache, and return the value loaded to both of them', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + // Update underlying data + await act(async () => Onyx.merge(ONYXKEYS.TEST_KEY, {value: 'updated'})); - const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); - const {result: result2} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + // Selector should recompute due to data change + expect(result.current[0]).toBe('constant:updated'); + expect(selectorCallCount).toBe(2); + }); + }); - expect(result1.current[0]).toBeUndefined(); - expect(result1.current[1].status).toEqual('loading'); + describe('dependencies', () => { + it('should return the updated selected value when a external value passed to the dependencies list changes', async () => { + Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, { + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'}, + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'}, + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`]: {id: 'entry3_id', name: 'entry3_name'}, + } as GenericCollection); - expect(result2.current[0]).toBeUndefined(); - expect(result2.current[1].status).toEqual('loading'); + let externalValue = 'ex1'; + + const {result, rerender} = renderHook(() => + useOnyx(ONYXKEYS.COLLECTION.TEST_KEY, { + selector: ((entries: OnyxCollection<{id: string; name: string}>) => + Object.entries(entries ?? {}).reduce>>((acc, [key, value]) => { + acc[key] = `${value?.id}_${externalValue}`; + return acc; + }, {})) as UseOnyxSelector>>, + }), + ); await act(async () => waitForPromisesToResolve()); - expect(result1.current[0]).toEqual('test'); - expect(result1.current[1].status).toEqual('loaded'); + expect(result.current[0]).toEqual({ + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: 'entry1_id_ex1', + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: 'entry2_id_ex1', + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`]: 'entry3_id_ex1', + }); + expect(result.current[1].status).toEqual('loaded'); - expect(result2.current[0]).toEqual('test'); - expect(result2.current[1].status).toEqual('loaded'); + externalValue = 'ex2'; + + await act(async () => { + rerender(undefined); + }); + + expect(result.current[0]).toEqual({ + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: 'entry1_id_ex2', + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: 'entry2_id_ex2', + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`]: 'entry3_id_ex2', + }); + expect(result.current[1].status).toEqual('loaded'); }); }); @@ -1004,122 +952,5 @@ describe('useOnyx', () => { // A single render — no extra render caused by subscribe resetting state on initial mount. expect(renderCount).toBe(1); }); - - it('should render exactly twice (loading → loaded) when the key is not cached', async () => { - let renderCount = 0; - const {result} = renderHook(() => { - renderCount++; - return useOnyx(ONYXKEYS.TEST_KEY); - }); - - await act(async () => waitForPromisesToResolve()); - - expect(result.current[0]).toBeUndefined(); - expect(result.current[1].status).toEqual('loaded'); - // Exactly two renders: initial 'loading' + transition to 'loaded' after the connection callback fires. - // If the regression returns, a third render sneaks in from the subscribe-time state reset. - expect(renderCount).toBe(2); - }); - - it('should render exactly twice when the key value is only present in storage', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'storage_value'); - - let renderCount = 0; - const {result} = renderHook(() => { - renderCount++; - return useOnyx(ONYXKEYS.TEST_KEY); - }); - - await act(async () => waitForPromisesToResolve()); - - expect(result.current[0]).toEqual('storage_value'); - expect(result.current[1].status).toEqual('loaded'); - expect(renderCount).toBe(2); - }); - - it('should render exactly twice for a non-cached collection member key', async () => { - let renderCount = 0; - const {result} = renderHook(() => { - renderCount++; - return useOnyx(`${ONYXKEYS.COLLECTION.TEST_KEY}1`); - }); - - await act(async () => waitForPromisesToResolve()); - - expect(result.current[0]).toBeUndefined(); - expect(result.current[1].status).toEqual('loaded'); - expect(renderCount).toBe(2); - }); - - // Covers the `if (hasMountedRef.current)` branch — i.e. the reset that runs on key-change re-subscriptions. - // The reset is what makes the hook transition through 'loading' for the new key instead of leaking the - // previous key's value/status. These tests verify both the render count AND the loading transition, - // so removing the reset (regression in the other direction) is also caught. - it('should transition through loading and render exactly 4 times when switching from a cached key to an uncached one', async () => { - await Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}A`, 'A_value'); - - const renders: Array<{value: unknown; status: string}> = []; - const {result, rerender} = renderHook( - (key: string) => { - const r = useOnyx(key); - renders.push({value: r[0], status: r[1].status}); - return r; - }, - {initialProps: `${ONYXKEYS.COLLECTION.TEST_KEY}A` as string}, - ); - - await act(async () => waitForPromisesToResolve()); - - expect(result.current[0]).toEqual('A_value'); - expect(result.current[1].status).toEqual('loaded'); - const rendersAfterMount = renders.length; - expect(rendersAfterMount).toBe(1); - - await act(async () => { - rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}B`); - }); - await act(async () => waitForPromisesToResolve()); - - expect(result.current[0]).toBeUndefined(); - expect(result.current[1].status).toEqual('loaded'); - // 1 mount render + 3 renders for the key switch (transient stale render, post-subscribe 'loading', - // callback-driven 'loaded'). The 'loading' render only happens because the subscribe-time reset - // clears the previous key's resultRef — removing the reset makes this assertion fail. - expect(renders.length).toBe(4); - // Verify the reset took effect: a 'loading' frame must appear after the key change. - const postSwitchStatuses = renders.slice(rendersAfterMount).map((r) => r.status); - expect(postSwitchStatuses).toContain('loading'); - expect(postSwitchStatuses[postSwitchStatuses.length - 1]).toBe('loaded'); - }); - - it('should transition through loading and render exactly 3 times when switching between two cached keys', async () => { - await Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}A`, 'A_value'); - await Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}B`, 'B_value'); - - const renders: Array<{value: unknown; status: string}> = []; - const {result, rerender} = renderHook( - (key: string) => { - const r = useOnyx(key); - renders.push({value: r[0], status: r[1].status}); - return r; - }, - {initialProps: `${ONYXKEYS.COLLECTION.TEST_KEY}A` as string}, - ); - - await act(async () => waitForPromisesToResolve()); - - expect(result.current[0]).toEqual('A_value'); - expect(renders.length).toBe(1); - - await act(async () => { - rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}B`); - }); - await act(async () => waitForPromisesToResolve()); - - expect(result.current[0]).toEqual('B_value'); - expect(result.current[1].status).toEqual('loaded'); - // 1 mount render + 2 renders for the cached-to-cached switch. - expect(renders.length).toBe(3); - }); }); }); From 86e4d308c3a3dc80c0d8eb55076feb61fbdf5be6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Wed, 24 Jun 2026 08:44:13 +0100 Subject: [PATCH 03/12] useOnyx: stabilize selector output via useSyncExternalStoreWithSelector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebuild the store-based useOnyx on React's useSyncExternalStoreWithSelector (use-sync-external-store/with-selector) with deepEqual as the equality fn. Its committed-value dedup collapses content-equal selections to a stable reference and survives the selector function's identity churning every render — so consumers can pass inline selectors that close over freshly allocated arrays/objects without stabilizing the inputs (no more useStableArrayReference). No-selector subscriptions keep the Object.is fast-path since the raw cache value is already reference-stable. Adds use-sync-external-store + @types/use-sync-external-store. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/useOnyx.ts | 52 ++++++++++++++++++++++++----------------------- package-lock.json | 23 +++++++++++++++++---- package.json | 4 +++- 3 files changed, 49 insertions(+), 30 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 859f3160f..3d96e4ce3 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -1,5 +1,6 @@ -import {useCallback, useMemo, useRef, useSyncExternalStore} from 'react'; -import createMemoizedSelector from './createMemoizedSelector'; +import {useCallback, useMemo} from 'react'; +import {deepEqual} from 'fast-equals'; +import {useSyncExternalStoreWithSelector} from 'use-sync-external-store/with-selector'; import onyxStore from './OnyxStore'; import type {OnyxKey, OnyxValue} from './types'; @@ -7,10 +8,11 @@ type UseOnyxSelector> = (da type UseOnyxOptions = { /** - * Subscribe to a subset of an Onyx key's data. The component re-renders only when - * the selector's output reference changes; selectors that allocate fresh objects - * (e.g. `(e) => ({id: e?.id})`) are handled by an internal input-cache + deepEqual - * fallback so they don't cause `useSyncExternalStore` to loop. + * Subscribe to a subset of an Onyx key's data. The component re-renders only when the + * selector's output *changes by deep equality* — a selector that allocates a fresh object + * (e.g. `(e) => ({id: e?.id})`) or one whose identity churns every render (an inline + * selector closing over a fresh array) is collapsed to a stable reference internally, so it + * never causes `useSyncExternalStore` to loop and never forces a redundant re-render. */ selector?: UseOnyxSelector; }; @@ -38,34 +40,34 @@ const LOADED_METADATA: ResultMetadata = {status: 'loaded'}; * Returns `[value, {status: 'loaded'}]`. With eager-load + the structural-sharing cache, * there's no loading phase — the cache always has an answer (a value or "absent"). The * `status` field is retained for API compatibility and is always `'loaded'`. + * + * Selector stability is delegated to React's `useSyncExternalStoreWithSelector`: the selection + * is deduped against the last value committed to React (by deep equality when a selector is + * present), and that dedup survives the selector function's *identity* changing every render. + * So consumers can pass inline selectors that close over freshly allocated arrays/objects + * without stabilizing the inputs themselves. Subscriptions without a selector read the raw, + * already reference-stable cache value and rely on the default `Object.is` comparison (no + * deep-equal cost). */ function useOnyx>(key: TKey, options?: UseOnyxOptions): UseOnyxResult { const selector = options?.selector; - // The memoized selector is recreated only when the selector function identity changes. - // Inside, it caches by input reference; that's what keeps useSyncExternalStore from - // looping when consumers pass inline-allocating selectors. - const memoizedSelector = useMemo(() => (selector ? createMemoizedSelector(selector) : null), [selector]); - const subscribe = useCallback((onStoreChange: () => void) => onyxStore.subscribe(key, onStoreChange), [key]); + const getSnapshot = useCallback(() => onyxStore.getState(key) as OnyxValue | undefined, [key]); - // resultRef holds the last tuple returned to React. We return the same tuple reference - // when value hasn't changed so React skips the re-render. - const resultRef = useRef>([undefined, LOADED_METADATA]); + // Normalizes `null` -> `undefined` and applies the consumer's selector (or passes the raw value + // through). Re-created only when the selector's identity changes; the committed-value dedup inside + // `useSyncExternalStoreWithSelector` is what makes a churning identity harmless. + const select = useCallback((data: OnyxValue | undefined): TReturnValue | undefined => (selector ? selector(data) : (data as TReturnValue | undefined)) ?? undefined, [selector]); - const getSnapshot = useCallback((): UseOnyxResult => { - const raw = onyxStore.getState(key); - const selected = memoizedSelector ? memoizedSelector(raw as OnyxValue) : (raw as TReturnValue | undefined); - const nextValue = (selected ?? undefined) as NonNullable | undefined; + // With a selector, dedupe the (possibly freshly allocated) output by deep equality. Without one, + // the raw cache value is already reference-stable, so the default `Object.is` is enough. + const isEqual = selector ? deepEqual : undefined; - if (resultRef.current[0] === nextValue) { - return resultRef.current; - } - resultRef.current = [nextValue, LOADED_METADATA]; - return resultRef.current; - }, [key, memoizedSelector]); + const value = useSyncExternalStoreWithSelector | undefined, TReturnValue | undefined>(subscribe, getSnapshot, undefined, select, isEqual); - return useSyncExternalStore(subscribe, getSnapshot); + // Stable result tuple: re-allocated only when the (already deduped) `value` reference changes. + return useMemo>(() => [value as NonNullable | undefined, LOADED_METADATA], [value]); } export default useOnyx; diff --git a/package-lock.json b/package-lock.json index b3f9f1314..78d869d3c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15,7 +15,8 @@ "lodash.clone": "^4.5.0", "lodash.pick": "^4.4.0", "lodash.transform": "^4.6.0", - "underscore": "^1.13.6" + "underscore": "^1.13.6", + "use-sync-external-store": "^1.6.0" }, "devDependencies": { "@actions/core": "^1.10.1", @@ -35,6 +36,7 @@ "@types/react": "^18.2.14", "@types/react-native": "^0.70.0", "@types/underscore": "^1.11.15", + "@types/use-sync-external-store": "^1.5.0", "@typescript-eslint/eslint-plugin": "^8.51.0", "@typescript-eslint/parser": "^8.51.0", "@vercel/ncc": "0.38.1", @@ -4535,6 +4537,13 @@ "dev": true, "license": "MIT" }, + "node_modules/@types/use-sync-external-store": { + "version": "1.5.0", + "resolved": "https://registry.npmjs.org/@types/use-sync-external-store/-/use-sync-external-store-1.5.0.tgz", + "integrity": "sha512-5dyB8nLC/qogMrlCizZnYWQTA4lnb/v+It+sqNl5YnSRAPMlIqY/X0Xn+gZw8vOL+TgTTr28VEbn3uf8fUtAkw==", + "dev": true, + "license": "MIT" + }, "node_modules/@types/yargs": { "version": "17.0.33", "resolved": "https://registry.npmjs.org/@types/yargs/-/yargs-17.0.33.tgz", @@ -11265,7 +11274,6 @@ "version": "4.0.0", "resolved": "https://registry.npmjs.org/js-tokens/-/js-tokens-4.0.0.tgz", "integrity": "sha512-RdJUflcE3cUzKiMqQgsCu06FPu9UdIJO0beYbPhHN4k6apgJtifcoCtT9bcxOpYBtpD2kCM6Sbzg4CausW/PKQ==", - "dev": true, "license": "MIT" }, "node_modules/js-yaml": { @@ -11839,7 +11847,6 @@ "version": "1.4.0", "resolved": "https://registry.npmjs.org/loose-envify/-/loose-envify-1.4.0.tgz", "integrity": "sha512-lyuxPGr/Wfhrlem2CL/UcnUc1zcqKAImBDzukY7Y5F/yQiNdko6+fRLevlw1HgMySw7f611UIY408EtxRSoK3Q==", - "dev": true, "license": "MIT", "dependencies": { "js-tokens": "^3.0.0 || ^4.0.0" @@ -13792,7 +13799,6 @@ "version": "18.2.0", "resolved": "https://registry.npmjs.org/react/-/react-18.2.0.tgz", "integrity": "sha512-/3IjMdb2L9QbBdWiW5e3P2/npwMBaU9mHCSCUzNln0ZCYbcfTsGbTJrU/kGemdH2IWmB2ioZ+zkxtmq6g09fGQ==", - "dev": true, "license": "MIT", "dependencies": { "loose-envify": "^1.1.0" @@ -16586,6 +16592,15 @@ "requires-port": "^1.0.0" } }, + "node_modules/use-sync-external-store": { + "version": "1.6.0", + "resolved": "https://registry.npmjs.org/use-sync-external-store/-/use-sync-external-store-1.6.0.tgz", + "integrity": "sha512-Pp6GSwGP/NrPIrxVFAIkOQeyw8lFenOHijQWkUTrDvrF4ALqylP2C/KCkeS9dpUM3KvYRQhna5vt7IL95+ZQ9w==", + "license": "MIT", + "peerDependencies": { + "react": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0" + } + }, "node_modules/util-deprecate": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/util-deprecate/-/util-deprecate-1.0.2.tgz", diff --git a/package.json b/package.json index eae29f47e..50583d9e9 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,8 @@ "lodash.clone": "^4.5.0", "lodash.pick": "^4.4.0", "lodash.transform": "^4.6.0", - "underscore": "^1.13.6" + "underscore": "^1.13.6", + "use-sync-external-store": "^1.6.0" }, "devDependencies": { "@actions/core": "^1.10.1", @@ -69,6 +70,7 @@ "@types/react": "^18.2.14", "@types/react-native": "^0.70.0", "@types/underscore": "^1.11.15", + "@types/use-sync-external-store": "^1.5.0", "@typescript-eslint/eslint-plugin": "^8.51.0", "@typescript-eslint/parser": "^8.51.0", "@vercel/ncc": "0.38.1", From 31fdebdf8aca0236927f80c648abb2af87536241 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Wed, 19 Aug 2026 10:39:05 +0100 Subject: [PATCH 04/12] fix: restore loading status for in-flight merges in useOnyx --- lib/useOnyx.ts | 13 +++++++++---- tests/unit/useOnyxTest.ts | 26 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 3d96e4ce3..0ea0d54ce 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -2,6 +2,7 @@ import {useCallback, useMemo} from 'react'; import {deepEqual} from 'fast-equals'; import {useSyncExternalStoreWithSelector} from 'use-sync-external-store/with-selector'; import onyxStore from './OnyxStore'; +import OnyxUtils from './OnyxUtils'; import type {OnyxKey, OnyxValue} from './types'; type UseOnyxSelector> = (data: OnyxValue | undefined) => TReturnValue; @@ -30,8 +31,6 @@ type ResultMetadata = { type UseOnyxResult = [NonNullable | undefined, ResultMetadata]; -const LOADED_METADATA: ResultMetadata = {status: 'loaded'}; - /** * Subscribes a React component to an Onyx key. The component re-renders when the value * at `key` changes (for collection keys, when any member changes — the returned value is @@ -66,8 +65,14 @@ function useOnyx>(key: TKey const value = useSyncExternalStoreWithSelector | undefined, TReturnValue | undefined>(subscribe, getSnapshot, undefined, select, isEqual); - // Stable result tuple: re-allocated only when the (already deduped) `value` reference changes. - return useMemo>(() => [value as NonNullable | undefined, LOADED_METADATA], [value]); + // The `loading` flag means a write is in flight: a pending `Onyx.merge` whose result the cache may not + // reflect yet. It flips back to `loaded` when the merge applies — that write fires a store + // notification which re-renders this hook. Cached reads are synchronous, so they are always `loaded`. + const loadingStatus: FetchStatus = OnyxUtils.hasPendingMergeForKey(key) ? 'loading' : 'loaded'; + + // Stable result tuple: re-built only when the (already deduped) `value` reference or the primitive + // `loadingStatus` changes, so render-to-render the same cached tuple (and metadata object) is returned. + return useMemo>(() => [value as NonNullable | undefined, {status: loadingStatus}], [value, loadingStatus]); } export default useOnyx; diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 9d6c0246c..621ba3bf8 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -953,4 +953,30 @@ describe('useOnyx', () => { expect(renderCount).toBe(1); }); }); + + describe('loading status', () => { + it('should report loading while a pending merge is in flight, then loaded once it resolves', async () => { + Onyx.merge(ONYXKEYS.TEST_KEY, 'test1'); + Onyx.merge(ONYXKEYS.TEST_KEY, 'test2'); + Onyx.merge(ONYXKEYS.TEST_KEY, 'test3'); + + const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + + expect(result.current[1].status).toEqual('loading'); + + await act(async () => waitForPromisesToResolve()); + + expect(result.current[0]).toEqual('test3'); + expect(result.current[1].status).toEqual('loaded'); + }); + + it('should report loaded immediately for a cached value with no pending merge', async () => { + Onyx.set(ONYXKEYS.TEST_KEY, 'cached'); + + const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + + expect(result.current[0]).toEqual('cached'); + expect(result.current[1].status).toEqual('loaded'); + }); + }); }); From db69973ccd9ac61331d4d5469379bb8c4b971119 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 21 Aug 2026 16:22:28 +0100 Subject: [PATCH 05/12] fix: scope useOnyx loading status to a key's first connection --- lib/useOnyx.ts | 29 ++++++++++---- tests/unit/useOnyxTest.ts | 82 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 7 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 0ea0d54ce..1cf14dd21 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -1,4 +1,4 @@ -import {useCallback, useMemo} from 'react'; +import {useCallback, useEffect, useMemo, useRef} from 'react'; import {deepEqual} from 'fast-equals'; import {useSyncExternalStoreWithSelector} from 'use-sync-external-store/with-selector'; import onyxStore from './OnyxStore'; @@ -51,6 +51,10 @@ type UseOnyxResult = [NonNullable | undefined, ResultMetadata]; function useOnyx>(key: TKey, options?: UseOnyxOptions): UseOnyxResult { const selector = options?.selector; + // Tracks the key this hook has already connected to, so we can tell a key's first render apart from + // later ones (see the loading-status gate below). Starts null so the initial mount counts as first. + const connectedKeyRef = useRef(null); + const subscribe = useCallback((onStoreChange: () => void) => onyxStore.subscribe(key, onStoreChange), [key]); const getSnapshot = useCallback(() => onyxStore.getState(key) as OnyxValue | undefined, [key]); @@ -65,14 +69,25 @@ function useOnyx>(key: TKey const value = useSyncExternalStoreWithSelector | undefined, TReturnValue | undefined>(subscribe, getSnapshot, undefined, select, isEqual); - // The `loading` flag means a write is in flight: a pending `Onyx.merge` whose result the cache may not - // reflect yet. It flips back to `loaded` when the merge applies — that write fires a store - // notification which re-renders this hook. Cached reads are synchronous, so they are always `loaded`. - const loadingStatus: FetchStatus = OnyxUtils.hasPendingMergeForKey(key) ? 'loading' : 'loaded'; + // `loading` only on a key's first render (mount or key change) when a merge is still in flight for it. + // `connectedKeyRef` differs from `key` only on that first render; the effect below catches it up, so a + // later merge on an already-connected key never surfaces loading. + // Reading the ref during render is safe: it's written only in the effect below and re-renders are driven + // by `useSyncExternalStore` and the `key` prop, so it can't cause a missed update — it gates a one-shot signal. + // eslint-disable-next-line react-hooks/refs + const isLoading = connectedKeyRef.current !== key && OnyxUtils.hasPendingMergeForKey(key); + const loadingStatus: FetchStatus = isLoading ? 'loading' : 'loaded'; + + useEffect(() => { + connectedKeyRef.current = key; + }, [key]); + + // While loading, the pending merge's result isn't in cache yet, so surface `undefined` until it applies. + const result = isLoading ? undefined : (value as NonNullable | undefined); - // Stable result tuple: re-built only when the (already deduped) `value` reference or the primitive + // Stable result tuple: re-built only when the (already deduped) `result` reference or the primitive // `loadingStatus` changes, so render-to-render the same cached tuple (and metadata object) is returned. - return useMemo>(() => [value as NonNullable | undefined, {status: loadingStatus}], [value, loadingStatus]); + return useMemo>(() => [result, {status: loadingStatus}], [result, loadingStatus]); } export default useOnyx; diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 621ba3bf8..aa10cab0c 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -978,5 +978,87 @@ describe('useOnyx', () => { expect(result.current[0]).toEqual('cached'); expect(result.current[1].status).toEqual('loaded'); }); + + it('should not flip back to loading for a merge queued after the hook has already connected', async () => { + await Onyx.set(ONYXKEYS.TEST_KEY, 'existing'); + + const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + + // Let the hook complete its first connection. + await act(async () => waitForPromisesToResolve()); + expect(result.current[0]).toEqual('existing'); + expect(result.current[1].status).toEqual('loaded'); + + // A merge queued after the hook has connected is an optimistic update — status must stay loaded + // so already-shown data is never blanked mid-interaction. + await act(async () => { + Onyx.merge(ONYXKEYS.TEST_KEY, 'updated'); + await waitForPromisesToResolve(); + }); + expect(result.current[0]).toEqual('updated'); + expect(result.current[1].status).toEqual('loaded'); + }); + + it('should not re-enter loading after the value is cleared while connected', async () => { + await Onyx.set(ONYXKEYS.TEST_KEY, 'existing'); + + const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + + await act(async () => waitForPromisesToResolve()); + expect(result.current[1].status).toEqual('loaded'); + + // Clear the value and queue a merge to repopulate it. The hook has already connected, so this is + // not a first connection and status must stay loaded — a value going away does not re-trigger loading. + await act(async () => { + Onyx.set(ONYXKEYS.TEST_KEY, null); + Onyx.merge(ONYXKEYS.TEST_KEY, 'again'); + await waitForPromisesToResolve(); + }); + expect(result.current[0]).toEqual('again'); + expect(result.current[1].status).toEqual('loaded'); + }); + + it('should report loading with a selector while a merge is pending, then return the selected value', async () => { + const selector = ((entry: OnyxEntry<{id: string}>) => entry?.id) as UseOnyxSelector; + + Onyx.merge(ONYXKEYS.TEST_KEY, {id: 'abc'}); + + const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY, {selector})); + + expect(result.current[0]).toBeUndefined(); + expect(result.current[1].status).toEqual('loading'); + + await act(async () => waitForPromisesToResolve()); + + expect(result.current[0]).toEqual('abc'); + expect(result.current[1].status).toEqual('loaded'); + }); + }); + + describe('clear', () => { + it('should return the cleared value for both existing and newly-connected subscribers, then propagate a later merge', async () => { + await Onyx.set(ONYXKEYS.TEST_KEY, 'test'); + + const {result: existing} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + await act(async () => waitForPromisesToResolve()); + expect(existing.current[0]).toEqual('test'); + + await act(async () => Onyx.clear()); + + // A subscriber that connects after the clear sees the cleared value, not stale data. + const {result: fresh} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + await act(async () => waitForPromisesToResolve()); + + expect(existing.current[0]).toBeUndefined(); + expect(existing.current[1].status).toEqual('loaded'); + expect(fresh.current[0]).toBeUndefined(); + expect(fresh.current[1].status).toEqual('loaded'); + + // A merge after the clear reaches both the pre-clear and post-clear subscribers. + await act(async () => Onyx.merge(ONYXKEYS.TEST_KEY, 'test2')); + + expect(existing.current[0]).toEqual('test2'); + expect(fresh.current[0]).toEqual('test2'); + }); }); }); From 11bd40cdabe3a4c6e0d5b8adc21b6a71a41d42fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 28 Aug 2026 20:08:30 +0100 Subject: [PATCH 06/12] Simplify comments and remove snapshot wording --- lib/OnyxStore.ts | 78 ++++++++++++++++++------------------- lib/OnyxUtils.ts | 12 +++--- tests/unit/OnyxStoreTest.ts | 48 +++++++++++------------ 3 files changed, 67 insertions(+), 71 deletions(-) diff --git a/lib/OnyxStore.ts b/lib/OnyxStore.ts index 449c0751f..e1cd33ceb 100644 --- a/lib/OnyxStore.ts +++ b/lib/OnyxStore.ts @@ -4,23 +4,20 @@ import * as Logger from './Logger'; import type {CollectionKeyBase, KeyValueMapping, OnyxCollection, OnyxKey, OnyxValue} from './types'; /** - * Listener fired when an exact key's value changes. For collection root keys this is the - * snapshot-mode listener: receives the frozen collection snapshot every time a member changes. + * Listener fired when an exact key's value changes. For a collection root key this is the + * collection listener: it receives the frozen collection object every time a member changes. */ type KeyListener = (value: OnyxValue, key: TKey) => void; /** - * `OnyxStore` is a single listener registry for Onyx reads/subscriptions. It replaces the - * connection manager's several per-subscription bookkeeping structures with one index: + * `OnyxStore` is a single listener registry for Onyx reads/subscriptions. One index backs + * every subscription: * - * keyListeners — listeners on an exact key (a single key, a collection root in snapshot - * mode, or a specific collection member). + * keyListeners: exact-key listeners (a single key, a collection root in collection mode, + * or a specific collection member). * * Write paths call `notifyKey()` (single-key write) or `notifyCollection()` (batch collection * update from `mergeCollection`/`setCollection`/`clear`). - * - * NOTE: This module is introduced inert — nothing calls it yet. The subscription/notification - * paths (`Onyx.connect`, `useOnyx`, `OnyxUtils.notify*`) are wired onto it in a later change. */ class OnyxStore { private keyListeners: Map>; @@ -30,7 +27,7 @@ class OnyxStore { } /** - * Sync, cache-only read. Returns the frozen collection snapshot for collection + * Sync, cache-only read. Returns the frozen collection object for collection * keys, the cached value for single keys, or `undefined` if not in cache. */ getState(key: TKey): OnyxValue { @@ -41,10 +38,9 @@ class OnyxStore { } /** - * Subscribe to an exact key. For collection root keys this is "snapshot mode" — - * the listener fires with the frozen collection snapshot whenever any member - * changes. For collection member keys or regular keys, the listener fires when - * that specific key's value changes. + * Subscribe to an exact key. For a collection root key this is "collection mode": the + * listener fires with the frozen collection object whenever any member changes. For a + * collection member key or a regular key, the listener fires when that key's value changes. * * Returns an unsubscribe function. */ @@ -71,15 +67,15 @@ class OnyxStore { * Notify of a single-key write. * * Dispatch: - * 1. keyListeners.get(key) — exact-key subscribers (always fires) - * 2. If key is a collection member: keyListeners.get(collectionKey) — snapshot - * subscribers for the parent collection (unless suppressed). + * 1. keyListeners.get(key): exact-key subscribers (always fires). + * 2. If key is a collection member, keyListeners.get(collectionKey): collection + * listeners for the parent collection (unless suppressed). * - * `options.suppressCollectionSnapshot` skips step 2 — used by collection-batch - * write paths so each member-write doesn't re-trigger the collection-level - * snapshot listeners; the outer `notifyCollection()` fires those once. + * `options.suppressCollectionNotify` skips step 2. Collection-batch write paths set + * it so each member write doesn't re-trigger the collection-level listeners; + * the outer `notifyCollection()` fires those once. */ - notifyKey(key: TKey, value: OnyxValue, options?: {suppressCollectionSnapshot?: boolean}): void { + notifyKey(key: TKey, value: OnyxValue, options?: {suppressCollectionNotify?: boolean}): void { // 1. Exact-key listeners const exact = this.keyListeners.get(key); if (exact && exact.size > 0) { @@ -88,17 +84,17 @@ class OnyxStore { } } - // 2. Collection-level snapshot routing — only fires when the write is to a member key. - // Direct writes to a collection root (e.g. `Onyx.merge(COLLECTION_KEY, ...)`) are - // an unsupported anti-pattern — treat them as opaque single-key writes. + // 2. Collection-level routing. Only fires when the write is to a member key. + // Direct writes to a collection root (e.g. `Onyx.merge(COLLECTION_KEY, ...)`) are an + // unsupported anti-pattern; treat them as opaque single-key writes. const collectionKey = OnyxKeys.getCollectionKey(key); const isCollectionMemberWrite = collectionKey !== undefined && collectionKey !== key; - if (isCollectionMemberWrite && !options?.suppressCollectionSnapshot) { - const snapshotListeners = this.keyListeners.get(collectionKey); - if (snapshotListeners && snapshotListeners.size > 0) { - const snapshot = cache.getCollectionData(collectionKey); - for (const listener of snapshotListeners) { - this.safeInvoke(() => listener(snapshot as OnyxValue, collectionKey), collectionKey); + if (isCollectionMemberWrite && !options?.suppressCollectionNotify) { + const collectionListeners = this.keyListeners.get(collectionKey); + if (collectionListeners && collectionListeners.size > 0) { + const collectionData = cache.getCollectionData(collectionKey); + for (const listener of collectionListeners) { + this.safeInvoke(() => listener(collectionData as OnyxValue, collectionKey), collectionKey); } } } @@ -109,9 +105,9 @@ class OnyxStore { * `setCollection`, and `clear`'s collection path. * * Dispatch: - * 1. keyListeners.get(collectionKey) — fires ONCE with the new snapshot. - * 2. keyListeners.get(memberKey) — fires per changed member where the value - * differs from the previous (for ref-equality on unchanged members). + * 1. keyListeners.get(collectionKey): fires once with the new collection object. + * 2. keyListeners.get(memberKey): fires per changed member whose value differs from + * the previous, preserving ref-equality on unchanged members. */ notifyCollection( collectionKey: TKey, @@ -124,22 +120,22 @@ class OnyxStore { } const previous = partialPreviousCollection ?? {}; - // Read the merged snapshot once. `cache.getCollectionData()` returns the post-merge + // Read the merged collection once. `cache.getCollectionData()` returns the post-merge // frozen object, which is what listeners should see (not the raw `partialCollection` // input, which is just the delta and lacks fields preserved during merge). - const snapshot = cache.getCollectionData(collectionKey); + const collectionData = cache.getCollectionData(collectionKey); - // 1. Snapshot subscribers fire once with the new snapshot. - const snapshotListeners = this.keyListeners.get(collectionKey); - if (snapshotListeners && snapshotListeners.size > 0) { - for (const listener of snapshotListeners) { - this.safeInvoke(() => listener(snapshot as OnyxValue, collectionKey), collectionKey); + // 1. Collection listeners fire once with the new collection object. + const collectionListeners = this.keyListeners.get(collectionKey); + if (collectionListeners && collectionListeners.size > 0) { + for (const listener of collectionListeners) { + this.safeInvoke(() => listener(collectionData as OnyxValue, collectionKey), collectionKey); } } // 2. Exact-member subscribers fire per changed key (skip if ref unchanged vs previous). for (const memberKey of changedKeys) { - const value = snapshot?.[memberKey]; + const value = collectionData?.[memberKey]; const prev = previous[memberKey]; if (value === prev) { continue; diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 6b1b02b3c..c33daaee3 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -514,11 +514,11 @@ function getCachedCollection(collectionKey: TKey * that also performs LRU bookkeeping for eviction. Write paths call this instead * of touching the subscriber registry directly. * - * Pass `suppressCollectionSnapshot: true` when notifying within a collection-batch - * operation — the outer `notifyCollection()` fires snapshot listeners once, so + * Pass `suppressCollectionNotify: true` when notifying within a collection-batch + * operation. The outer `notifyCollection()` fires collection listeners once, so * each per-key fire shouldn't re-trigger them. */ -function notifyKey(key: TKey, value: OnyxValue, options?: {suppressCollectionSnapshot?: boolean}): void { +function notifyKey(key: TKey, value: OnyxValue, options?: {suppressCollectionNotify?: boolean}): void { if (value !== null && value !== undefined) { cache.addLastAccessedKey(key, OnyxKeys.isCollectionKey(key)); } else { @@ -552,12 +552,12 @@ function notifyCollection( /** * Remove a key from Onyx and update the subscribers. * - * `suppressCollectionSnapshot` skips the collection-level snapshot fire — used by + * `suppressCollectionNotify` skips the collection-level fire. Used by * `prepareKeyValuePairsForStorage()` when called inside a collection-batch operation * (setCollection/mergeCollection/partialSetCollection/multiSet's collection batch), - * because the outer `notifyCollection()` fires snapshot listeners once. + * because the outer `notifyCollection()` fires collection listeners once. */ -function remove(key: TKey, options?: {suppressCollectionSnapshot?: boolean}): Promise { +function remove(key: TKey, options?: {suppressCollectionNotify?: boolean}): Promise { cache.drop(key); notifyKey(key, undefined as OnyxValue, options); diff --git a/tests/unit/OnyxStoreTest.ts b/tests/unit/OnyxStoreTest.ts index e1eb3d1b3..be54297c1 100644 --- a/tests/unit/OnyxStoreTest.ts +++ b/tests/unit/OnyxStoreTest.ts @@ -111,10 +111,10 @@ describe('OnyxStore', () => { }); }); - describe('collection-snapshot routing on notifyKey', () => { - it('should fire the collection-root snapshot listener with the cache snapshot when a member is written', () => { - const snapshot = {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}; - const getCollectionData = jest.spyOn(cache, 'getCollectionData').mockReturnValue(snapshot); + describe('collection routing on notifyKey', () => { + it('should fire the collection-root listener with the cache collection object when a member is written', () => { + const collectionData = {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}; + const getCollectionData = jest.spyOn(cache, 'getCollectionData').mockReturnValue(collectionData); const callback = jest.fn(); onyxStore.subscribe(COLLECTION, callback); @@ -123,37 +123,37 @@ describe('OnyxStore', () => { expect(getCollectionData).toHaveBeenCalledWith(COLLECTION); expect(callback).toHaveBeenCalledTimes(1); - expect(callback).toHaveBeenCalledWith(snapshot, COLLECTION); + expect(callback).toHaveBeenCalledWith(collectionData, COLLECTION); }); - it('should fire both the exact-member listener and the collection-root snapshot listener', () => { - const snapshot = {[MEMBER_1]: {id: 1}}; - jest.spyOn(cache, 'getCollectionData').mockReturnValue(snapshot); + it('should fire both the exact-member listener and the collection-root listener', () => { + const collectionData = {[MEMBER_1]: {id: 1}}; + jest.spyOn(cache, 'getCollectionData').mockReturnValue(collectionData); const memberCallback = jest.fn(); - const snapshotCallback = jest.fn(); + const collectionCallback = jest.fn(); onyxStore.subscribe(MEMBER_1, memberCallback); - onyxStore.subscribe(COLLECTION, snapshotCallback); + onyxStore.subscribe(COLLECTION, collectionCallback); onyxStore.notifyKey(MEMBER_1, {id: 1}); expect(memberCallback).toHaveBeenCalledWith({id: 1}, MEMBER_1); - expect(snapshotCallback).toHaveBeenCalledWith(snapshot, COLLECTION); + expect(collectionCallback).toHaveBeenCalledWith(collectionData, COLLECTION); }); - it('should skip the collection-root listener but still fire the exact-member listener when suppressCollectionSnapshot is set', () => { + it('should skip the collection-root listener but still fire the exact-member listener when suppressCollectionNotify is set', () => { const getCollectionData = jest.spyOn(cache, 'getCollectionData').mockReturnValue({}); const memberCallback = jest.fn(); - const snapshotCallback = jest.fn(); + const collectionCallback = jest.fn(); onyxStore.subscribe(MEMBER_1, memberCallback); - onyxStore.subscribe(COLLECTION, snapshotCallback); + onyxStore.subscribe(COLLECTION, collectionCallback); - onyxStore.notifyKey(MEMBER_1, {id: 1}, {suppressCollectionSnapshot: true}); + onyxStore.notifyKey(MEMBER_1, {id: 1}, {suppressCollectionNotify: true}); expect(memberCallback).toHaveBeenCalledTimes(1); - expect(snapshotCallback).not.toHaveBeenCalled(); - // The snapshot is never read when suppressed. + expect(collectionCallback).not.toHaveBeenCalled(); + // The collection object is never read when suppressed. expect(getCollectionData).not.toHaveBeenCalled(); }); @@ -170,9 +170,9 @@ describe('OnyxStore', () => { }); describe('notifyCollection', () => { - it('should fire the snapshot listener once with the cache snapshot', () => { - const snapshot = {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}; - jest.spyOn(cache, 'getCollectionData').mockReturnValue(snapshot); + it('should fire the collection listener once with the cache collection object', () => { + const collectionData = {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}; + jest.spyOn(cache, 'getCollectionData').mockReturnValue(collectionData); const callback = jest.fn(); onyxStore.subscribe(COLLECTION, callback); @@ -180,13 +180,13 @@ describe('OnyxStore', () => { onyxStore.notifyCollection(COLLECTION, {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}); expect(callback).toHaveBeenCalledTimes(1); - expect(callback).toHaveBeenCalledWith(snapshot, COLLECTION); + expect(callback).toHaveBeenCalledWith(collectionData, COLLECTION); }); it('should fire exact-member listeners only for members whose value reference changed', () => { - const shared = {id: 2}; // same reference in snapshot and previous → should be skipped - const snapshot = {[MEMBER_1]: {id: 1}, [MEMBER_2]: shared}; - jest.spyOn(cache, 'getCollectionData').mockReturnValue(snapshot); + const shared = {id: 2}; // same reference in collection and previous, should be skipped + const collectionData = {[MEMBER_1]: {id: 1}, [MEMBER_2]: shared}; + jest.spyOn(cache, 'getCollectionData').mockReturnValue(collectionData); const member1Callback = jest.fn(); const member2Callback = jest.fn(); From 4d023b4d41bf9c426d23eed3c1e5cf4a3b53f4e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 31 Aug 2026 11:43:16 +0100 Subject: [PATCH 07/12] Simplify comments --- lib/Onyx.ts | 51 +++++++++++++-------------- lib/OnyxCache.ts | 4 +-- lib/OnyxUtils.ts | 10 +++--- lib/types.ts | 15 ++++---- lib/useOnyx.ts | 26 +++++++------- tests/unit/onyxCacheTest.tsx | 4 +-- tests/unit/onyxClearWebStorageTest.ts | 2 +- tests/unit/onyxTest.ts | 41 ++++++++++----------- tests/unit/onyxUtilsTest.ts | 6 ++-- tests/unit/useOnyxTest.ts | 4 +-- 10 files changed, 77 insertions(+), 86 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 975cabc86..3ccf52c8a 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -43,11 +43,11 @@ type Connection = { }; /** - * Shared sentinel for "nothing delivered yet" in `connect()`'s per-subscription dedup. - * A unique Symbol can't collide with any real Onyx value, so the first `Object.is` check - * never matches and the initial fire always runs — even for a key whose genuine first - * value is `undefined`. It only needs to be distinct from real values, not unique per - * subscription, so a single module-level instance is reused by every connection. + * Sentinel for "nothing delivered yet" in `connect()`'s per-subscription dedup. A Symbol + * can't collide with any real Onyx value, so the first `Object.is` check never matches and + * the initial fire runs even when a key's genuine first value is `undefined`. It only needs + * to be distinct from real values, not unique per subscription, so one module-level instance + * is reused by every connection. */ // eslint-disable-next-line rulesdir/no-negated-variables const NOT_DELIVERED = Symbol('NOT_DELIVERED'); @@ -82,8 +82,7 @@ function init({ const collectionBatches = new Map>; previous: NonUndefined>}>(); for (const [key, value] of pairs) { - // RAM-only keys should never sync from storage as they may have stale persisted data - // from before the key was migrated to RAM-only. + // RAM-only keys never sync from storage; any persisted data for them is stale. if (OnyxKeys.isRamOnlyKey(key)) { continue; } @@ -91,7 +90,7 @@ function init({ const collectionKey = OnyxKeys.getCollectionKey(key); const isCollectionMember = !!collectionKey && OnyxKeys.isCollectionMemberKey(collectionKey, key); - // Capture the previous cached value BEFORE cache.set() so notifyCollection() can diff old vs new per member. + // Capture the previous cached value before cache.set() so notifyCollection() can diff old vs new per member. const previousValue = isCollectionMember ? cache.get(key) : undefined; cache.set(key, value); @@ -137,7 +136,7 @@ function init({ } /** - * Sync, cache-only read of an Onyx key. Returns the frozen collection snapshot for + * Sync, cache-only read of an Onyx key. Returns the frozen collection object for * collection keys, the cached value for single keys, or `undefined` if the key isn't * in cache (no storage fallback). * @@ -151,10 +150,10 @@ function getState(key: TKey): OnyxValue { * Defer initial-fire of `Onyx.connect` callbacks far enough that any Onyx writes * scheduled in the same synchronous tick have applied before the callback reads cache. * - * The legacy `subscribeToKey` chain (`deferredInitTask.then(getAllKeys).then(multiGet) + * FIXME: The legacy `subscribeToKey` chain (`deferredInitTask.then(getAllKeys).then(multiGet) * .then(sendDataToConnection)`) reached this depth incidentally via storage I/O. The * new store-based wrapper has no storage chain, so we have to introduce the depth - * explicitly. The three nested `.then()`s match the legacy effective depth — enough + * explicitly. The three nested `.then()`s match the legacy effective depth, enough * to outpace the longest in-flight write chain: `Onyx.update` -> `clearPromise.then` * -> per-item `Onyx.merge` -> `OnyxUtils.get(key).then(applyMerge)` is two hops to * apply, so the third hop guarantees initial-fire reads the post-write cache. @@ -162,7 +161,7 @@ function getState(key: TKey): OnyxValue { * Microtask depth (not `setTimeout(0)`) is required because Jest test bodies run * entirely in microtask land via chained `.then()`s; a macrotask-deferred initial * fire would not run until the chain returns to the event loop, which can be after - * the test's assertions execute — leaving module-level Onyx subscribers stale. + * the test's assertions execute, leaving module-level Onyx subscribers stale. */ function scheduleInitialFire(fn: () => void): void { Promise.resolve() @@ -175,7 +174,7 @@ function scheduleInitialFire(fn: () => void): void { * Subscribe to changes for `key`. * * For a collection root key, the callback fires with the entire frozen collection - * snapshot whenever any member changes; signature `(collection, collectionKey)`. + * object whenever any member changes; signature `(collection, collectionKey)`. * For any other key, the callback fires with the value at that key; signature * `(value, key)`. Initial fire is deferred via `scheduleInitialFire` so it reads * cache after any same-tick writes have applied. @@ -194,32 +193,30 @@ function connect(connectOptions: ConnectOptions): Co } if (OnyxKeys.isCollectionKey(key)) { - // Collection-root snapshot mode — listener fires with the whole snapshot per - // collection change. Callback shape is `(snapshot, key)`. Dedup: skip identical - // snapshot refs. Initial fire always delivers the current snapshot (frozen `{}` - // for an empty-but-known collection, `undefined` only if the collection key has - // not been seen yet). - let lastDeliveredSnapshot: unknown = NOT_DELIVERED; - const deliverSnapshot = (rawSnapshot: OnyxValue | undefined, k: TKey) => { - if (Object.is(lastDeliveredSnapshot, rawSnapshot)) { + // Collection-root mode: dedup skips identical collection refs. Initial fire delivers + // the current collection object: frozen `{}` for an empty-but-known collection, + // `undefined` only if the collection key has not been seen yet. + let lastDeliveredCollection: unknown = NOT_DELIVERED; + const deliverCollection = (rawCollection: OnyxValue | undefined, k: TKey) => { + if (Object.is(lastDeliveredCollection, rawCollection)) { return; } - lastDeliveredSnapshot = rawSnapshot; - (callback as CollectionConnectCallback | undefined)?.(rawSnapshot as NonNullable>, k); + lastDeliveredCollection = rawCollection; + (callback as CollectionConnectCallback | undefined)?.(rawCollection as NonNullable>, k); }; unsubscribeFn = onyxStore.subscribe(key, (value, k) => { - deliverSnapshot(value as unknown as OnyxValue, k as TKey); + deliverCollection(value as unknown as OnyxValue, k as TKey); }); scheduleInitialFire(() => { if (!active) { return; } - deliverSnapshot(onyxStore.getState(key) as unknown as OnyxValue, key as TKey); + deliverCollection(onyxStore.getState(key) as unknown as OnyxValue, key as TKey); }); return; } - // Non-collection key (or a specific collection member) — single-value subscription. + // Non-collection key (or a specific collection member): single-value subscription. let lastDelivered: unknown = NOT_DELIVERED; const deliverValue = (value: OnyxValue, k: TKey | undefined) => { if (Object.is(lastDelivered, value)) { @@ -259,7 +256,7 @@ function connect(connectOptions: ConnectOptions): Co } /** - * Identical to `connect()` — kept for naming consistency with existing call sites. + * Alias of `connect()` for call-site naming consistency. */ function connectWithoutView(connectOptions: ConnectOptions): Connection { return connect(connectOptions); diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index fe2024caf..8128bf017 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -563,9 +563,9 @@ class OnyxCache { const snapshot = this.collectionSnapshots.get(collectionKey); // No entry for this collection key means init hasn't seeded it yet (pre-load), so there's - // genuinely nothing to return. `setCollectionKeys()` (called inside `Onyx.init`) seeds every + // nothing to return. `setCollectionKeys()` (called inside `Onyx.init`) seeds every // known collection with a frozen empty entry, so the presence of an entry is the reliable - // post-init "loaded" signal — and unlike `storageKeys.size > 0`, it doesn't flip back to + // post-init "loaded" signal. Unlike `storageKeys.size > 0`, it doesn't flip back to // "not loaded" after `Onyx.clear()` wipes the storage-keys index. An empty collection is // stored as the shared `FROZEN_EMPTY_COLLECTION` reference (see `rebuildCollectionSnapshot`), // so returning `snapshot` directly hands back that frozen empty object with a quick `===` diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index c33daaee3..8ec1b8f0b 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1155,7 +1155,7 @@ function multiSetWithRetry(data: OnyxMultiSetInput, retryAttempt?: number): Prom const collectionKey = OnyxKeys.getCollectionKey(key); if (collectionKey && OnyxKeys.isCollectionMemberKey(collectionKey, key)) { - // Capture the previous cached value BEFORE calling cache.set() so notifyCollection() + // Capture the previous cached value before calling cache.set() so notifyCollection() // can diff old vs new per-member. const previousValue = cache.get(key); cache.set(key, value); @@ -1200,7 +1200,7 @@ function multiSetWithRetry(data: OnyxMultiSetInput, retryAttempt?: number): Prom } } - // One notifyCollection() per collection — fires each collection-level subscriber once and lets + // One notifyCollection() per collection: fires each collection-level subscriber once and lets // notifyCollection() internally decide which individual member subscribers need notification. // Skip on retry — already notified on attempt 0 (see same-reason comment above). if (!retryAttempt) { @@ -1288,7 +1288,7 @@ function setCollectionWithRetry({collectionKey, const {pairs: keyValuePairs, keysToRemove: removalCandidates} = OnyxUtils.prepareKeyValuePairsForStorage(mutableCollection, true); // Removals of keys that are neither cached nor persisted are no-ops and skipped. const keysToRemove = removalCandidates.filter((key) => cache.get(key) !== undefined || persistedKeys.has(key)); - // Snapshot before cache mutations so notifyCollection() can diff removed members. + // Capture the previous collection before cache mutations so notifyCollection() can diff removed members. const previousCollection = OnyxUtils.getCachedCollection(collectionKey); for (const [key, value] of keyValuePairs) cache.set(key, value); @@ -1462,7 +1462,7 @@ function mergeCollectionWithPatches( ? multiGet(existingKeys).catch((err) => Logger.logInfo(`mergeCollectionWithPatches pre-warm failed; proceeding with cache-only merge. Error: ${err}`)) : Promise.resolve(); return prewarmPromise.then(() => { - // Snapshot previous values from the (now-warm) cache for the subscriber diff, then update + // Capture previous values from the (now-warm) cache for the subscriber diff, then update // cache and notify subscribers synchronously BEFORE issuing storage writes. This matches // the cache-first / storage-second invariant followed by every other Onyx write method // (setWithRetry, applyMerge, setCollectionWithRetry, partialSetCollection, clear), @@ -1568,7 +1568,7 @@ function partialSetCollection({collectionKey, co const {pairs: keyValuePairs, keysToRemove: removalCandidates} = prepareKeyValuePairsForStorage(mutableCollection, true); // Removals of keys that are neither cached nor persisted are no-ops and skipped. const keysToRemove = removalCandidates.filter((key) => cache.get(key) !== undefined || persistedKeys.has(key)); - // Snapshot before cache mutations so notifyCollection() can diff removed members. + // Capture the previous collection before cache mutations so notifyCollection() can diff removed members. const previousCollection = getCachedCollection(collectionKey, existingKeys); for (const [key, value] of keyValuePairs) cache.set(key, value); diff --git a/lib/types.ts b/lib/types.ts index 928376b7c..57516c63b 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -223,11 +223,10 @@ type CollectionConnectCallback = (value: NonUndefined = { /** The Onyx key to subscribe to. */ @@ -236,9 +235,9 @@ type ConnectOptions = { /** * A function that will be called when the Onyx data we are subscribed changes. * - * The value is a conditional *parameter* (collection snapshot vs. entry) inside a single - * function type — rather than a union of two distinct callback types — so that callers using a - * generic or union `TKey` still get an assignable, non-`any` callback. Collection snapshots stay + * The value is a conditional parameter (collection object vs. entry) inside a single + * function type, not a union of two distinct callback types, so that callers using a + * generic or union `TKey` still get an assignable, non-`any` callback. Collection objects stay * `NonUndefined`. */ callback?: (value: TKey extends CollectionKeyBase ? NonUndefined> : OnyxEntry, key: TKey) => void; diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 1cf14dd21..babf2f99d 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -10,18 +10,18 @@ type UseOnyxSelector> = (da type UseOnyxOptions = { /** * Subscribe to a subset of an Onyx key's data. The component re-renders only when the - * selector's output *changes by deep equality* — a selector that allocates a fresh object - * (e.g. `(e) => ({id: e?.id})`) or one whose identity churns every render (an inline - * selector closing over a fresh array) is collapsed to a stable reference internally, so it + * selector's output changes by deep equality. A selector that allocates a fresh object + * (e.g. `(e) => ({id: e?.id})`), or one whose identity churns every render (an inline + * selector closing over a fresh array), is deduped to a stable reference internally, so it * never causes `useSyncExternalStore` to loop and never forces a redundant re-render. */ selector?: UseOnyxSelector; }; /** - * Always `'loaded'` in the store-based design. The type is preserved so existing - * destructures like `const [val, {status}] = useOnyx(KEY)` keep compiling. Will be - * removed in a future cleanup once consumers stop reading it. + * `loading` only on a key's first connection while a merge for it is still in flight; + * `loaded` otherwise. Retained so existing destructures like + * `const [val, {status}] = useOnyx(KEY)` and `isLoadingOnyxValue` consumers keep working. */ type FetchStatus = 'loading' | 'loaded'; @@ -33,16 +33,16 @@ type UseOnyxResult = [NonNullable | undefined, ResultMetadata]; /** * Subscribes a React component to an Onyx key. The component re-renders when the value - * at `key` changes (for collection keys, when any member changes — the returned value is - * the frozen collection snapshot). + * at `key` changes (for a collection key, when any member changes; the returned value is + * the frozen collection object). * - * Returns `[value, {status: 'loaded'}]`. With eager-load + the structural-sharing cache, - * there's no loading phase — the cache always has an answer (a value or "absent"). The - * `status` field is retained for API compatibility and is always `'loaded'`. + * Returns `[value, {status}]`. `status` is `loading` only on a key's first connection while + * a merge for it is still in flight, and `loaded` otherwise. With eager-load and the + * structural-sharing cache the cache otherwise always has an answer (a value or "absent"). * * Selector stability is delegated to React's `useSyncExternalStoreWithSelector`: the selection * is deduped against the last value committed to React (by deep equality when a selector is - * present), and that dedup survives the selector function's *identity* changing every render. + * present), and that dedup survives the selector function's identity changing every render. * So consumers can pass inline selectors that close over freshly allocated arrays/objects * without stabilizing the inputs themselves. Subscriptions without a selector read the raw, * already reference-stable cache value and rely on the default `Object.is` comparison (no @@ -73,7 +73,7 @@ function useOnyx>(key: TKey // `connectedKeyRef` differs from `key` only on that first render; the effect below catches it up, so a // later merge on an already-connected key never surfaces loading. // Reading the ref during render is safe: it's written only in the effect below and re-renders are driven - // by `useSyncExternalStore` and the `key` prop, so it can't cause a missed update — it gates a one-shot signal. + // by `useSyncExternalStore` and the `key` prop, so it can't cause a missed update; it gates a one-shot signal. // eslint-disable-next-line react-hooks/refs const isLoading = connectedKeyRef.current !== key && OnyxUtils.hasPendingMergeForKey(key); const loadingStatus: FetchStatus = isLoading ? 'loading' : 'loaded'; diff --git a/tests/unit/onyxCacheTest.tsx b/tests/unit/onyxCacheTest.tsx index f0d5c5452..4acba0ec7 100644 --- a/tests/unit/onyxCacheTest.tsx +++ b/tests/unit/onyxCacheTest.tsx @@ -868,11 +868,11 @@ describe('Onyx', () => { expect(Object.keys(first!)).toHaveLength(0); }); - it('should return the frozen-empty snapshot for empty collections once init has registered the collection key', async () => { + it('should return the frozen empty collection object for empty collections once init has registered the collection key', async () => { await initOnyx(); // Post-init, a known collection key with no members resolves to the frozen - // empty snapshot — not `undefined`. Returning `{}` reliably across init, + // empty collection object, not `undefined`. Returning `{}` reliably across init, // writes, and `Onyx.clear()` keeps `Onyx.connect({waitForCollectionCallback: true})` // subscribers seeing a consistent "collection is empty" signal instead of // mistakenly skipping the update. diff --git a/tests/unit/onyxClearWebStorageTest.ts b/tests/unit/onyxClearWebStorageTest.ts index b1a9cf8d7..1fccca227 100644 --- a/tests/unit/onyxClearWebStorageTest.ts +++ b/tests/unit/onyxClearWebStorageTest.ts @@ -239,7 +239,7 @@ describe('Set data while storage is clearing', () => { expect(collectionCallback).toHaveBeenCalledTimes(3); // And it should be called with the expected parameters each time. Initial fire - // delivers `{}` (legacy `undefined`-for-empty-initial shim was removed). + // delivers `{}` for a known-but-empty collection. expect(collectionCallback).toHaveBeenNthCalledWith(1, {}, ONYX_KEYS.COLLECTION.TEST); expect(collectionCallback).toHaveBeenNthCalledWith( 2, diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 066feb9da..2d91e8258 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -920,7 +920,7 @@ describe('Onyx', () => { return waitForPromisesToResolve(); }) .then(() => { - // Snapshot mode: multiSet fires the collection callback per write. + // Collection mode: multiSet fires the collection callback per write. expect(mockCallback).toHaveBeenLastCalledWith({test_1: {existingData: 'test'}, test_2: {existingData: 'test'}}, ONYX_KEYS.COLLECTION.TEST_KEY); mockCallback.mockReset(); @@ -1061,9 +1061,8 @@ describe('Onyx', () => { // Then we expect the callback to have called twice, once for the initial connect call + once for the collection update expect(mockCallback).toHaveBeenCalledTimes(2); - // Initial fire delivers the post-init frozen empty collection `{}` (the legacy - // "undefined for empty-on-initial-fire" shim was removed; callers that needed - // that behavior now guard at the consumer level). + // Initial fire delivers the post-init frozen empty collection `{}`. Callers that + // need a different signal for an empty collection guard at the consumer level. expect(mockCallback).toHaveBeenNthCalledWith(1, {}, ONYX_KEYS.COLLECTION.TEST_POLICY); // AND the value for the second call should be collectionUpdate since the collection was updated @@ -1092,9 +1091,8 @@ describe('Onyx', () => { // Then we expect the callback to have called twice, once for the initial connect call + once for the collection update expect(mockCallback).toHaveBeenCalledTimes(2); - // Initial fire delivers `(undefined, key)` — the cache has no entry for - // `testPolicy_1` yet, but we still pass the key. (Legacy `(undefined, undefined)` - // no-match shim was removed.) + // Initial fire delivers `(undefined, key)`: the cache has no entry for + // `testPolicy_1` yet, but we still pass the key. expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, 'testPolicy_1'); // AND the value for the second call should be collectionUpdate since the collection was updated @@ -1122,7 +1120,7 @@ describe('Onyx', () => { // Then we expect the callback to have called twice, once for the initial connect call + once for the collection update expect(mockCallback).toHaveBeenCalledTimes(2); - // Initial fire delivers `{}` (legacy `undefined`-for-empty-initial shim was removed). + // Initial fire delivers `{}` for a known-but-empty collection. expect(mockCallback).toHaveBeenNthCalledWith(1, {}, ONYX_KEYS.COLLECTION.TEST_POLICY); expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY); }) @@ -1200,7 +1198,7 @@ describe('Onyx', () => { expect(collectionCallback).toHaveBeenNthCalledWith(2, {[itemKey]: {a: 'a'}}, ONYX_KEYS.COLLECTION.TEST_UPDATE); expect(testCallback).toHaveBeenCalledTimes(2); - // Initial fire delivers `(undefined, key)` — cache has no entry yet, but we still pass the key. + // Initial fire delivers `(undefined, key)`: cache has no entry yet, but we still pass the key. expect(testCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.TEST_KEY); expect(testCallback).toHaveBeenNthCalledWith(2, 'taco', ONYX_KEYS.TEST_KEY); @@ -1492,7 +1490,7 @@ describe('Onyx', () => { await Onyx.update([{key: cat, value: finalValue, onyxMethod: Onyx.METHOD.MERGE}]); - // Snapshot mode: callback fires with the whole SNAPSHOT-collection snapshot. + // Collection mode: callback fires with the whole SNAPSHOT collection object. expect(callback).toBeCalledTimes(2); expect(callback).toHaveBeenNthCalledWith(1, {[snapshot1]: {data: {[cat]: initialValue}}}, ONYX_KEYS.COLLECTION.SNAPSHOT); expect(callback).toHaveBeenNthCalledWith(2, {[snapshot1]: {data: {[cat]: finalValue}}}, ONYX_KEYS.COLLECTION.SNAPSHOT); @@ -1524,7 +1522,7 @@ describe('Onyx', () => { await Onyx.update([{key: cat, value: finalValue, onyxMethod: Onyx.METHOD.MERGE}]); - // Snapshot mode: callback fires with the whole SNAPSHOT-collection snapshot. + // Collection mode: callback fires with the whole SNAPSHOT collection object. expect(callback).toBeCalledTimes(2); expect(callback).toHaveBeenNthCalledWith(1, {[snapshot1]: {data: {[cat]: initialValue}}}, ONYX_KEYS.COLLECTION.SNAPSHOT); expect(callback).toHaveBeenNthCalledWith( @@ -1672,9 +1670,9 @@ describe('Onyx', () => { }, ]).then(() => { // Initial fire is deferred past in-flight writes via `scheduleInitialFire`, - // so it reads the post-update snapshot. The write-driven fire already - // delivered the same snapshot, so the dedup in `deliverSnapshot` suppresses - // the initial fire — matching legacy timing. + // so it reads the post-update collection. The write-driven fire already + // delivered the same collection, so the dedup in `deliverCollection` suppresses + // the initial fire. expect(routesCollectionCallback).toHaveBeenCalledTimes(1); expect(routesCollectionCallback).toHaveBeenNthCalledWith( 1, @@ -1756,11 +1754,9 @@ describe('Onyx', () => { {onyxMethod: Onyx.METHOD.MERGE, key: lisa, value: {car: 'SUV', age: 21}}, {onyxMethod: Onyx.METHOD.MERGE, key: bob, value: {age: 25}}, ]).then(() => { - // The store-based wrapper always fires an initial callback before the - // post-update callback (the legacy ConnectionManager's deep-promise chain - // suppressed it accidentally). We assert on the final post-update call - // via `toHaveBeenLastCalledWith` instead of pinning specific indices. - // The `sourceValue` 3rd argument was also dropped. + // The wrapper fires an initial callback before the post-update callback, so we + // assert on the final post-update call via `toHaveBeenLastCalledWith` instead of + // pinning specific indices. expect(testCallback).toHaveBeenLastCalledWith({food: 'taco', drink: 'wine'}, ONYX_KEYS.TEST_KEY); expect(otherTestCallback).toHaveBeenLastCalledWith({food: 'pizza', drink: 'water'}, ONYX_KEYS.OTHER_TEST); @@ -3405,10 +3401,9 @@ describe('RAM-only keys should not read from storage', () => { }); await act(async () => waitForPromisesToResolve()); - // Initial fire delivers the post-init frozen `{}` snapshot for a known-but-empty - // collection (legacy `undefined`-for-empty-initial shim was removed). What matters - // for this test is that the RAM-only members have NOT been hydrated from storage — - // the snapshot has no entries, and `cache.get(member)` returns `undefined`. + // Initial fire delivers the post-init frozen `{}` for a known-but-empty collection. + // What matters for this test is that the RAM-only members have not been hydrated from + // storage: the collection has no entries, and `cache.get(member)` returns `undefined`. expect(receivedCollection).toEqual({}); expect(cache.get(collectionMember1)).toBeUndefined(); expect(cache.get(collectionMember2)).toBeUndefined(); diff --git a/tests/unit/onyxUtilsTest.ts b/tests/unit/onyxUtilsTest.ts index dc1931dd6..56b07e8f9 100644 --- a/tests/unit/onyxUtilsTest.ts +++ b/tests/unit/onyxUtilsTest.ts @@ -373,7 +373,7 @@ describe('OnyxUtils', () => { it('should stop firing callbacks for a collection subscriber that disconnects itself mid-batch', async () => { // A collection subscriber (waitForCollectionCallback=false) disconnects itself when - // it receives the first member. Subsequent changed members in the same batch must NOT + // it receives the first member. Subsequent changed members in the same batch must not // trigger further callbacks for this subscriber. const callback = jest.fn(); const connection = Onyx.connect({ @@ -866,7 +866,7 @@ describe('OnyxUtils', () => { // re-enters the failing method on the next attempt. const transientError = new Error('Transient storage error'); - it('mergeCollection — waitForCollectionCallback subscriber fires once across retries', async () => { + it('mergeCollection: waitForCollectionCallback subscriber fires once across retries', async () => { const collectionKey = ONYXKEYS.COLLECTION.TEST_KEY; const existingMemberKey = `${collectionKey}1`; const newMemberKey = `${collectionKey}2`; @@ -889,7 +889,7 @@ describe('OnyxUtils', () => { } as GenericCollection); // Before this fix, every retry attempt re-fired keysChanged() — and - // waitForCollectionCallback subscribers fire on every keysChanged() call by contract. + // waitForCollectionCallback subscribers fire on every notifyCollection() call by contract. // After the fix, retries skip the keysChanged re-fire, so subscribers are notified // exactly once per logical operation. expect(collectionCallback).toHaveBeenCalledTimes(1); diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index aa10cab0c..8697fbfb4 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -989,7 +989,7 @@ describe('useOnyx', () => { expect(result.current[0]).toEqual('existing'); expect(result.current[1].status).toEqual('loaded'); - // A merge queued after the hook has connected is an optimistic update — status must stay loaded + // A merge queued after the hook has connected is an optimistic update, so status must stay loaded // so already-shown data is never blanked mid-interaction. await act(async () => { Onyx.merge(ONYXKEYS.TEST_KEY, 'updated'); @@ -1008,7 +1008,7 @@ describe('useOnyx', () => { expect(result.current[1].status).toEqual('loaded'); // Clear the value and queue a merge to repopulate it. The hook has already connected, so this is - // not a first connection and status must stay loaded — a value going away does not re-trigger loading. + // not a first connection and status must stay loaded; a value going away does not re-trigger loading. await act(async () => { Onyx.set(ONYXKEYS.TEST_KEY, null); Onyx.merge(ONYXKEYS.TEST_KEY, 'again'); From e0a71a426f54322aa31f328c895d7688e3de42fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 1 Sep 2026 08:03:20 +0100 Subject: [PATCH 08/12] Remove dead code --- lib/OnyxUtils.ts | 26 ----- lib/createMemoizedSelector.ts | 38 ------- lib/memoizedShallowEqual.ts | 41 ------- tests/perf-test/OnyxUtils.perf-test.ts | 27 ----- tests/unit/createMemoizedSelectorTest.ts | 129 ----------------------- tests/unit/memoizedShallowEqualTest.ts | 76 ------------- 6 files changed, 337 deletions(-) delete mode 100644 lib/createMemoizedSelector.ts delete mode 100644 lib/memoizedShallowEqual.ts delete mode 100644 tests/unit/createMemoizedSelectorTest.ts delete mode 100644 tests/unit/memoizedShallowEqualTest.ts diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 8ec1b8f0b..4dbc89de3 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -7,7 +7,6 @@ import cache, {TASK} from './OnyxCache'; import OnyxKeys from './OnyxKeys'; import StorageCircuitBreaker from './StorageCircuitBreaker'; import onyxStore from './OnyxStore'; -import * as Str from './Str'; import Storage from './storage'; import {StorageErrorClass} from './storage/errors'; import type { @@ -439,30 +438,6 @@ function getAllKeys(): Promise> { return cache.captureTask(TASK.GET_ALL_KEYS, promise) as Promise>; } -/** - * Tries to get a value from the cache. If the value is not present in cache it will return the default value or undefined. - * If the requested key is a collection, it will return an object with all the collection members. - */ -function tryGetCachedValue(key: TKey): OnyxValue { - let val = cache.get(key); - - if (OnyxKeys.isCollectionKey(key)) { - const collectionData = cache.getCollectionData(key); - if (collectionData !== undefined) { - val = collectionData; - } else { - // If we haven't loaded all keys yet, we can't determine if the collection exists - if (cache.getAllKeys().size === 0) { - return; - } - // Set an empty collection object for collections that exist but have no data - val = {}; - } - } - - return val; -} - function getCachedCollection(collectionKey: TKey, collectionMemberKeys?: string[]): NonNullable> { // Use optimized collection data retrieval when cache is populated const collectionData = cache.getCollectionData(collectionKey); @@ -1631,7 +1606,6 @@ const OnyxUtils = { sendActionToDevTools, get, getAllKeys, - tryGetCachedValue, getCachedCollection, notifyKey, notifyCollection, diff --git a/lib/createMemoizedSelector.ts b/lib/createMemoizedSelector.ts deleted file mode 100644 index 3dbdafbff..000000000 --- a/lib/createMemoizedSelector.ts +++ /dev/null @@ -1,38 +0,0 @@ -import {deepEqual} from 'fast-equals'; - -/** - * Wraps a selector function so that: - * - Calling the wrapper with the same input reference twice short-circuits to the cached output - * (cheap `===` check, no recompute). - * - Calling with a different input that produces a deep-equal output returns the *previous* - * output reference, so downstream `===` comparisons treat it as unchanged. - * - * This is the minimum needed for `useSyncExternalStore` to not loop when consumers pass - * inline selectors that allocate fresh objects on every call (e.g. `(e) => ({id: e?.id})`): - * without the deep-equal fallback, every `getSnapshot` would return a new reference and React - * would re-render (or throw "getSnapshot should be cached") indefinitely. - * - * Stateful by design — each call to `createMemoizedSelector` produces an independent wrapper - * with its own `lastInput`/`lastOutput` cache, so a wrapper must not be shared across - * subscriptions that can see different inputs. - */ -function createMemoizedSelector(selector: (input: TInput) => TOutput): (input: TInput) => TOutput { - let lastInput: TInput; - let lastOutput: TOutput; - let hasComputed = false; - - return (input) => { - if (hasComputed && lastInput === input) { - return lastOutput; - } - const next = selector(input); - lastInput = input; - if (!hasComputed || !deepEqual(lastOutput, next)) { - lastOutput = next; - hasComputed = true; - } - return lastOutput; - }; -} - -export default createMemoizedSelector; diff --git a/lib/memoizedShallowEqual.ts b/lib/memoizedShallowEqual.ts deleted file mode 100644 index 4a015f3d9..000000000 --- a/lib/memoizedShallowEqual.ts +++ /dev/null @@ -1,41 +0,0 @@ -import {shallowEqual} from 'fast-equals'; - -/** - * Memoizes shallowEqual verdicts by the identity of the compared objects. Onyx values are - * treated as immutable (merge/set replace objects, never mutate), so a (prev, next) reference - * pair always yields the same verdict. In the hot case — N no-selector hooks on the same big - * key — every hook compares the exact same two cache-owned objects, so the first hook pays for - * the O(keys) walk and the rest resolve in O(1). WeakMap keys make stale entries impossible to - * read (lookup requires holding both exact objects) and let GC reclaim them. - */ -const shallowEqualVerdicts = new WeakMap>(); - -/** - * Identity-pair-memoized shallowEqual: same (a, b) references → cached verdict, no walk. - */ -function memoizedShallowEqual(a: unknown, b: unknown): boolean { - // Only object pairs are memoizable (WeakMap keys) — anything else is O(1) to compare anyway. - if (typeof a !== 'object' || a === null || typeof b !== 'object' || b === null) { - return shallowEqual(a, b); - } - - let verdictsForA = shallowEqualVerdicts.get(a); - - if (!verdictsForA) { - verdictsForA = new WeakMap(); - shallowEqualVerdicts.set(a, verdictsForA); - } - - const cachedVerdict = verdictsForA.get(b); - - if (cachedVerdict !== undefined) { - return cachedVerdict; - } - - const verdict = shallowEqual(a, b); - verdictsForA.set(b, verdict); - - return verdict; -} - -export default memoizedShallowEqual; diff --git a/tests/perf-test/OnyxUtils.perf-test.ts b/tests/perf-test/OnyxUtils.perf-test.ts index c3a1fa89f..2a04d5ed2 100644 --- a/tests/perf-test/OnyxUtils.perf-test.ts +++ b/tests/perf-test/OnyxUtils.perf-test.ts @@ -197,33 +197,6 @@ describe('OnyxUtils', () => { }); }); - describe('tryGetCachedValue', () => { - const key = `${collectionKey}0`; - const reportAction = mockedReportActionsMap[`${collectionKey}0`]; - const collections = { - ...getRandomReportActions(ONYXKEYS.COLLECTION.TEST_KEY_2), - ...getRandomReportActions(collectionKey), - }; - - test('one call passing normal key', async () => { - await measureFunction(() => OnyxUtils.tryGetCachedValue(key), { - beforeEach: async () => { - await Onyx.set(key, reportAction); - }, - afterEach: clearOnyxAfterEachMeasure, - }); - }); - - test('one call passing collection key', async () => { - await measureFunction(() => OnyxUtils.tryGetCachedValue(collectionKey), { - beforeEach: async () => { - await Onyx.multiSet(collections); - }, - afterEach: clearOnyxAfterEachMeasure, - }); - }); - }); - describe('removeLastAccessedKey', () => { test('one call removing one key', async () => { await measureFunction(() => OnyxCache.removeLastAccessedKey(`${collectionKey}5000`), { diff --git a/tests/unit/createMemoizedSelectorTest.ts b/tests/unit/createMemoizedSelectorTest.ts deleted file mode 100644 index 445911181..000000000 --- a/tests/unit/createMemoizedSelectorTest.ts +++ /dev/null @@ -1,129 +0,0 @@ -import createMemoizedSelector from '../../lib/createMemoizedSelector'; - -describe('createMemoizedSelector', () => { - it('computes the output on the first call', () => { - const selector = jest.fn((input: number) => input * 2); - const memoized = createMemoizedSelector(selector); - - expect(memoized(21)).toBe(42); - expect(selector).toHaveBeenCalledTimes(1); - }); - - it('short-circuits without recomputing when called with the same input reference', () => { - const input = {value: 1}; - const selector = jest.fn((data: {value: number}) => ({doubled: data.value * 2})); - const memoized = createMemoizedSelector(selector); - - const first = memoized(input); - const second = memoized(input); - - // Same input reference → selector not called again, same output reference returned. - expect(selector).toHaveBeenCalledTimes(1); - expect(second).toBe(first); - }); - - it('recomputes when the input reference changes', () => { - const selector = jest.fn((data: {value: number}) => data.value * 10); - const memoized = createMemoizedSelector(selector); - - expect(memoized({value: 1})).toBe(10); - expect(memoized({value: 2})).toBe(20); - expect(selector).toHaveBeenCalledTimes(2); - }); - - it('returns the previous output reference when a new input produces a deep-equal output', () => { - // New object input every call, but the selector output is structurally identical. - const selector = (data: {id: number; name: string}) => ({id: data.id}); - const memoized = createMemoizedSelector(selector); - - const first = memoized({id: 1, name: 'a'}); - const second = memoized({id: 1, name: 'b'}); // different input, deep-equal output {id: 1} - - // Output is deep-equal, so the *previous* reference is preserved for `===` consumers. - expect(second).toBe(first); - expect(second).toEqual({id: 1}); - }); - - it('returns a new output reference when a new input produces a deep-unequal output', () => { - const selector = (data: {id: number}) => ({id: data.id}); - const memoized = createMemoizedSelector(selector); - - const first = memoized({id: 1}); - const second = memoized({id: 2}); - - expect(second).not.toBe(first); - expect(second).toEqual({id: 2}); - }); - - it('preserves the output reference across an A → B(deep-equal A) → A sequence', () => { - const selector = (data: {id: number; extra: string}) => ({id: data.id}); - const memoized = createMemoizedSelector(selector); - - const a = memoized({id: 1, extra: 'x'}); - const b = memoized({id: 1, extra: 'y'}); // deep-equal output, keeps `a` - const c = memoized({id: 1, extra: 'z'}); // deep-equal output, keeps `a` - - expect(b).toBe(a); - expect(c).toBe(a); - }); - - it('handles primitive outputs', () => { - const selector = jest.fn((data: {n: number}) => data.n > 0); - const memoized = createMemoizedSelector(selector); - - expect(memoized({n: 1})).toBe(true); - // Different input, same boolean output — deepEqual(true, true) is true, value preserved. - expect(memoized({n: 5})).toBe(true); - expect(memoized({n: -1})).toBe(false); - expect(selector).toHaveBeenCalledTimes(3); - }); - - it('handles undefined input and undefined output', () => { - const selector = jest.fn((data: {x: number} | undefined) => data?.x); - const memoized = createMemoizedSelector(selector); - - expect(memoized(undefined)).toBeUndefined(); - // Same undefined input reference → short-circuits. - expect(memoized(undefined)).toBeUndefined(); - expect(selector).toHaveBeenCalledTimes(1); - }); - - it('treats the first call as a real computation even when the output is undefined', () => { - const selector = jest.fn(() => undefined); - const memoized = createMemoizedSelector(selector); - - const input1 = {a: 1}; - const input2 = {a: 2}; - - expect(memoized(input1)).toBeUndefined(); - expect(memoized(input2)).toBeUndefined(); - // Both inputs differ by reference, but both outputs are undefined (deep-equal) — recomputed - // on the second call, then collapsed to the preserved reference (both undefined anyway). - expect(selector).toHaveBeenCalledTimes(2); - }); - - it('keeps independent caches per wrapper instance', () => { - const selectorA = jest.fn((n: number) => n + 1); - const selectorB = jest.fn((n: number) => n + 100); - const memoizedA = createMemoizedSelector(selectorA); - const memoizedB = createMemoizedSelector(selectorB); - - expect(memoizedA(1)).toBe(2); - expect(memoizedB(1)).toBe(101); - expect(selectorA).toHaveBeenCalledTimes(1); - expect(selectorB).toHaveBeenCalledTimes(1); - }); - - it('preserves nested object reference identity on deep-equal recompute', () => { - const selector = (data: {id: number}) => ({meta: {id: data.id}, items: [data.id]}); - const memoized = createMemoizedSelector(selector); - - const first = memoized({id: 7}); - const second = memoized({id: 7}); // new input ref, deep-equal output - - // Whole output reference preserved, so nested members are reference-stable too. - expect(second).toBe(first); - expect(second.meta).toBe(first.meta); - expect(second.items).toBe(first.items); - }); -}); diff --git a/tests/unit/memoizedShallowEqualTest.ts b/tests/unit/memoizedShallowEqualTest.ts deleted file mode 100644 index 15b76159d..000000000 --- a/tests/unit/memoizedShallowEqualTest.ts +++ /dev/null @@ -1,76 +0,0 @@ -import memoizedShallowEqual from '../../lib/memoizedShallowEqual'; - -describe('memoizedShallowEqual', () => { - describe('shallowEqual semantics', () => { - it('should return true for the same reference', () => { - const obj = {a: 1}; - expect(memoizedShallowEqual(obj, obj)).toBe(true); - }); - - it('should return true for different references with shallowly-equal content', () => { - const member = {name: 'John'}; - expect(memoizedShallowEqual({a: 1, member}, {a: 1, member})).toBe(true); - }); - - it('should return false when a top-level value differs', () => { - expect(memoizedShallowEqual({a: 1}, {a: 2})).toBe(false); - }); - - it('should return false when key counts differ', () => { - expect(memoizedShallowEqual({a: 1}, {a: 1, b: 2})).toBe(false); - }); - - it('should return false for equal deep content with different nested references', () => { - // Shallow, not deep: nested objects are compared by reference. - expect(memoizedShallowEqual({member: {name: 'John'}}, {member: {name: 'John'}})).toBe(false); - }); - - it('should handle non-object inputs', () => { - expect(memoizedShallowEqual(undefined, undefined)).toBe(true); - expect(memoizedShallowEqual(undefined, {})).toBe(false); - expect(memoizedShallowEqual('a', 'a')).toBe(true); - expect(memoizedShallowEqual('a', 'b')).toBe(false); - expect(memoizedShallowEqual(1, 1)).toBe(true); - expect(memoizedShallowEqual(NaN, NaN)).toBe(true); - }); - - it('should handle arrays', () => { - expect(memoizedShallowEqual([1, 2], [1, 2])).toBe(true); - expect(memoizedShallowEqual([1, 2], [1, 3])).toBe(false); - }); - }); - - describe('memoization', () => { - it('should return the cached verdict for the same object pair without re-comparing', () => { - const a = {name: 'John'}; - const b = {name: 'Jane'}; - expect(memoizedShallowEqual(a, b)).toBe(false); - - // Mutate `b` so the objects are now content-equal. Onyx values are immutable, - // so the memo is expected to keep returning the verdict computed for this exact - // (a, b) pair — proving the second call resolved from the cache, not a re-compare. - b.name = 'John'; - expect(memoizedShallowEqual(a, b)).toBe(false); - }); - - it('should cache verdicts per pair, not per object', () => { - const a = {x: 1}; - const equalToA = {x: 1}; - const differentFromA = {x: 2}; - - expect(memoizedShallowEqual(a, equalToA)).toBe(true); - expect(memoizedShallowEqual(a, differentFromA)).toBe(false); - - // Both verdicts are retained independently for the same `a`. - expect(memoizedShallowEqual(a, equalToA)).toBe(true); - expect(memoizedShallowEqual(a, differentFromA)).toBe(false); - }); - - it('should not memoize non-object inputs', () => { - // Primitives cannot be WeakMap keys; these calls must not throw and must compare directly. - expect(memoizedShallowEqual(1, {})).toBe(false); - expect(memoizedShallowEqual({}, 1)).toBe(false); - expect(memoizedShallowEqual(null, null)).toBe(true); - }); - }); -}); From bfb01441de07274bcdf267141ac318300f16f45d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 1 Sep 2026 08:10:03 +0100 Subject: [PATCH 09/12] Simplify comments --- lib/useOnyx.ts | 51 ++++++++++++++------------------------------------ 1 file changed, 14 insertions(+), 37 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index babf2f99d..9f0e0deb7 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -9,19 +9,14 @@ type UseOnyxSelector> = (da type UseOnyxOptions = { /** - * Subscribe to a subset of an Onyx key's data. The component re-renders only when the - * selector's output changes by deep equality. A selector that allocates a fresh object - * (e.g. `(e) => ({id: e?.id})`), or one whose identity churns every render (an inline - * selector closing over a fresh array), is deduped to a stable reference internally, so it - * never causes `useSyncExternalStore` to loop and never forces a redundant re-render. + * Select a subset of the key's data. Re-renders only when the selector's output changes by deep + * equality, so an inline selector that allocates fresh objects/arrays each render is safe. */ selector?: UseOnyxSelector; }; /** - * `loading` only on a key's first connection while a merge for it is still in flight; - * `loaded` otherwise. Retained so existing destructures like - * `const [val, {status}] = useOnyx(KEY)` and `isLoadingOnyxValue` consumers keep working. + * `loading` only on a key's first connection while a merge is still in flight, else `loaded`. */ type FetchStatus = 'loading' | 'loaded'; @@ -32,48 +27,32 @@ type ResultMetadata = { type UseOnyxResult = [NonNullable | undefined, ResultMetadata]; /** - * Subscribes a React component to an Onyx key. The component re-renders when the value - * at `key` changes (for a collection key, when any member changes; the returned value is - * the frozen collection object). + * Subscribes a component to an Onyx key, re-rendering when the value changes (for a collection key, + * when any member changes; the value is the frozen collection object). Returns `[value, {status}]`, + * `status` `loading` only on the first connection while a merge is in flight. * - * Returns `[value, {status}]`. `status` is `loading` only on a key's first connection while - * a merge for it is still in flight, and `loaded` otherwise. With eager-load and the - * structural-sharing cache the cache otherwise always has an answer (a value or "absent"). - * - * Selector stability is delegated to React's `useSyncExternalStoreWithSelector`: the selection - * is deduped against the last value committed to React (by deep equality when a selector is - * present), and that dedup survives the selector function's identity changing every render. - * So consumers can pass inline selectors that close over freshly allocated arrays/objects - * without stabilizing the inputs themselves. Subscriptions without a selector read the raw, - * already reference-stable cache value and rely on the default `Object.is` comparison (no - * deep-equal cost). + * Selection is delegated to `useSyncExternalStoreWithSelector`, whose dedup survives the selector's + * identity changing every render, so consumers can pass inline selectors without stabilizing them. */ function useOnyx>(key: TKey, options?: UseOnyxOptions): UseOnyxResult { const selector = options?.selector; - // Tracks the key this hook has already connected to, so we can tell a key's first render apart from - // later ones (see the loading-status gate below). Starts null so the initial mount counts as first. + // First-render marker for the loading gate below. const connectedKeyRef = useRef(null); const subscribe = useCallback((onStoreChange: () => void) => onyxStore.subscribe(key, onStoreChange), [key]); const getSnapshot = useCallback(() => onyxStore.getState(key) as OnyxValue | undefined, [key]); - // Normalizes `null` -> `undefined` and applies the consumer's selector (or passes the raw value - // through). Re-created only when the selector's identity changes; the committed-value dedup inside - // `useSyncExternalStoreWithSelector` is what makes a churning identity harmless. const select = useCallback((data: OnyxValue | undefined): TReturnValue | undefined => (selector ? selector(data) : (data as TReturnValue | undefined)) ?? undefined, [selector]); - // With a selector, dedupe the (possibly freshly allocated) output by deep equality. Without one, - // the raw cache value is already reference-stable, so the default `Object.is` is enough. + // Deep-equal only with a selector (its output may be freshly allocated); raw values are ref-stable. const isEqual = selector ? deepEqual : undefined; const value = useSyncExternalStoreWithSelector | undefined, TReturnValue | undefined>(subscribe, getSnapshot, undefined, select, isEqual); - // `loading` only on a key's first render (mount or key change) when a merge is still in flight for it. - // `connectedKeyRef` differs from `key` only on that first render; the effect below catches it up, so a - // later merge on an already-connected key never surfaces loading. - // Reading the ref during render is safe: it's written only in the effect below and re-renders are driven - // by `useSyncExternalStore` and the `key` prop, so it can't cause a missed update; it gates a one-shot signal. + // Loading only on a key's first render while a merge is in flight; the effect below advances + // connectedKeyRef so a later merge never re-surfaces it. Reading the ref in render is safe: it + // gates this one-shot signal only, and re-renders are driven by SES and the key prop. // eslint-disable-next-line react-hooks/refs const isLoading = connectedKeyRef.current !== key && OnyxUtils.hasPendingMergeForKey(key); const loadingStatus: FetchStatus = isLoading ? 'loading' : 'loaded'; @@ -82,11 +61,9 @@ function useOnyx>(key: TKey connectedKeyRef.current = key; }, [key]); - // While loading, the pending merge's result isn't in cache yet, so surface `undefined` until it applies. + // Blank the value while loading: the pending merge isn't in cache yet. const result = isLoading ? undefined : (value as NonNullable | undefined); - // Stable result tuple: re-built only when the (already deduped) `result` reference or the primitive - // `loadingStatus` changes, so render-to-render the same cached tuple (and metadata object) is returned. return useMemo>(() => [result, {status: loadingStatus}], [result, loadingStatus]); } From 3fd8b9142f844b8718a281a767fccc9898a5b143 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 1 Sep 2026 08:47:08 +0100 Subject: [PATCH 10/12] Fix useOnyx stuck on loading when a merge is pending for an already-cached key --- lib/useOnyx.ts | 20 ++++++++++++-------- tests/unit/useOnyxTest.ts | 22 ++++++++++++++++++++-- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 9f0e0deb7..72fc2e9f9 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -1,9 +1,12 @@ -import {useCallback, useEffect, useMemo, useRef} from 'react'; import {deepEqual} from 'fast-equals'; +import {useCallback, useEffect, useMemo, useRef} from 'react'; import {useSyncExternalStoreWithSelector} from 'use-sync-external-store/with-selector'; + +import type {OnyxKey, OnyxValue} from './types'; + +import cache from './OnyxCache'; import onyxStore from './OnyxStore'; import OnyxUtils from './OnyxUtils'; -import type {OnyxKey, OnyxValue} from './types'; type UseOnyxSelector> = (data: OnyxValue | undefined) => TReturnValue; @@ -16,7 +19,8 @@ type UseOnyxOptions = { }; /** - * `loading` only on a key's first connection while a merge is still in flight, else `loaded`. + * `loading` only on a key's first connection while a merge is in flight and nothing is cached yet + * (the merge will produce the first value); `loaded` otherwise. */ type FetchStatus = 'loading' | 'loaded'; @@ -29,7 +33,7 @@ type UseOnyxResult = [NonNullable | undefined, ResultMetadata]; /** * Subscribes a component to an Onyx key, re-rendering when the value changes (for a collection key, * when any member changes; the value is the frozen collection object). Returns `[value, {status}]`, - * `status` `loading` only on the first connection while a merge is in flight. + * `status` `loading` only on the first connection while a merge is in flight and nothing is cached yet. * * Selection is delegated to `useSyncExternalStoreWithSelector`, whose dedup survives the selector's * identity changing every render, so consumers can pass inline selectors without stabilizing them. @@ -50,11 +54,11 @@ function useOnyx>(key: TKey const value = useSyncExternalStoreWithSelector | undefined, TReturnValue | undefined>(subscribe, getSnapshot, undefined, select, isEqual); - // Loading only on a key's first render while a merge is in flight; the effect below advances - // connectedKeyRef so a later merge never re-surfaces it. Reading the ref in render is safe: it - // gates this one-shot signal only, and re-renders are driven by SES and the key prop. + // Loading only on a key's first render when a merge is in flight and nothing is cached yet. + // A cached key stays loaded, so an optimistic merge never blanks shown data and a no-op merge can't leave it stuck. + // connectedKeyRef limits this to the first render. // eslint-disable-next-line react-hooks/refs - const isLoading = connectedKeyRef.current !== key && OnyxUtils.hasPendingMergeForKey(key); + const isLoading = connectedKeyRef.current !== key && !cache.hasCacheForKey(key) && OnyxUtils.hasPendingMergeForKey(key); const loadingStatus: FetchStatus = isLoading ? 'loading' : 'loaded'; useEffect(() => { diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 8697fbfb4..098ce160d 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -1,10 +1,12 @@ import {act, renderHook} from '@testing-library/react-native'; + import type {OnyxCollection, OnyxEntry, OnyxKey} from '../../lib'; +import type {UseOnyxSelector} from '../../lib/useOnyx'; +import type GenericCollection from '../utils/GenericCollection'; + import Onyx, {useOnyx} from '../../lib'; import StorageMock from '../../lib/storage'; -import type GenericCollection from '../utils/GenericCollection'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; -import type {UseOnyxSelector} from '../../lib/useOnyx'; const ONYXKEYS = { TEST_KEY: 'test', @@ -1033,6 +1035,22 @@ describe('useOnyx', () => { expect(result.current[0]).toEqual('abc'); expect(result.current[1].status).toEqual('loaded'); }); + + it('should show a cached value as loaded when a merge is pending on first render', async () => { + await Onyx.set(ONYXKEYS.TEST_KEY, {a: 1}); + + Onyx.merge(ONYXKEYS.TEST_KEY, {a: 1}); + + const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + + expect(result.current[0]).toEqual({a: 1}); + expect(result.current[1].status).toEqual('loaded'); + + await act(async () => waitForPromisesToResolve()); + + expect(result.current[0]).toEqual({a: 1}); + expect(result.current[1].status).toEqual('loaded'); + }); }); describe('clear', () => { From ca08a9de145325e25991ce5c8a4787fe9941699d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 3 Sep 2026 16:12:49 +0100 Subject: [PATCH 11/12] Simplify comments and types --- lib/OnyxStore.ts | 72 +++++++++++++++++++++++++++++++----------------- 1 file changed, 47 insertions(+), 25 deletions(-) diff --git a/lib/OnyxStore.ts b/lib/OnyxStore.ts index e1cd33ceb..d9e14af8f 100644 --- a/lib/OnyxStore.ts +++ b/lib/OnyxStore.ts @@ -1,33 +1,45 @@ +import type {CollectionKeyBase, KeyValueMapping, OnyxCollection, OnyxKey, OnyxValue} from './types'; + +import * as Logger from './Logger'; import cache from './OnyxCache'; import OnyxKeys from './OnyxKeys'; -import * as Logger from './Logger'; -import type {CollectionKeyBase, KeyValueMapping, OnyxCollection, OnyxKey, OnyxValue} from './types'; /** - * Listener fired when an exact key's value changes. For a collection root key this is the - * collection listener: it receives the frozen collection object every time a member changes. + * Listener fired when an exact key's value changes. */ type KeyListener = (value: OnyxValue, key: TKey) => void; +/** + * Storage form of a listener, value erased so one Map can hold listeners for every key type. + */ +type StoredListener = (value: unknown, key: OnyxKey) => void; + +type NotifyKeyOptions = { + /** + * Skips collection-level routing. Collection-batch write paths set it so each member write + * doesn't re-trigger the collection-level listeners; the outer `notifyCollection()` fires those once. + */ + suppressCollectionNotify?: boolean; +}; + /** * `OnyxStore` is a single listener registry for Onyx reads/subscriptions. One index backs * every subscription: * - * keyListeners: exact-key listeners (a single key, a collection root in collection mode, + * keyListeners: exact-key listeners (a single key, a collection object, * or a specific collection member). * - * Write paths call `notifyKey()` (single-key write) or `notifyCollection()` (batch collection - * update from `mergeCollection`/`setCollection`/`clear`). + * Write paths call `notifyKey()` (single-key write) or `notifyCollection()` (batch collection update). */ class OnyxStore { - private keyListeners: Map>; + private keyListeners: Map>; constructor() { this.keyListeners = new Map(); } /** - * Sync, cache-only read. Returns the frozen collection object for collection + * Returns the frozen collection object for collection * keys, the cached value for single keys, or `undefined` if not in cache. */ getState(key: TKey): OnyxValue { @@ -50,13 +62,17 @@ class OnyxStore { listeners = new Set(); this.keyListeners.set(key, listeners); } - listeners.add(listener as unknown as KeyListener); + + listeners.add(listener as StoredListener); + return () => { const set = this.keyListeners.get(key); if (!set) { return; } - set.delete(listener as unknown as KeyListener); + + set.delete(listener as StoredListener); + if (set.size === 0) { this.keyListeners.delete(key); } @@ -69,18 +85,14 @@ class OnyxStore { * Dispatch: * 1. keyListeners.get(key): exact-key subscribers (always fires). * 2. If key is a collection member, keyListeners.get(collectionKey): collection - * listeners for the parent collection (unless suppressed). - * - * `options.suppressCollectionNotify` skips step 2. Collection-batch write paths set - * it so each member write doesn't re-trigger the collection-level listeners; - * the outer `notifyCollection()` fires those once. + * listeners for the parent collection (unless `options.suppressCollectionNotify`). */ - notifyKey(key: TKey, value: OnyxValue, options?: {suppressCollectionNotify?: boolean}): void { + notifyKey(key: TKey, value: OnyxValue, options?: NotifyKeyOptions): void { // 1. Exact-key listeners const exact = this.keyListeners.get(key); if (exact && exact.size > 0) { for (const listener of exact) { - this.safeInvoke(() => listener(value as OnyxValue, key), key); + this.safeInvoke(() => listener(value, key), key); } } @@ -94,15 +106,14 @@ class OnyxStore { if (collectionListeners && collectionListeners.size > 0) { const collectionData = cache.getCollectionData(collectionKey); for (const listener of collectionListeners) { - this.safeInvoke(() => listener(collectionData as OnyxValue, collectionKey), collectionKey); + this.safeInvoke(() => listener(collectionData, collectionKey), collectionKey); } } } } /** - * Notify of a collection-level batch update. Used by `mergeCollection`, - * `setCollection`, and `clear`'s collection path. + * Notify of a collection-level batch update. * * Dispatch: * 1. keyListeners.get(collectionKey): fires once with the new collection object. @@ -129,7 +140,7 @@ class OnyxStore { const collectionListeners = this.keyListeners.get(collectionKey); if (collectionListeners && collectionListeners.size > 0) { for (const listener of collectionListeners) { - this.safeInvoke(() => listener(collectionData as OnyxValue, collectionKey), collectionKey); + this.safeInvoke(() => listener(collectionData, collectionKey), collectionKey); } } @@ -140,33 +151,44 @@ class OnyxStore { if (value === prev) { continue; } + const exact = this.keyListeners.get(memberKey); if (!exact || exact.size === 0) { continue; } + for (const listener of exact) { - this.safeInvoke(() => listener(value as OnyxValue, memberKey), memberKey); + this.safeInvoke(() => listener(value, memberKey), memberKey); } } } - /** Wipe all subscriptions. Used by tests and `Onyx.clear()` follow-on. */ + /** + * Wipe all subscriptions. Used by tests and `Onyx.clear()` follow-on. + */ clearAll(): void { this.keyListeners.clear(); } - /** True if there are any subscribers for the given key (exact or parent collection). */ + /** + * True if there are any subscribers for the given key (exact or parent collection). + */ hasListenersForKey(key: OnyxKey): boolean { if ((this.keyListeners.get(key)?.size ?? 0) > 0) { return true; } + const collectionKey = OnyxKeys.getCollectionKey(key); if (collectionKey && collectionKey !== key && (this.keyListeners.get(collectionKey)?.size ?? 0) > 0) { return true; } + return false; } + /** + * Runs a listener, catching and logging any throw so one failing listener can't stop the rest. + */ private safeInvoke(fn: () => void, contextKey: OnyxKey): void { try { fn(); From 3282f0e81f6eead0bb0e32212d724645846bbc08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 3 Sep 2026 17:07:39 +0100 Subject: [PATCH 12/12] fix: snapshot listener sets before dispatch so subscription changes during a notify only affect later ones --- lib/OnyxStore.ts | 8 +++--- tests/unit/OnyxStoreTest.ts | 49 +++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/lib/OnyxStore.ts b/lib/OnyxStore.ts index d9e14af8f..629350257 100644 --- a/lib/OnyxStore.ts +++ b/lib/OnyxStore.ts @@ -91,7 +91,7 @@ class OnyxStore { // 1. Exact-key listeners const exact = this.keyListeners.get(key); if (exact && exact.size > 0) { - for (const listener of exact) { + for (const listener of [...exact]) { this.safeInvoke(() => listener(value, key), key); } } @@ -105,7 +105,7 @@ class OnyxStore { const collectionListeners = this.keyListeners.get(collectionKey); if (collectionListeners && collectionListeners.size > 0) { const collectionData = cache.getCollectionData(collectionKey); - for (const listener of collectionListeners) { + for (const listener of [...collectionListeners]) { this.safeInvoke(() => listener(collectionData, collectionKey), collectionKey); } } @@ -139,7 +139,7 @@ class OnyxStore { // 1. Collection listeners fire once with the new collection object. const collectionListeners = this.keyListeners.get(collectionKey); if (collectionListeners && collectionListeners.size > 0) { - for (const listener of collectionListeners) { + for (const listener of [...collectionListeners]) { this.safeInvoke(() => listener(collectionData, collectionKey), collectionKey); } } @@ -157,7 +157,7 @@ class OnyxStore { continue; } - for (const listener of exact) { + for (const listener of [...exact]) { this.safeInvoke(() => listener(value, memberKey), memberKey); } } diff --git a/tests/unit/OnyxStoreTest.ts b/tests/unit/OnyxStoreTest.ts index be54297c1..4e2cf2d90 100644 --- a/tests/unit/OnyxStoreTest.ts +++ b/tests/unit/OnyxStoreTest.ts @@ -248,6 +248,55 @@ describe('OnyxStore', () => { }); }); + describe('subscription mutation during dispatch', () => { + it('should fire a listener that unsubscribes and re-subscribes itself during dispatch only once', () => { + let unsubscribe: () => void = jest.fn(); + const callback = jest.fn(() => { + unsubscribe(); + unsubscribe = onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback); + }); + unsubscribe = onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback); + + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'x'); + + expect(callback).toHaveBeenCalledTimes(1); + }); + + it('should not deliver the in-flight notification to a listener added during dispatch', () => { + const lateCallback = jest.fn(); + const firstCallback = jest.fn(() => { + onyxStore.subscribe(ONYXKEYS.TEST_KEY, lateCallback); + }); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, firstCallback); + + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'first'); + expect(lateCallback).not.toHaveBeenCalled(); + + // It receives later notifications normally. + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'second'); + expect(lateCallback).toHaveBeenCalledTimes(1); + expect(lateCallback).toHaveBeenCalledWith('second', ONYXKEYS.TEST_KEY); + }); + + it('should still fire a sibling unsubscribed during dispatch this round, but not on later notifications', () => { + const siblingCallback = jest.fn(); + let unsubscribeSibling: () => void = jest.fn(); + const firstCallback = jest.fn(() => { + unsubscribeSibling(); + }); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, firstCallback); + unsubscribeSibling = onyxStore.subscribe(ONYXKEYS.TEST_KEY, siblingCallback); + + // The sibling was registered when dispatch began, so the snapshot still fires it. + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'first'); + expect(siblingCallback).toHaveBeenCalledTimes(1); + + // Now unsubscribed, it does not fire again. + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'second'); + expect(siblingCallback).toHaveBeenCalledTimes(1); + }); + }); + describe('listener error isolation', () => { it('should log a throwing listener and still fire the other listeners', () => { const logAlertSpy = jest.spyOn(Logger, 'logAlert').mockImplementation(() => {