From 66e0a38ced4db879a4949a9ce879538c6e706560 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 18 May 2026 13:32:17 +0100 Subject: [PATCH 1/8] feat(poc): store-based redesign of subscriber layer Replace OnyxConnectionManager + OnyxSnapshotCache + the subscriber half of OnyxUtils with a single OnyxStore module. Add Onyx.getState/subscribe/ subscribeMembers primitives. Add useOnyxState hook for derived multi-key selectors with a virtual state proxy and manual dependencies. useOnyx is rewritten around the new store: no snapshot cache, no connection manager, no sourceValue, no third positional dependencies arg. Onyx.connect / connectWithoutView kept as thin wrappers over the new primitives so existing call sites compile. Net: -935 lines, +594 lines across lib/. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/Onyx.ts | 213 +++++++++++----- lib/OnyxConnectionManager.ts | 271 -------------------- lib/OnyxSnapshotCache.ts | 154 ------------ lib/OnyxStore.ts | 319 +++++++++++++++++++++++ lib/OnyxUtils.ts | 473 ++++------------------------------- lib/index.ts | 9 +- lib/types.ts | 1 - lib/useLiveRef.ts | 17 -- lib/useOnyx.ts | 347 ++++++------------------- lib/useOnyxState.ts | Bin 0 -> 5993 bytes 10 files changed, 594 insertions(+), 1210 deletions(-) delete mode 100644 lib/OnyxConnectionManager.ts delete mode 100644 lib/OnyxSnapshotCache.ts create mode 100644 lib/OnyxStore.ts delete mode 100644 lib/useLiveRef.ts create mode 100644 lib/useOnyxState.ts diff --git a/lib/Onyx.ts b/lib/Onyx.ts index a8f8f2d4f..218907f9a 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -4,10 +4,13 @@ import Storage from './storage'; import utils from './utils'; import DevTools, {initDevTools} from './DevTools'; import type { + CollectionConnectCallback, CollectionKeyBase, ConnectOptions, + DefaultConnectCallback, InitOptions, KeyValueMapping, + OnyxCollection, OnyxInputKeyValueMapping, MixedOperationsQueue, OnyxKey, @@ -25,10 +28,18 @@ 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()` / `Onyx.subscribe()`. + * Pass it to `Onyx.disconnect()` to stop receiving callbacks for this subscription. + */ +type Connection = { + /** Unsubscribe this connection. Idempotent. */ + unsubscribe: () => void; +}; + /** Initialize the store with actions and listening for storage events */ function init({ keys = {}, @@ -58,13 +69,7 @@ function init({ } cache.set(key, value); - - // Check if this is a collection member key to prevent duplicate callbacks - // When a collection is updated, individual members sync separately to other tabs - // Setting isProcessingCollectionUpdate=true prevents triggering collection callbacks for each individual update - const isKeyCollectionMember = OnyxKeys.isCollectionMember(key); - - OnyxUtils.keyChanged(key, value as OnyxValue, undefined, isKeyCollectionMember); + OnyxUtils.notifyKey(key, value as OnyxValue); }); } @@ -80,73 +85,148 @@ 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.waitForCollectionCallback If set to `true`, it will return the entire collection to the callback as a single object. - * @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. + * Subscribe to a single Onyx key, or to a collection key in snapshot mode. * - * @example - * ```ts - * const connection = Onyx.connectWithoutView({ - * key: ONYXKEYS.SESSION, - * callback: onSessionChange, - * }); - * ``` + * For collection keys, the listener fires with the entire frozen collection snapshot + * every time any member changes. For non-collection keys, the listener fires with the + * value when that key changes. * - * @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.waitForCollectionCallback If set to `true`, it will return the entire collection to the callback as a single object. - * @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()`. + * @returns An unsubscribe function. */ -function connectWithoutView(connectOptions: ConnectOptions): Connection { - return connectionManager.connect(connectOptions); +function subscribe(key: TKey, listener: (value: OnyxValue, key: TKey) => void): () => void { + return onyxStore.subscribe(key, listener); } /** - * Disconnects and removes the listener from the Onyx key. + * Subscribe to per-member changes on a collection key. The listener fires once per + * changed member with `(memberValue, memberKey)`. * - * @example - * ```ts - * const connection = Onyx.connectWithoutView({ - * key: ONYXKEYS.SESSION, - * callback: onSessionChange, - * }); + * Use this when you care about which specific member changed (e.g. keeping a local + * map indexed by member ID up-to-date). For "always deliver the whole collection", + * use `subscribe()` instead. * - * Onyx.disconnect(connection); - * ``` + * @returns An unsubscribe function. + */ +function subscribeMembers(collectionKey: TKey, listener: (value: OnyxValue, memberKey: OnyxKey) => void): () => void { + return onyxStore.subscribeMembers(collectionKey, listener); +} + +/** + * @deprecated Prefer `Onyx.subscribe()` / `Onyx.subscribeMembers()`. Kept as a + * compatibility wrapper for existing call sites; routes to the new primitives + * based on the legacy `waitForCollectionCallback` option. * - * @param connection Connection object returned by calling `Onyx.connect()` or `Onyx.connectWithoutView()`. + * Returns synchronously with a `Connection` handle. The underlying subscription + * wires up after init completes; `disconnect()` works at any point. + */ +function connect(connectOptions: ConnectOptions): Connection { + const {key, callback, waitForCollectionCallback} = connectOptions; + + let active = true; + let unsubscribeFn: (() => void) | null = null; + + const wireUp = () => { + if (!active) { + return; + } + + if (OnyxKeys.isCollectionKey(key)) { + if (waitForCollectionCallback === true) { + // Snapshot mode — listener fires with the whole snapshot per collection change. + unsubscribeFn = onyxStore.subscribe(key, (value, k) => { + (callback as CollectionConnectCallback | undefined)?.(value as NonNullable>, k as TKey); + }); + // Initial fire deferred to a microtask so connect() returns first, + // matching the old ConnectionManager behavior. + Promise.resolve().then(() => { + if (!active || !callback) { + return; + } + const initial = onyxStore.getState(key); + (callback as CollectionConnectCallback)(initial as NonNullable>, key as TKey); + }); + return; + } + + // Per-member mode — one callback per changed member. + unsubscribeFn = onyxStore.subscribeMembers(key as CollectionKeyBase, (value, memberKey) => { + (callback as DefaultConnectCallback | undefined)?.(value, memberKey as TKey); + }); + Promise.resolve().then(() => { + if (!active || !callback) { + return; + } + const initial = onyxStore.getState(key) as OnyxCollection | undefined; + if (!initial) { + return; + } + for (const memberKey of Object.keys(initial)) { + (callback as DefaultConnectCallback)(initial[memberKey], memberKey as TKey); + } + }); + return; + } + + // Non-collection key — single-value subscription. + unsubscribeFn = onyxStore.subscribe(key, (value, k) => { + (callback as DefaultConnectCallback | undefined)?.(value, k as TKey); + }); + Promise.resolve().then(() => { + if (!active || !callback) { + return; + } + const initial = onyxStore.getState(key); + (callback as DefaultConnectCallback)(initial, key as TKey); + }); + }; + + OnyxUtils.afterInit(() => { + wireUp(); + return Promise.resolve(); + }); + + return { + unsubscribe: () => { + if (!active) { + return; + } + active = false; + if (unsubscribeFn) { + unsubscribeFn(); + unsubscribeFn = null; + } + }, + }; +} + +/** + * @deprecated Prefer `Onyx.subscribe()` / `Onyx.subscribeMembers()`. 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()` + * / `subscribe()` / `subscribeMembers()`. */ function disconnect(connection: Connection): void { - connectionManager.disconnect(connection); + if (!connection) { + return; + } + connection.unsubscribe(); } /** @@ -284,7 +364,7 @@ function merge(key: TKey, changes: OnyxMergeInput): * @param collection Object collection keyed by individual collection member keys and values */ function mergeCollection(collectionKey: TKey, collection: OnyxMergeCollectionInput): Promise { - return OnyxUtils.afterInit(() => OnyxUtils.mergeCollectionWithPatches({collectionKey, collection, isProcessingCollectionUpdate: true})); + return OnyxUtils.afterInit(() => OnyxUtils.mergeCollectionWithPatches({collectionKey, collection})); } /** @@ -384,17 +464,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); } }); }) @@ -525,7 +604,6 @@ function update(data: Array>): Promise, mergeReplaceNullPatches: batchedCollectionUpdates.mergeReplaceNullPatches, - isProcessingCollectionUpdate: true, }), ); } @@ -574,6 +652,9 @@ function setCollection(collectionKey: TKey, coll const Onyx = { METHOD: OnyxUtils.METHOD, + getState, + subscribe, + subscribeMembers, connect, connectWithoutView, disconnect, @@ -589,4 +670,4 @@ const Onyx = { }; export default Onyx; -export type {OnyxUpdate, ConnectOptions, SetOptions}; +export type {OnyxUpdate, ConnectOptions, SetOptions, Connection}; diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts deleted file mode 100644 index 5b0f32d01..000000000 --- a/lib/OnyxConnectionManager.ts +++ /dev/null @@ -1,271 +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, DefaultConnectOptions, 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; - - /** - * The value that triggered the last update - */ - sourceValue?: OnyxValue; - - /** - * Whether the subscriber is waiting for the collection callback to be fired. - */ - waitForCollectionCallback?: boolean; -}; - -/** - * 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, waitForCollectionCallback} = 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 in any of the following situations: - // - `reuseConnection` is `false`. That means the subscriber explicitly wants the connection to not be reused. - // - `key` is a collection key AND `waitForCollectionCallback` is `undefined/false`. This combination needs a new connection at every subscription - // in order to send all the collection entries, so the connection can't be reused. - if (reuseConnection === false || (OnyxKeys.isCollectionKey(key) && (waitForCollectionCallback === undefined || waitForCollectionCallback === false))) { - suffix += `,uniqueID=${Str.guid()}`; - } - - return `onyxKey=${key},waitForCollectionCallback=${waitForCollectionCallback ?? false}${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()) { - if (connection.waitForCollectionCallback) { - (callback as CollectionConnectCallback)(connection.cachedCallbackValue as Record, connection.cachedCallbackKey as OnyxKey, connection.sourceValue); - } else { - (callback as DefaultConnectCallback)(connection.cachedCallbackValue, connection.cachedCallbackKey as OnyxKey); - } - } - } - - /** - * 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, key, sourceValue) => { - 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; - createdConnection.sourceValue = sourceValue; - this.fireCallbacks(connectionID); - } - }; - - subscriptionID = OnyxUtils.subscribeToKey({ - ...connectOptions, - callback: callback as DefaultConnectCallback, - }); - - connectionMetadata = { - subscriptionID, - onyxKey: connectOptions.key, - isConnectionMade: false, - callbacks: new Map(), - waitForCollectionCallback: connectOptions.waitForCollectionCallback, - }; - - 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 as DefaultConnectOptions).callback?.(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 b1968a2ca..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(options: Pick, 'selector'>): string { - const selectorID = options?.selector ? this.getSelectorID(options.selector) : 'no_selector'; - const cacheKey = String(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/OnyxStore.ts b/lib/OnyxStore.ts new file mode 100644 index 000000000..cf5d5a2b0 --- /dev/null +++ b/lib/OnyxStore.ts @@ -0,0 +1,319 @@ +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; + +/** + * Listener fired once per changed member when any member of a collection changes. + * Replaces today's "collection key without waitForCollectionCallback" delivery mode. + */ +type MemberListener = (value: OnyxValue, memberKey: OnyxKey) => void; + +/** + * Listener fired when any of a state-listener's declared dep keys changes. + */ +type StateListenerCallback = () => void; + +type StateListenerEntry = { + listener: StateListenerCallback; + deps: Set; +}; + +/** + * `OnyxStore` is the listener registry that replaces `OnyxConnectionManager`, + * `OnyxSnapshotCache`, and the subscriber-half of `OnyxUtils`. It owns three + * indexes: + * + * keyListeners — listeners on an exact key (single key, collection root + * in snapshot mode, or a specific collection member). + * memberListeners — listeners on "any member of this collection" — fires + * once per changed member, not once per collection change. + * stateListeners(ByDep) — listeners that re-evaluate when any of their declared + * deps change. Indexed by dep key for O(1) lookup in notify(). + * + * Write paths call `notifyKey()` (single key write) or `notifyCollection()` + * (batch collection update from `mergeCollection`/`setCollection`/`clear`). + */ +class OnyxStore { + private keyListeners: Map>; + + private memberListeners: Map>; + + private stateListeners: Set; + + private stateListenersByDep: Map>; + + constructor() { + this.keyListeners = new Map(); + this.memberListeners = new Map(); + this.stateListeners = new Set(); + this.stateListenersByDep = 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); + } + }; + } + + /** + * Subscribe to per-member changes on a collection. The listener fires once per + * changed member, with `(memberValue, memberKey)`. Replaces today's "collection + * key without waitForCollectionCallback" delivery mode. + * + * Returns an unsubscribe function. + */ + subscribeMembers(collectionKey: TKey, listener: MemberListener): () => void { + let listeners = this.memberListeners.get(collectionKey); + if (!listeners) { + listeners = new Set(); + this.memberListeners.set(collectionKey, listeners); + } + listeners.add(listener); + return () => { + const set = this.memberListeners.get(collectionKey); + if (!set) { + return; + } + set.delete(listener); + if (set.size === 0) { + this.memberListeners.delete(collectionKey); + } + }; + } + + /** + * Subscribe to state-tree changes. The listener fires when any of the declared + * deps changes. Used by `useOnyxState`. + * + * Returns an unsubscribe function. + */ + subscribeState(listener: StateListenerCallback, deps: readonly OnyxKey[]): () => void { + const entry: StateListenerEntry = {listener, deps: new Set(deps)}; + this.stateListeners.add(entry); + for (const dep of entry.deps) { + let set = this.stateListenersByDep.get(dep); + if (!set) { + set = new Set(); + this.stateListenersByDep.set(dep, set); + } + set.add(entry); + } + return () => { + this.stateListeners.delete(entry); + for (const dep of entry.deps) { + const set = this.stateListenersByDep.get(dep); + if (!set) { + continue; + } + set.delete(entry); + if (set.size === 0) { + this.stateListenersByDep.delete(dep); + } + } + }; + } + + /** + * Notify of a single-key write. + * + * Dispatch: + * 1. keyListeners.get(key) — exact-key subscribers (always fires) + * 2. If key is a collection member: + * 2a. keyListeners.get(collectionKey) — snapshot subscribers + * 2b. memberListeners.get(collectionKey) — per-member subscribers + * 3. State listeners whose deps include `key` or its collection key. + */ + notifyKey(key: TKey, value: OnyxValue): 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 routing if this key is a collection member. + const collectionKey = OnyxKeys.getCollectionKey(key); + const isCollectionMemberWrite = collectionKey !== undefined && collectionKey !== key; + if (isCollectionMemberWrite) { + 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); + } + } + const members = this.memberListeners.get(collectionKey); + if (members && members.size > 0) { + for (const listener of members) { + this.safeInvoke(() => listener(value as OnyxValue, key), key); + } + } + } + + // 3. State listeners + const fired = new Set(); + this.fireStateListenersForDep(key, fired); + if (isCollectionMemberWrite) { + this.fireStateListenersForDep(collectionKey, fired); + } + } + + /** + * 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. memberListeners.get(collectionKey) — fires once per changed member. + * 3. keyListeners.get(memberKey) — fires per changed member where the value + * differs from the previous (for ref-equality on unchanged members). + * 4. State listeners affected by the collection key OR any changed member key, + * each fired at most once. + */ + notifyCollection( + collectionKey: TKey, + partialCollection: OnyxCollection, + partialPreviousCollection?: OnyxCollection, + ): void { + const changedKeys = Object.keys(partialCollection ?? {}); + if (changedKeys.length === 0) { + return; + } + const previous = partialPreviousCollection ?? {}; + + // 1. Snapshot subscribers fire once with the new snapshot. + 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); + } + } + + // 2. Per-member subscribers fire once per changed member. + const members = this.memberListeners.get(collectionKey); + if (members && members.size > 0) { + for (const memberKey of changedKeys) { + const value = partialCollection?.[memberKey]; + for (const listener of members) { + this.safeInvoke(() => listener(value as OnyxValue, memberKey), memberKey); + } + } + } + + // 3. Exact-member subscribers fire per changed key (skip if ref unchanged). + for (const memberKey of changedKeys) { + const value = partialCollection?.[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); + } + } + + // 4. State listeners — each affected entry fires at most once. + const fired = new Set(); + this.fireStateListenersForDep(collectionKey, fired); + for (const memberKey of changedKeys) { + this.fireStateListenersForDep(memberKey, fired); + } + } + + /** Wipe all subscriptions. Used by tests and `Onyx.clear()` follow-on. */ + clearAll(): void { + this.keyListeners.clear(); + this.memberListeners.clear(); + this.stateListeners.clear(); + this.stateListenersByDep.clear(); + } + + /** True if there are any subscribers for the given key (exact or member). */ + hasListenersForKey(key: OnyxKey): boolean { + if ((this.keyListeners.get(key)?.size ?? 0) > 0) { + return true; + } + const collectionKey = OnyxKeys.getCollectionKey(key); + if (collectionKey && collectionKey !== key) { + if ((this.keyListeners.get(collectionKey)?.size ?? 0) > 0) { + return true; + } + if ((this.memberListeners.get(collectionKey)?.size ?? 0) > 0) { + return true; + } + } + return false; + } + + private fireStateListenersForDep(depKey: OnyxKey, alreadyFired: Set): void { + const set = this.stateListenersByDep.get(depKey); + if (!set || set.size === 0) { + return; + } + for (const entry of set) { + if (alreadyFired.has(entry)) { + continue; + } + alreadyFired.add(entry); + this.safeInvoke(entry.listener, depKey); + } + } + + 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, MemberListener, StateListenerCallback}; diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 56dbb10fb..35341ded1 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'; @@ -6,16 +5,13 @@ import * as Logger from './Logger'; import type Onyx from './Onyx'; import cache, {TASK} from './OnyxCache'; import OnyxKeys from './OnyxKeys'; +import onyxStore from './OnyxStore'; import * as Str from './Str'; import Storage from './storage'; import type { CollectionKeyBase, - ConnectOptions, DeepRecord, - DefaultConnectCallback, - DefaultConnectOptions, KeyValueMapping, - CallbackToStateMapping, MultiMergeReplaceNullPatches, OnyxCollection, OnyxEntry, @@ -70,23 +66,11 @@ type OnyxMethod = ValueOf; 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(); @@ -412,35 +396,6 @@ function tupleGet(keys: Keys): Promise<{[Index return Promise.all(keys.map((key) => get(key))) as Promise<{[Index in keyof Keys]: OnyxValue}>; } -/** - * 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 @@ -539,231 +494,47 @@ 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}); - - if (subscriber.waitForCollectionCallback) { - subscriber.callback(cachedCollection, subscriber.key, partialCollection); - continue; - } - - // Not using waitForCollectionCallback — notify per changed key. - // Re-check the subscription on each iteration because the callback may - // synchronously disconnect itself (removing it from callbackToStateMapping), - // in which case we must stop firing further callbacks for this subscriber. - for (const dataKey of changedMemberKeys) { - const currentSubscriber = callbackToStateMapping[subID]; - if (!currentSubscriber || typeof currentSubscriber.callback !== 'function') { - break; - } - if (cachedCollection[dataKey] === previousCollection[dataKey]) { - continue; - } - const currentSubscriberCallback = currentSubscriber.callback as DefaultConnectCallback; - currentSubscriberCallback(cachedCollection[dataKey], dataKey as TKey); - } - } 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. */ -function keyChanged( - key: TKey, - value: OnyxValue, - canUpdateSubscriber: (subscriber?: CallbackToStateMapping) => boolean = () => true, - isProcessingCollectionUpdate = false, -): void { - // Add or remove this key from the recentlyAccessedKeys list +function notifyKey(key: TKey, value: OnyxValue): 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) && subscriber.waitForCollectionCallback) { - // Skip individual key changes for collection callbacks during collection updates - // to prevent duplicate callbacks - the collection update will handle this properly - if (isProcessingCollectionUpdate) { - continue; - } - // 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, {[key]: value}); - 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); } /** - * 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 subscribers with waitForCollectionCallback, read the full collection. - // For individual key subscribers, read just that key's value. - let value: OnyxValue | undefined; - if (OnyxKeys.isCollectionKey(mapping.key) && mapping.waitForCollectionCallback) { - 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 as DefaultConnectOptions).callback?.(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 */ -function remove(key: TKey, isProcessingCollectionUpdate?: boolean): Promise { +function remove(key: TKey): Promise { cache.drop(key); - keyChanged(key, undefined as OnyxValue, undefined, isProcessingCollectionUpdate); + notifyKey(key, undefined as OnyxValue); if (OnyxKeys.isRamOnlyKey(key)) { return Promise.resolve(); @@ -840,7 +611,7 @@ function broadcastUpdate(key: TKey, value: OnyxValue } cache.set(key, value); - keyChanged(key, value); + notifyKey(key, value); } function hasPendingMergeForKey(key: OnyxKey): boolean { @@ -858,13 +629,12 @@ function prepareKeyValuePairsForStorage( data: Record>, shouldRemoveNestedNulls?: boolean, replaceNullPatches?: MultiMergeReplaceNullPatches, - isProcessingCollectionUpdate?: boolean, ): StorageKeyValuePair[] { const pairs: StorageKeyValuePair[] = []; for (const [key, value] of Object.entries(data)) { if (value === null) { - remove(key, isProcessingCollectionUpdate); + remove(key); continue; } @@ -998,7 +768,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) => { @@ -1016,7 +786,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); } }); } @@ -1049,137 +819,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; - - // If the subscriber is attempting to connect to a collection member whose ID is skippable (e.g. "undefined", "null", etc.) - // we suppress wiring the subscription fully to avoid unnecessary callback emissions such as for "report_undefined". - // We still return a valid subscriptionID so callers can disconnect safely. - try { - const skippableIDs = getSkippableCollectionMemberIDs(); - if (skippableIDs.size) { - const [, collectionMemberID] = OnyxKeys.splitCollectionMemberKey(mapping.key); - if (skippableIDs.has(collectionMemberID)) { - // Clean up the provisional mapping to avoid retaining unused subscribers. - cache.addNullishStorageKey(mapping.key); - delete callbackToStateMapping[subscriptionID]; - return subscriptionID; - } - } - } catch (e) { - // Not a collection member key, proceed as usual. - } - - // When keyChanged is called, a key is passed and the method looks through all the Subscribers in callbackToStateMapping for the matching key to get the subscriptionID - // to avoid having to loop through all the Subscribers all the time (even when just one connection belongs to one key), - // We create a mapping from key to lists of subscriptionIDs to access the specific list of subscriptionIDs. - 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.waitForCollectionCallback ? 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 we will either trigger the provided callback for each key we find or combine all values - // into an object and just make a single call. The latter behavior is enabled by providing a waitForCollectionCallback key - // combined with a subscription to a collection key. - if (typeof mapping.callback === 'function') { - if (OnyxKeys.isCollectionKey(mapping.key)) { - if (mapping.waitForCollectionCallback) { - getCollectionDataAndSendAsObject(matchingKeys, mapping); - return; - } - - // We did not opt into using waitForCollectionCallback mode so the callback is called for every matching key. - multiGet(matchingKeys).then(() => { - for (const key of matchingKeys) { - sendDataToConnection(mapping, key as TKey); - } - }); - 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 []; @@ -1367,9 +1006,9 @@ function multiSetWithRetry(data: OnyxMultiSetInput, retryAttempt?: number): Prom const keyValuePairsToSet = OnyxUtils.prepareKeyValuePairsForStorage(newData, true); // 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>; previous: Record>}>(); for (const [key, value] of keyValuePairsToSet) { @@ -1381,7 +1020,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); @@ -1394,18 +1033,18 @@ 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); - keyChanged(key, value); + notifyKey(key, value); } } - // 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. for (const [collectionKey, batch] of collectionBatches) { - keysChanged(collectionKey as CollectionKeyBase, batch.partial, batch.previous); + notifyCollection(collectionKey as CollectionKeyBase, batch.partial, batch.previous); } const keyValuePairsToStore = keyValuePairsToSet.filter((keyValuePair) => { @@ -1474,12 +1113,12 @@ function setCollectionWithRetry({collectionKey, mutableCollection[key] = null; } - const keyValuePairs = OnyxUtils.prepareKeyValuePairsForStorage(mutableCollection, true, undefined, true); + const keyValuePairs = OnyxUtils.prepareKeyValuePairsForStorage(mutableCollection, true, undefined); const previousCollection = OnyxUtils.getCachedCollection(collectionKey); for (const [key, value] of keyValuePairs) cache.set(key, value); - keysChanged(collectionKey, mutableCollection, previousCollection); + notifyCollection(collectionKey, mutableCollection, previousCollection); // RAM-only keys are not supposed to be saved to storage if (OnyxKeys.isRamOnlyKey(collectionKey)) { @@ -1505,11 +1144,10 @@ function setCollectionWithRetry({collectionKey, * @param params.collection Object collection keyed by individual collection member keys and values * @param params.mergeReplaceNullPatches Record where the key is a collection member key and the value is a list of * tuples that we'll use to replace the nested objects of that collection member record with something else. - * @param params.isProcessingCollectionUpdate whether this is part of a collection update operation. * @param retryAttempt retry attempt */ function mergeCollectionWithPatches( - {collectionKey, collection, mergeReplaceNullPatches, isProcessingCollectionUpdate = false}: MergeCollectionWithPatchesParams, + {collectionKey, collection, mergeReplaceNullPatches}: MergeCollectionWithPatchesParams, retryAttempt?: number, ): Promise { if (!isValidNonEmptyCollectionForMerge(collection)) { @@ -1548,7 +1186,7 @@ function mergeCollectionWithPatches( // Split to keys that exist in storage and keys that don't const keys = resultCollectionKeys.filter((key) => { if (resultCollection[key] === null) { - remove(key, isProcessingCollectionUpdate); + remove(key); return false; } return true; @@ -1622,7 +1260,7 @@ function mergeCollectionWithPatches( // and update all subscribers const promiseUpdate = previousCollectionPromise.then((previousCollection) => { cache.merge(finalMergedCollection); - keysChanged(collectionKey, finalMergedCollection, previousCollection); + notifyCollection(collectionKey, finalMergedCollection, previousCollection); }); return Promise.all(promises) @@ -1630,7 +1268,7 @@ function mergeCollectionWithPatches( retryOperation( error, mergeCollectionWithPatches, - {collectionKey, collection: resultCollection as OnyxMergeCollectionInput, mergeReplaceNullPatches, isProcessingCollectionUpdate}, + {collectionKey, collection: resultCollection as OnyxMergeCollectionInput, mergeReplaceNullPatches}, retryAttempt, ), ) @@ -1684,11 +1322,11 @@ function partialSetCollection({collectionKey, co const mutableCollection: OnyxInputKeyValueMapping = {...resultCollection}; const existingKeys = resultCollectionKeys.filter((key) => persistedKeys.has(key)); const previousCollection = getCachedCollection(collectionKey, existingKeys); - const keyValuePairs = prepareKeyValuePairsForStorage(mutableCollection, true, undefined, true); + const keyValuePairs = prepareKeyValuePairsForStorage(mutableCollection, true, undefined); for (const [key, value] of keyValuePairs) cache.set(key, value); - keysChanged(collectionKey, mutableCollection, previousCollection); + notifyCollection(collectionKey, mutableCollection, previousCollection); if (OnyxKeys.isRamOnlyKey(collectionKey)) { sendActionToDevTools(METHOD.SET_COLLECTION, undefined, mutableCollection); @@ -1711,22 +1349,12 @@ function logKeyRemoved(onyxMethod: Extract, key: On Logger.logInfo(`${onyxMethod} called for key: ${key} => null passed, so key was removed`); } -/** - * Getter - returns the callback to state mapping, useful in test environments. - */ -function getCallbackToStateMapping(): Record> { - return callbackToStateMapping; -} - /** * Clear internal variables used in this file, useful in test environments. */ function clearOnyxUtilsInternals() { mergeQueue = {}; mergeQueuePromise = {}; - callbackToStateMapping = {}; - onyxKeyToSubscriptionIDs = new Map(); - lastConnectionCallbackData = new Map(); } const OnyxUtils = { @@ -1742,10 +1370,8 @@ const OnyxUtils = { getAllKeys, tryGetCachedValue, getCachedCollection, - keysChanged, - keyChanged, - sendDataToConnection, - getCollectionDataAndSendAsObject, + notifyKey, + notifyCollection, remove, reportStorageQuota, retryOperation, @@ -1760,14 +1386,10 @@ const OnyxUtils = { tupleGet, isValidNonEmptyCollectionForMerge, doAllCollectionItemsBelongToSameParent, - subscribeToKey, - unsubscribeFromKey, getSkippableCollectionMemberIDs, setSkippableCollectionMemberIDs, getSnapshotMergeKeys, setSnapshotMergeKeys, - storeKeyBySubscriptions, - deleteKeyBySubscriptions, reduceCollectionWithSelector, updateSnapshots, mergeCollectionWithPatches, @@ -1777,7 +1399,6 @@ const OnyxUtils = { setWithRetry, multiSetWithRetry, setCollectionWithRetry, - getCallbackToStateMapping, }; export type {OnyxMethod}; diff --git a/lib/index.ts b/lib/index.ts index bb6df0e0c..df5bae723 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,12 +19,13 @@ import type { OnyxSetCollectionInput, } from './types'; import type {FetchStatus, ResultMetadata, UseOnyxResult, UseOnyxOptions} from './useOnyx'; -import type {Connection} from './OnyxConnectionManager'; import useOnyx from './useOnyx'; +import useOnyxState from './useOnyxState'; +import type {OnyxStateView, UseOnyxStateOptions} from './useOnyxState'; import type {OnyxSQLiteKeyValuePair} from './storage/providers/SQLiteProvider'; export default Onyx; -export {useOnyx}; +export {useOnyx, useOnyxState}; export type { ConnectOptions, CustomTypeOptions, @@ -44,10 +45,12 @@ export type { OnyxSetCollectionInput, OnyxUpdate, OnyxValue, + OnyxStateView, ResultMetadata, Selector, UseOnyxResult, Connection, UseOnyxOptions, + UseOnyxStateOptions, OnyxSQLiteKeyValuePair, }; diff --git a/lib/types.ts b/lib/types.ts index 039130df9..ce507a740 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -365,7 +365,6 @@ type MergeCollectionWithPatchesParams = { collectionKey: TKey; collection: OnyxMergeCollectionInput; mergeReplaceNullPatches?: MultiMergeReplaceNullPatches; - isProcessingCollectionUpdate?: boolean; }; type RetriableOnyxOperation = diff --git a/lib/useLiveRef.ts b/lib/useLiveRef.ts deleted file mode 100644 index 869f439db..000000000 --- a/lib/useLiveRef.ts +++ /dev/null @@ -1,17 +0,0 @@ -import {useRef} from 'react'; - -/** - * Creates a mutable reference to a value, useful when you need to - * maintain a reference to a value that may change over time without triggering re-renders. - * - * @deprecated This hook breaks the Rules of React, and should not be used. - * The migration effort to remove it safely is not currently planned. - */ -function useLiveRef(value: T) { - const ref = useRef(value); - ref.current = value; - - return ref; -} - -export default useLiveRef; diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 7ac7233aa..96f50d5b0 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -1,307 +1,110 @@ -import {deepEqual, shallowEqual} from 'fast-equals'; -import {useCallback, useEffect, useMemo, useRef, useSyncExternalStore} from 'react'; -import type {DependencyList} from 'react'; -import OnyxCache, {TASK} from './OnyxCache'; -import type {Connection} from './OnyxConnectionManager'; -import connectionManager from './OnyxConnectionManager'; +import {deepEqual} from 'fast-equals'; +import {useCallback, useMemo, useRef, useSyncExternalStore} from 'react'; +import onyxStore from './OnyxStore'; import OnyxUtils from './OnyxUtils'; -import OnyxKeys from './OnyxKeys'; -import type {CollectionKeyBase, OnyxKey, OnyxValue} from './types'; -import onyxSnapshotCache from './OnyxSnapshotCache'; -import useLiveRef from './useLiveRef'; +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. + * If set to `false`, the underlying subscription is not pooled with other consumers + * of the same key. Largely a no-op in the store-based design (subscriptions are cheap) + * but kept for API compatibility. */ 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; }; type FetchStatus = 'loading' | 'loaded'; -type ResultMetadata = { +type ResultMetadata = { status: FetchStatus; - sourceValue?: NonNullable | undefined; }; -type UseOnyxResult = [NonNullable | undefined, ResultMetadata]; - -function useOnyx>( - key: TKey, - options?: UseOnyxOptions, - dependencies: DependencyList = [], -): UseOnyxResult { - const connectionRef = useRef(null); - const currentDependenciesRef = useLiveRef(dependencies); - const selector = options?.selector; - - // Create memoized version of selector for performance - const memoizedSelector = useMemo((): UseOnyxSelector | null => { - if (!selector) { - return null; - } - - let lastInput: OnyxValue | undefined; - let lastOutput: TReturnValue; - let lastDependencies: DependencyList = []; - let hasComputed = false; - - return (input: OnyxValue | undefined): TReturnValue => { - const currentDependencies = currentDependenciesRef.current; - - // Recompute if input changed, dependencies changed, or first time - const dependenciesChanged = !shallowEqual(lastDependencies, currentDependencies); - if (!hasComputed || lastInput !== input || dependenciesChanged) { - const newOutput = selector(input); - - // Always track the current input to avoid re-running the selector - // when the same input is seen again (even if the output didn't change). - lastInput = input; - - // Only update the output reference if it actually changed - if (!hasComputed || !deepEqual(lastOutput, newOutput) || dependenciesChanged) { - lastOutput = newOutput; - lastDependencies = [...currentDependencies]; - hasComputed = true; - } - } - +type UseOnyxResult = [NonNullable | undefined, ResultMetadata]; + +/** + * Wraps a user-provided selector so that: + * - Calling the wrapper with the same input reference twice short-circuits to the cached output. + * - Calling with a different input that produces a deep-equal output returns the *previous* + * output reference, so React reconciliation can detect equality with `===`. + * + * This is the minimum needed for `useSyncExternalStore` to not loop when consumers pass + * inline selectors that allocate fresh objects on every call. + */ +function createMemoizedSelector(selector: UseOnyxSelector): UseOnyxSelector { + let lastInput: OnyxValue | undefined; + let lastOutput: TReturnValue; + let hasComputed = false; + + return (input) => { + if (hasComputed && lastInput === input) { return lastOutput; - }; - }, [currentDependenciesRef, 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); - - // Indicates if the hook is connecting to an Onyx key. - const isConnectingRef = useRef(false); - - // Stores the `onStoreChange()` function, which can be used to trigger a `getSnapshot()` update when desired. - const onStoreChangeFnRef = useRef<(() => void) | null>(null); - - // Indicates if we should get the newest cached value from Onyx during `getSnapshot()` execution. - const shouldGetCachedValueRef = useRef(true); - - // Inside useOnyx.ts, we need to track the sourceValue separately - const sourceValueRef = useRef | undefined>(undefined); - - // Cache the options key to avoid regenerating it every getSnapshot call - const cacheKey = useMemo( - () => - onyxSnapshotCache.registerConsumer({ - selector: options?.selector, - }), - [options?.selector], - ); - - useEffect(() => () => onyxSnapshotCache.deregisterConsumer(key, cacheKey), [key, cacheKey]); - - // Track previous dependencies to prevent infinite loops - const previousDependenciesRef = useRef([]); - - useEffect(() => { - // This effect will only run if the `dependencies` array changes. If it changes it will force the hook - // to trigger a `getSnapshot()` update by calling the stored `onStoreChange()` function reference, thus - // re-running the hook and returning the latest value to the consumer. - - // Deep equality check to prevent infinite loops when dependencies array reference changes - // but content remains the same - if (shallowEqual(previousDependenciesRef.current, dependencies)) { - return; } - - previousDependenciesRef.current = dependencies; - - if (connectionRef.current === null || isConnectingRef.current || connectedKeyRef.current !== key || !onStoreChangeFnRef.current) { - return; - } - - // Invalidate cache when dependencies change so selector runs with new closure values - onyxSnapshotCache.invalidateForKey(key); - shouldGetCachedValueRef.current = true; - onStoreChangeFnRef.current(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [...dependencies]); - - // 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) { - resultRef.current = cachedResult; - return cachedResult; - } - } - - // 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 next = selector(input); + lastInput = input; + if (!hasComputed || !deepEqual(lastOutput, next)) { + lastOutput = next; + hasComputed = true; } + return lastOutput; + }; +} - const hasCacheForKey = OnyxCache.hasCacheForKey(key); +/** + * 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}]`. `status` is `'loaded'` once init completes and the + * key has either a value or a confirmed-absent state in cache; `'loading'` before + * that. + */ +function useOnyx>(key: TKey, options?: UseOnyxOptions): UseOnyxResult { + const selector = options?.selector; - // Since the fetch status can be different given the use cases below, we define the variable right away. - let newFetchStatus: FetchStatus | undefined; + // 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]); - // 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'; - } + const subscribe = useCallback( + (onStoreChange: () => void) => onyxStore.subscribe(key, onStoreChange), + [key], + ); - // 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. - // Normalize null to undefined to ensure consistent comparison (both represent "no value"). - const areValuesEqual = shallowEqual(previousValueRef.current ?? undefined, newValueRef.current ?? undefined); + // resultRef holds the last tuple returned to React. We return the same tuple + // reference when value and status haven't changed so React skips the re-render. + const resultRef = useRef>([undefined, {status: 'loading'}]); - // 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; + 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; - // 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, - sourceValue: sourceValueRef.current, - }, - ]; - } + // Loading until init completes. After init the value is whatever the cache says + // (absent keys are simply `undefined`); there's no "still fetching" phase because + // eager-load guarantees the cache is populated. + const initDone = OnyxUtils.getDeferredInitTask().isResolved; + const nextStatus: FetchStatus = initDone ? 'loaded' : 'loading'; - if (newFetchStatus !== 'loading') { - onyxSnapshotCache.setCachedResult>(key, cacheKey, resultRef.current); + const [prevValue, prevMeta] = resultRef.current; + if (prevValue === nextValue && prevMeta.status === nextStatus) { + return resultRef.current; } - + resultRef.current = [nextValue, {status: nextStatus}]; 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; - sourceValueRef.current = undefined; - resultRef.current = [undefined, {status: 'loading'}]; - } - // Force a cache re-read on every (re)subscription so any side effects from - // subscribeToKey (e.g. addNullishStorageKey for skippable collection member ids) - // are reflected in the next getSnapshot. Resetting this flag does not change - // resultRef by itself, so it doesn't cause an extra mount render. - shouldGetCachedValueRef.current = true; - hasMountedRef.current = true; - isConnectingRef.current = true; - onStoreChangeFnRef.current = onStoreChange; - - connectionRef.current = connectionManager.connect({ - key, - callback: (value, callbackKey, sourceValue) => { - 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; - - // sourceValue is unknown type, so we need to cast it to the correct type. - sourceValueRef.current = sourceValue as NonNullable; - - // 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(); - }, - waitForCollectionCallback: OnyxKeys.isCollectionKey(key) as true, - 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/lib/useOnyxState.ts b/lib/useOnyxState.ts new file mode 100644 index 0000000000000000000000000000000000000000..9c0cf761358e6cab5d60482a3b30bff20e9d8173 GIT binary patch literal 5993 zcmcIoU2iM55$&^o#q=d!8M1`F`m1Q13lz9*9bCJ~gJDF}E@f@JTB=K~ZCzgj`4Rnv z`6W3sL+*#{8#E8~gC*^f!{N-CGlwiUTh|17DXiUo_)lx9S6Vf0Lx(Hl!?EQj{_ymm zjKf-QFHKb~P5v+?{P?HcIQjC)uH^H2SLYv|gKcV4T?g0LWUsL%54|N<@1Es-Ty8eM zN>BPhVYjvP!Y@AB-IVyt52k86o3lBOVE+#0M@I*AM4zlFj$K{t=&@`9oX4kqBKK$; z3TrDS$OjYbjJ|K&);3{BCeN++v z7(ZFiufP14R<5DkRh7*{2~&9CWjez7+1BK1v-N8irfinS`m(;SY(FkH`Pvq`(3CJ> z(bk1sm9;GvRIaFYfwqnHwhlAcU^D-66dG%(^t6F9O=+s~vn^)y0k$xByuZe^!Lm>I za=+HCafYZU8#rC<*zEj3g09Jc%^x^?=k#(w_(Kj03GVGjDr*TQ5_5ObXXfnV#p%H- z@*!6K?QLi1+Hz!Eb9Qq9dzs`wCrTiBohk3{v2?AMM|*GS3v~gR(D?=3ytmBNmZR@= z74=%6!R{Yv`eSAgG#`DU!ftI{Fa&0F!zoWM8hhNdHNOC)p4P~tt1Y>eghTP+HHU|q zIZyf8)c4lU=yDGXxohjd5HczESXQ88>RmFpv705Zsz#9A^8Vg7JViZ0iex8q`Bb-U zotM_1^x5H|>6wpO?@A_eSm=ZDyH8@6qj(RX%b+A07! zjuz+V=L>o&!#X)^5Inn8rl3{i1_8KR{)~)AGl<8J96nwncblzgN{=durG|~R7L_4; zlQAe{%x$*?Fv$4!59M|XXvZVg&{2xBnA zY4i*@L7@T(x!Hneq#{v+fP!$0&`#tG6C@UCdJoI8essBEq-)J~Y0Ocg!FCmuCDy?% zAIriv2oT_}l?8}Mh!`$Ht7j277nA+c(?ZHoSb+fVqGC%DxxeeLvrjg6O>yRd*!q4- zwFTo|aHZ>1PAsCE{S-`tPp)*goeW;`0FYe9yx~^zn*Mx#PKRpoaDOb1SMc<#aH`+{#f|6gLfx8fg+uVk_#?>8GqzhHxLb2S={KBUAQ#*R&y zf>*WyAKUJka3$!w(^tRCk<2FFH|3@ba2Tu)LQyI>PQdM#K&KHNWj<}Rf$L(z(KEUd z`sIFa0^1$3JFHs2MoR-J4E%z(2e!*Yvg;Krcv(0b-shR}tmZmWvEe4*v&SL6?)791d5SB#Eyn^#u_T(O^Aw@aTlBRm{bgE(2&DJaRB*mbV~p zFztEwRL7__0S^3p*IJ5pv^p63j@2}{k2;^-Q5WD6=vNM@azoRY9Ty1iM|uEis|yz^ zaUE>(#^KNm-ytjgFam4_ol-E+gJkxzE9>m=PlwT4-5-zro2)3YVwW=*mLA^fDphV5#zAJdG~@oOq&zL{9hQsj`P1!vNprGZhGn9Zls=5@h?-1}|tt1V}tOraE?vT2}&c*Oz;Ueo~>IkRF zK3`Le?00m^Z~hlc74Frp42LFL!)e7 zItc5O7~Y;;uxet5Y$S{O+n2b>G%WeR7-ji2+Cds5vPvY`H{W#PceK`U-)YuNc8&HU z0U7B(UM)(mcNf&D%9T78TAx4o5Ae|lM-&xOLh-nb;sJ^Ea7Qy3mQuV1sqM4ew)MAB zT1sqQ0~>i8fRnQ`?q6bM{VvpS9*(CSw#1?dGKEjo?1SjEL>Pli%A(pPy5*Qf?f`Z+0gUYLrH^Wwvj+*Dvv(^Pw(^I~ONL7THzzvff?!cpB;hx=9^=W!3_nC4P WEkWjZOh!^#co8JKERnwCI literal 0 HcmV?d00001 From 959946a3fd7dcaedf7af6a6d958151f9a4677820 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 18 May 2026 14:18:59 +0100 Subject: [PATCH 2/8] =?UTF-8?q?fix(poc):=20test=20fixes=20=E2=80=94=20init?= =?UTF-8?q?ial-fire=20dedup,=20collection-snapshot=20suppression,=20lazy?= =?UTF-8?q?=20storage=20load,=20legacy=20callback=20shape?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Several behavior gaps surfaced by the existing test suite: - Connect wrapper now dedups initial-fire vs racing notifyKey (fixes "called once with X" → previously "called twice: undefined then X"). - Collection-snapshot subscribers no longer fire per per-key remove during collection-batch ops; threaded `suppressCollectionSnapshot` through `notifyKey`/`prepareKeyValuePairsForStorage` (fixes setCollection-with-nulls firing the snapshot listener N+1 times). - useOnyx now lazy-loads from storage when a key is missing from cache (covers `StorageMock.setItem` bypass test path); `loading → loaded` transition relies on `hasCacheForKey` plus the post-subscribe callback fire. - `notifyCollection` reads the merged value from `cache.getCollectionData(key)` for per-member and exact-member fires (fixes mergeCollection callback delivering only the merge-input fields, dropping pre-existing fields). - Skippable-member subscriptions short-circuit in the connect wrapper (no listener wired, no callback fired) — matches old `subscribeToKey` skippable branch. - Snapshot-mode connect callback restores the legacy 3-arg `(value, key, partial)` shape. `partial` is computed locally in the wrapper from prev/current snapshots (no `sourceValue` flowing through the store, so no PR #679 staleness). - Initial-fire-only "empty collection → undefined" coercion matches `sendDataToConnection`; subsequent fires still deliver `{}`. - Per-member subscriptions fire `(undefined, undefined)` on initial when no members exist (legacy "no matching keys" signal). - Single-key subscriptions fire `(undefined, undefined)` on initial when nothing is cached, `(value, key)` otherwise. Test housekeeping: - Deleted obsolete `OnyxConnectionManagerTest.ts` and `OnyxSnapshotCacheTest.ts` (both target deleted modules). - Deleted the `keysChanged` describe block in `onyxUtilsTest.ts` (directly tests `OnyxUtils.keysChanged`/`keyChanged` which are gone — their behavior is now covered by integration tests in `onyxTest.ts`). - Removed one `getCallbackToStateMapping` test (deleted API). - `useOnyxTest.ts` lost two stale `onyxSnapshotCache` import lines. Currently 402/412 pass; remaining 10 are edge cases (pending-merge loading state, clear-during-load, render count assertions on key switch, two `Onyx.update` batching cases). Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/Onyx.ts | 118 +++++- lib/OnyxStore.ts | 35 +- lib/OnyxUtils.ts | 36 +- lib/useOnyx.ts | 48 ++- tests/unit/OnyxConnectionManagerTest.ts | 481 ------------------------ tests/unit/OnyxSnapshotCacheTest.ts | 231 ------------ tests/unit/onyxUtilsTest.ts | 219 ----------- tests/unit/useOnyxTest.ts | 3 - 8 files changed, 187 insertions(+), 984 deletions(-) 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 218907f9a..5eb26b4b0 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -133,6 +133,23 @@ function subscribeMembers(collectionKey: TKey, l function connect(connectOptions: ConnectOptions): Connection { const {key, callback, waitForCollectionCallback} = connectOptions; + // Skippable members (e.g. "report_undefined" when "undefined" is configured as a + // skippable member ID) get no subscription wired up — we just mark the key as + // nullish in the cache so any reads return undefined cleanly. Returning a no-op + // unsubscribe handle keeps callers' disconnect() flows working. + try { + const skippableIDs = OnyxUtils.getSkippableCollectionMemberIDs(); + if (skippableIDs.size) { + const [, collectionMemberID] = OnyxKeys.splitCollectionMemberKey(key); + if (skippableIDs.has(collectionMemberID)) { + cache.addNullishStorageKey(key); + return {unsubscribe: () => undefined}; + } + } + } catch (e) { + // Not a collection member key — fall through and subscribe normally. + } + let active = true; let unsubscribeFn: (() => void) | null = null; @@ -144,50 +161,119 @@ function connect(connectOptions: ConnectOptions): Co if (OnyxKeys.isCollectionKey(key)) { if (waitForCollectionCallback === true) { // Snapshot mode — listener fires with the whole snapshot per collection change. + // + // Legacy contract preservation (the old ConnectionManager + sendDataToConnection + // had these quirks; some tests pin them): + // - Callback shape is `(snapshot, key, partial)` where `partial` is a record + // of members whose reference changed since the last delivery. The wrapper + // computes `partial` locally from snapshots, so it can't go stale across + // React renders (the original `sourceValue` staleness from PR #679). + // - Initial fire only: empty collection → `undefined` (legacy `sendDataToConnection`). + // Subsequent fires deliver the actual `{}` so consumers see "now empty". + // - Dedup: skip if the snapshot reference didn't change (e.g. a write raced + // with the initial-fire microtask). + const NOT_DELIVERED = Symbol('NOT_DELIVERED'); + let lastDeliveredSnapshot: unknown = NOT_DELIVERED; + const deliverSnapshot = (rawSnapshot: OnyxValue | undefined, k: TKey, isInitialFire: boolean) => { + if (Object.is(lastDeliveredSnapshot, rawSnapshot)) { + return; + } + // Compute per-member delta against the last delivered snapshot. Cheap thanks to + // structural sharing — unchanged members keep the same reference. + let partial: Record | undefined; + const current = rawSnapshot as Record | undefined; + const previous = (lastDeliveredSnapshot === NOT_DELIVERED ? undefined : lastDeliveredSnapshot) as Record | undefined; + if (current && previous) { + const changed: Record = {}; + for (const memberKey of Object.keys(current)) { + if (current[memberKey] !== previous[memberKey]) { + changed[memberKey] = current[memberKey]; + } + } + for (const memberKey of Object.keys(previous)) { + if (!(memberKey in current)) { + changed[memberKey] = undefined; + } + } + partial = Object.keys(changed).length > 0 ? changed : undefined; + } + lastDeliveredSnapshot = rawSnapshot; + const isEmpty = rawSnapshot !== undefined && rawSnapshot !== null && typeof rawSnapshot === 'object' && Object.keys(rawSnapshot as object).length === 0; + const valueToDeliver = isInitialFire && isEmpty ? undefined : rawSnapshot; + (callback as CollectionConnectCallback | undefined)?.( + valueToDeliver as NonNullable>, + k, + partial as OnyxValue | undefined, + ); + }; unsubscribeFn = onyxStore.subscribe(key, (value, k) => { - (callback as CollectionConnectCallback | undefined)?.(value as NonNullable>, k as TKey); + deliverSnapshot(value, k as TKey, false); }); - // Initial fire deferred to a microtask so connect() returns first, - // matching the old ConnectionManager behavior. Promise.resolve().then(() => { - if (!active || !callback) { + if (!active) { return; } - const initial = onyxStore.getState(key); - (callback as CollectionConnectCallback)(initial as NonNullable>, key as TKey); + deliverSnapshot(onyxStore.getState(key), key as TKey, true); }); return; } - // Per-member mode — one callback per changed member. - unsubscribeFn = onyxStore.subscribeMembers(key as CollectionKeyBase, (value, memberKey) => { + // Per-member mode — one callback per changed member. Dedup per member key. + const memberLastDelivered = new Map(); + const deliverMember = (value: OnyxValue, memberKey: OnyxKey) => { + if (memberLastDelivered.has(memberKey) && Object.is(memberLastDelivered.get(memberKey), value)) { + return; + } + memberLastDelivered.set(memberKey, value); (callback as DefaultConnectCallback | undefined)?.(value, memberKey as TKey); - }); + }; + unsubscribeFn = onyxStore.subscribeMembers(key as CollectionKeyBase, deliverMember); Promise.resolve().then(() => { if (!active || !callback) { return; } const initial = onyxStore.getState(key) as OnyxCollection | undefined; - if (!initial) { + const memberKeys = initial ? Object.keys(initial) : []; + if (memberKeys.length === 0) { + // Legacy semantic: when a per-member subscription finds no existing + // members, fire once with (undefined, undefined) so callers can clear + // any prior state and stop showing loading. Matches the old + // `sendDataToConnection(mapping, undefined)` path. + (callback as DefaultConnectCallback)(undefined as OnyxValue, undefined as unknown as TKey); return; } - for (const memberKey of Object.keys(initial)) { - (callback as DefaultConnectCallback)(initial[memberKey], memberKey as TKey); + for (const memberKey of memberKeys) { + deliverMember(initial?.[memberKey], memberKey); } }); return; } - // Non-collection key — single-value subscription. - unsubscribeFn = onyxStore.subscribe(key, (value, k) => { + // Non-collection key (or a specific collection member) — single-value subscription. + // Same dedup pattern. + const NOT_DELIVERED = Symbol('NOT_DELIVERED'); + 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); }); Promise.resolve().then(() => { - if (!active || !callback) { + if (!active) { return; } + // Legacy semantic: when there's no cached data for the key, fire with + // `(undefined, undefined)` as the "no match" signal — matches what the old + // `sendDataToConnection(mapping, undefined)` did. When data exists, fire + // with `(value, key)`. + const hasCache = cache.hasCacheForKey(key); const initial = onyxStore.getState(key); - (callback as DefaultConnectCallback)(initial, key as TKey); + deliverValue(initial, hasCache ? (key as TKey) : (undefined as unknown as TKey)); }); }; diff --git a/lib/OnyxStore.ts b/lib/OnyxStore.ts index cf5d5a2b0..c6fabe6e6 100644 --- a/lib/OnyxStore.ts +++ b/lib/OnyxStore.ts @@ -158,11 +158,15 @@ class OnyxStore { * Dispatch: * 1. keyListeners.get(key) — exact-key subscribers (always fires) * 2. If key is a collection member: - * 2a. keyListeners.get(collectionKey) — snapshot subscribers + * 2a. keyListeners.get(collectionKey) — snapshot subscribers (unless suppressed) * 2b. memberListeners.get(collectionKey) — per-member subscribers * 3. State listeners whose deps include `key` or its collection key. + * + * `options.suppressCollectionSnapshot` skips step 2a — used by collection-batch + * write paths so each removed/changed member doesn't re-trigger the collection-level + * snapshot listeners; the outer `notifyCollection()` fires those once. */ - notifyKey(key: TKey, value: OnyxValue): void { + notifyKey(key: TKey, value: OnyxValue, options?: {suppressCollectionSnapshot?: boolean}): void { // 1. Exact-key listeners const exact = this.keyListeners.get(key); if (exact && exact.size > 0) { @@ -175,11 +179,13 @@ class OnyxStore { const collectionKey = OnyxKeys.getCollectionKey(key); const isCollectionMemberWrite = collectionKey !== undefined && collectionKey !== key; if (isCollectionMemberWrite) { - 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 (!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); + } } } const members = this.memberListeners.get(collectionKey); @@ -221,29 +227,34 @@ class OnyxStore { } const previous = partialPreviousCollection ?? {}; + // Read the merged snapshot once; reuse for snapshot-mode AND for per-member reads. + // `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 from the previous values 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) { - const snapshot = cache.getCollectionData(collectionKey); for (const listener of snapshotListeners) { this.safeInvoke(() => listener(snapshot as OnyxValue, collectionKey), collectionKey); } } - // 2. Per-member subscribers fire once per changed member. + // 2. Per-member subscribers fire once per changed member with the merged value. const members = this.memberListeners.get(collectionKey); if (members && members.size > 0) { for (const memberKey of changedKeys) { - const value = partialCollection?.[memberKey]; + const value = snapshot?.[memberKey]; for (const listener of members) { this.safeInvoke(() => listener(value as OnyxValue, memberKey), memberKey); } } } - // 3. Exact-member subscribers fire per changed key (skip if ref unchanged). + // 3. Exact-member subscribers fire per changed key (skip if ref unchanged vs previous). for (const memberKey of changedKeys) { - const value = partialCollection?.[memberKey]; + const value = snapshot?.[memberKey]; const prev = previous[memberKey]; if (value === prev) { continue; diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 35341ded1..96125a5e0 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -497,14 +497,18 @@ function getCachedCollection(collectionKey: TKey * 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 notifyKey(key: TKey, value: OnyxValue): void { +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); } - onyxStore.notifyKey(key, value); + onyxStore.notifyKey(key, value, options); } /** @@ -530,11 +534,16 @@ function notifyCollection( } /** - * 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); - notifyKey(key, undefined as OnyxValue); + notifyKey(key, undefined as OnyxValue, options); if (OnyxKeys.isRamOnlyKey(key)) { return Promise.resolve(); @@ -629,12 +638,16 @@ function prepareKeyValuePairsForStorage( data: Record>, shouldRemoveNestedNulls?: boolean, replaceNullPatches?: MultiMergeReplaceNullPatches, + isProcessingCollectionUpdate?: boolean, ): StorageKeyValuePair[] { const pairs: StorageKeyValuePair[] = []; for (const [key, value] of Object.entries(data)) { if (value === null) { - remove(key); + // Within a collection batch, the outer notifyCollection() will fire snapshot + // listeners once with the final state — so each per-key remove should not + // re-fire the snapshot listener (which would cause N+1 callback invocations). + remove(key, {suppressCollectionSnapshot: !!isProcessingCollectionUpdate}); continue; } @@ -1113,7 +1126,7 @@ function setCollectionWithRetry({collectionKey, mutableCollection[key] = null; } - const keyValuePairs = OnyxUtils.prepareKeyValuePairsForStorage(mutableCollection, true, undefined); + const keyValuePairs = OnyxUtils.prepareKeyValuePairsForStorage(mutableCollection, true, undefined, true); const previousCollection = OnyxUtils.getCachedCollection(collectionKey); for (const [key, value] of keyValuePairs) cache.set(key, value); @@ -1186,7 +1199,8 @@ function mergeCollectionWithPatches( // Split to keys that exist in storage and keys that don't const keys = resultCollectionKeys.filter((key) => { if (resultCollection[key] === null) { - remove(key); + // Suppress collection-snapshot fire — outer notifyCollection() handles it. + remove(key, {suppressCollectionSnapshot: true}); return false; } return true; @@ -1229,11 +1243,11 @@ function mergeCollectionWithPatches( // When (multi-)merging the values with the existing values in storage, // we don't want to remove nested null values from the data that we pass to the storage layer, // because the storage layer uses them to remove nested keys from storage natively. - const keyValuePairsForExistingCollection = prepareKeyValuePairsForStorage(existingKeyCollection, false, mergeReplaceNullPatches); + const keyValuePairsForExistingCollection = prepareKeyValuePairsForStorage(existingKeyCollection, false, mergeReplaceNullPatches, true); // We can safely remove nested null values when using (multi-)set, // because we will simply overwrite the existing values in storage. - const keyValuePairsForNewCollection = prepareKeyValuePairsForStorage(newCollection, true); + const keyValuePairsForNewCollection = prepareKeyValuePairsForStorage(newCollection, true, undefined, true); const promises = []; @@ -1322,7 +1336,7 @@ function partialSetCollection({collectionKey, co const mutableCollection: OnyxInputKeyValueMapping = {...resultCollection}; const existingKeys = resultCollectionKeys.filter((key) => persistedKeys.has(key)); const previousCollection = getCachedCollection(collectionKey, existingKeys); - const keyValuePairs = prepareKeyValuePairsForStorage(mutableCollection, true, undefined); + const keyValuePairs = prepareKeyValuePairsForStorage(mutableCollection, true, undefined, true); for (const [key, value] of keyValuePairs) cache.set(key, value); diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 96f50d5b0..a76ce36b8 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -1,5 +1,7 @@ import {deepEqual} from 'fast-equals'; import {useCallback, useMemo, useRef, useSyncExternalStore} from 'react'; +import OnyxCache from './OnyxCache'; +import OnyxKeys from './OnyxKeys'; import onyxStore from './OnyxStore'; import OnyxUtils from './OnyxUtils'; import type {OnyxKey, OnyxValue} from './types'; @@ -60,13 +62,15 @@ function createMemoizedSelector(selector: Us } /** - * 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). + * Subscribes a React component to an Onyx key. * - * Returns `[value, {status}]`. `status` is `'loaded'` once init completes and the - * key has either a value or a confirmed-absent state in cache; `'loading'` before - * that. + * Returns `[value, {status}]`. Status is `'loaded'` when the cache holds an + * answer for the key (either a value or a confirmed-absent marker), and + * `'loading'` otherwise. For collection keys, `'loaded'` is always true post-init. + * + * If a key isn't in cache (rare with eager-load, but happens for storage writes + * that bypass Onyx), the hook lazy-loads via `OnyxUtils.get()` and transitions + * `loading → loaded` once the read settles. */ function useOnyx>(key: TKey, options?: UseOnyxOptions): UseOnyxResult { const selector = options?.selector; @@ -76,8 +80,28 @@ function useOnyx>(key: TKey // looping when consumers pass inline-allocating selectors. const memoizedSelector = useMemo(() => (selector ? createMemoizedSelector(selector) : null), [selector]); + // Flips to `true` after a callback (real change or lazy-load completion) fires for + // the current subscription. Combined with `hasCacheForKey` to decide 'loaded' vs 'loading'. + const hasCallbackFiredRef = useRef(false); + const subscribe = useCallback( - (onStoreChange: () => void) => onyxStore.subscribe(key, onStoreChange), + (onStoreChange: () => void) => { + hasCallbackFiredRef.current = false; + const unsubscribe = onyxStore.subscribe(key, () => { + hasCallbackFiredRef.current = true; + onStoreChange(); + }); + // Lazy-load from storage if cache doesn't have this key. Collection keys are + // treated as always-resolved post-init (an empty collection IS a valid answer). + // After the read settles, notify so the hook transitions to 'loaded'. + if (!OnyxKeys.isCollectionKey(key) && !OnyxCache.hasCacheForKey(key)) { + OnyxUtils.get(key).then(() => { + hasCallbackFiredRef.current = true; + onStoreChange(); + }); + } + return unsubscribe; + }, [key], ); @@ -90,11 +114,13 @@ function useOnyx>(key: TKey const selected = memoizedSelector ? memoizedSelector(raw as OnyxValue) : (raw as TReturnValue | undefined); const nextValue = (selected ?? undefined) as NonNullable | undefined; - // Loading until init completes. After init the value is whatever the cache says - // (absent keys are simply `undefined`); there's no "still fetching" phase because - // eager-load guarantees the cache is populated. const initDone = OnyxUtils.getDeferredInitTask().isResolved; - const nextStatus: FetchStatus = initDone ? 'loaded' : 'loading'; + const isCollectionKey = OnyxKeys.isCollectionKey(key); + const hasCacheForKey = isCollectionKey || OnyxCache.hasCacheForKey(key); + // 'loaded' when init is done AND (cache has answered for this key OR the + // subscriber callback has fired since (re)subscribing). The callback path + // covers lazy storage reads and writes that race with mount. + const nextStatus: FetchStatus = initDone && (hasCacheForKey || hasCallbackFiredRef.current) ? 'loaded' : 'loading'; const [prevValue, prevMeta] = resultRef.current; if (prevValue === nextValue && prevMeta.status === nextStatus) { diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts deleted file mode 100644 index 543dc23c1..000000000 --- a/tests/unit/OnyxConnectionManagerTest.ts +++ /dev/null @@ -1,481 +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},waitForCollectionCallback=false,sessionID=${getSessionID()}`); - }); - - it("should generate a stable connection ID regardless of the order which the option's properties were passed", async () => { - const connectionID = generateConnectionID({key: ONYXKEYS.TEST_KEY, waitForCollectionCallback: true}); - expect(connectionID).toEqual(`onyxKey=${ONYXKEYS.TEST_KEY},waitForCollectionCallback=true,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},waitForCollectionCallback=false,sessionID=${getSessionID()},uniqueID=`)).toBeTruthy(); - expect(connectionID2.startsWith(`onyxKey=${ONYXKEYS.TEST_KEY},waitForCollectionCallback=false,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 key but with different options, and fire the callbacks differently', 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, waitForCollectionCallback: true}); - - expect(connection1.id).not.toEqual(connection2.id); - expect(connectionsMap.size).toEqual(2); - expect(connectionsMap.has(connection1.id)).toBeTruthy(); - expect(connectionsMap.has(connection2.id)).toBeTruthy(); - - await act(async () => waitForPromisesToResolve()); - - expect(callback1).toHaveBeenCalledTimes(2); - expect(callback1).toHaveBeenNthCalledWith(1, obj1, `${ONYXKEYS.COLLECTION.TEST_KEY}entry1`); - expect(callback1).toHaveBeenNthCalledWith(2, obj2, `${ONYXKEYS.COLLECTION.TEST_KEY}entry2`); - - expect(callback2).toHaveBeenCalledTimes(1); - expect(callback2).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY, undefined); - - 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 create a separate connection to the same key when it's a collection one and waitForCollectionCallback is undefined/false", 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, waitForCollectionCallback: undefined, callback: callback1}); - - await act(async () => waitForPromisesToResolve()); - - expect(callback1).toHaveBeenCalledTimes(3); - expect(callback1).toHaveBeenNthCalledWith(1, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry1`); - expect(callback1).toHaveBeenNthCalledWith(2, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry2`); - expect(callback1).toHaveBeenNthCalledWith(3, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry3`); - - const callback2 = jest.fn(); - const connection2 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, waitForCollectionCallback: 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(); - - await act(async () => waitForPromisesToResolve()); - - expect(callback2).toHaveBeenCalledTimes(3); - expect(callback2).toHaveBeenNthCalledWith(1, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry1`); - expect(callback2).toHaveBeenNthCalledWith(2, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry2`); - expect(callback2).toHaveBeenNthCalledWith(3, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry3`); - }); - - 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('sourceValue parameter', () => { - it('should pass the sourceValue parameter to collection callbacks when waitForCollectionCallback is true', 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, - waitForCollectionCallback: true, - }); - - await act(async () => waitForPromisesToResolve()); - - // Initial callback with undefined values - expect(callback).toHaveBeenCalledTimes(1); - expect(callback).toHaveBeenCalledWith(undefined, ONYXKEYS.COLLECTION.TEST_KEY, undefined); - - // 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, {[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: obj1}); - - // 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, - {[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: obj2}, - ); - - connectionManager.disconnect(connection); - }); - - it('should not pass sourceValue to regular callbacks when waitForCollectionCallback is false', async () => { - const obj1 = {id: 'entry1_id', name: 'entry1_name'}; - - const callback = jest.fn(); - const connection = connectionManager.connect({ - key: ONYXKEYS.COLLECTION.TEST_KEY, - callback, - waitForCollectionCallback: false, - }); - - await act(async () => waitForPromisesToResolve()); - - // Update with object - await Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`, obj1); - - expect(callback).toHaveBeenCalledWith(obj1, `${ONYXKEYS.COLLECTION.TEST_KEY}entry1`); - - connectionManager.disconnect(connection); - }); - }); -}); diff --git a/tests/unit/OnyxSnapshotCacheTest.ts b/tests/unit/OnyxSnapshotCacheTest.ts deleted file mode 100644 index 35b46a5cb..000000000 --- a/tests/unit/OnyxSnapshotCacheTest.ts +++ /dev/null @@ -1,231 +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(optionsWithSelector); - const keyWithoutSelector = cache.registerConsumer(optionsWithoutSelector); - const keyWithUndefined = cache.registerConsumer({}); - - // Selector cache keys are the selector ID as a string; no-selector consumers share the same key - expect(keyWithSelector).toBe('0'); - expect(keyWithoutSelector).toBe('no_selector'); - expect(keyWithUndefined).toBe('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/onyxUtilsTest.ts b/tests/unit/onyxUtilsTest.ts index cc1568365..2ed3667b9 100644 --- a/tests/unit/onyxUtilsTest.ts +++ b/tests/unit/onyxUtilsTest.ts @@ -153,16 +153,6 @@ describe('OnyxUtils', () => { expect(Object.keys(received ?? {})).not.toContain(undefinedKey); }); - it('does not register an active subscription in callbackToStateMapping for a skippable member', async () => { - const skippableKey = `${BASE}undefined`; - Onyx.connect({key: skippableKey, callback: jest.fn()}); - - await act(async () => waitForPromisesToResolve()); - - const mappings = OnyxUtils.getCallbackToStateMapping(); - const hasActiveSubscription = Object.values(mappings).some((m) => m.key === skippableKey); - expect(hasActiveSubscription).toBe(false); - }); }); describe('partialSetCollection', () => { @@ -500,215 +490,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 waitForCollectionCallback', 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, - waitForCollectionCallback: true, - }); - - 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, subscriber.key, and partial - const [receivedCollection, receivedKey, receivedPartial] = collectionCallback.mock.calls[0]; - expect(receivedKey).toBe(ONYXKEYS.COLLECTION.TEST_KEY); - expect(receivedCollection[entryKey]).toEqual(entryData); - expect(receivedPartial).toEqual({[entryKey]: 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); diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index c5a5e8830..5fe582f21 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', () => { From 5949c440125ad9c28e0b52a765032bf62c3190c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 18 May 2026 16:12:22 +0100 Subject: [PATCH 3/8] fix(poc): drop loading/loaded transitions, sourceValue partial arg, pending-merge gate, lazy storage fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per design decisions: - useOnyx always returns `[value, {status: 'loaded'}]` — `status` retained for destructure compat; no loading transitions. - `CollectionConnectCallback` narrowed to `(value, key)` — `sourceValue`/`partial` 3rd arg removed in line with PR #679's anti-pattern reasoning. - `useOnyx` no longer lazy-loads from storage (eager-load handles the common case). - `useOnyx` no longer surfaces pending-merge loading state. - Direct writes to a collection root (`Onyx.merge(COLLECTION_KEY, ...)`) treated as opaque single-key writes; per-member subscribers no longer notified of root writes. The single test relying on this pattern is deleted. Test housekeeping: - onyxTest.ts: deleted the merge-on-collection-root test, updated 8 `toHaveBeenCalledWith`/`toHaveBeenNthCalledWith` assertions to drop the trailing 3rd-arg (sourceValue/partial). - onyxClearWebStorageTest.ts: updated 3 assertions for the same reason. - useOnyxTest.ts: deleted ~15 tests that exercised the removed loading transitions / lazy storage fallback / pending-merge gate. Remaining failures (2): legacy timing-dependent `Onyx.update` tests that fired only once with the old deep-promise-chain subscribe but fire twice with the simpler new wrapper. To be updated next. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/Onyx.ts | 45 +-- lib/OnyxStore.ts | 4 +- lib/types.ts | 2 +- lib/useOnyx.ts | 72 ++--- tests/unit/onyxClearWebStorageTest.ts | 15 +- tests/unit/onyxTest.ts | 37 +-- tests/unit/useOnyxTest.ts | 389 -------------------------- 7 files changed, 46 insertions(+), 518 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 5eb26b4b0..610e363b6 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -162,49 +162,23 @@ function connect(connectOptions: ConnectOptions): Co if (waitForCollectionCallback === true) { // Snapshot mode — listener fires with the whole snapshot per collection change. // - // Legacy contract preservation (the old ConnectionManager + sendDataToConnection - // had these quirks; some tests pin them): - // - Callback shape is `(snapshot, key, partial)` where `partial` is a record - // of members whose reference changed since the last delivery. The wrapper - // computes `partial` locally from snapshots, so it can't go stale across - // React renders (the original `sourceValue` staleness from PR #679). - // - Initial fire only: empty collection → `undefined` (legacy `sendDataToConnection`). - // Subsequent fires deliver the actual `{}` so consumers see "now empty". - // - Dedup: skip if the snapshot reference didn't change (e.g. a write raced - // with the initial-fire microtask). + // The `sourceValue` 3rd-arg has been dropped (anti-pattern: leaked stale state + // through useOnyx, see PR #679). Callback shape is now `(snapshot, key)`. + // + // Legacy preserved on initial fire only: empty collection → `undefined` + // (matches old `sendDataToConnection`). Subsequent fires deliver the actual `{}`. + // + // Dedup: skip if the snapshot reference didn't change. const NOT_DELIVERED = Symbol('NOT_DELIVERED'); let lastDeliveredSnapshot: unknown = NOT_DELIVERED; const deliverSnapshot = (rawSnapshot: OnyxValue | undefined, k: TKey, isInitialFire: boolean) => { if (Object.is(lastDeliveredSnapshot, rawSnapshot)) { return; } - // Compute per-member delta against the last delivered snapshot. Cheap thanks to - // structural sharing — unchanged members keep the same reference. - let partial: Record | undefined; - const current = rawSnapshot as Record | undefined; - const previous = (lastDeliveredSnapshot === NOT_DELIVERED ? undefined : lastDeliveredSnapshot) as Record | undefined; - if (current && previous) { - const changed: Record = {}; - for (const memberKey of Object.keys(current)) { - if (current[memberKey] !== previous[memberKey]) { - changed[memberKey] = current[memberKey]; - } - } - for (const memberKey of Object.keys(previous)) { - if (!(memberKey in current)) { - changed[memberKey] = undefined; - } - } - partial = Object.keys(changed).length > 0 ? changed : undefined; - } lastDeliveredSnapshot = rawSnapshot; const isEmpty = rawSnapshot !== undefined && rawSnapshot !== null && typeof rawSnapshot === 'object' && Object.keys(rawSnapshot as object).length === 0; const valueToDeliver = isInitialFire && isEmpty ? undefined : rawSnapshot; - (callback as CollectionConnectCallback | undefined)?.( - valueToDeliver as NonNullable>, - k, - partial as OnyxValue | undefined, - ); + (callback as CollectionConnectCallback | undefined)?.(valueToDeliver as NonNullable>, k); }; unsubscribeFn = onyxStore.subscribe(key, (value, k) => { deliverSnapshot(value, k as TKey, false); @@ -237,8 +211,7 @@ function connect(connectOptions: ConnectOptions): Co if (memberKeys.length === 0) { // Legacy semantic: when a per-member subscription finds no existing // members, fire once with (undefined, undefined) so callers can clear - // any prior state and stop showing loading. Matches the old - // `sendDataToConnection(mapping, undefined)` path. + // any prior state. Matches the old `sendDataToConnection(mapping, undefined)`. (callback as DefaultConnectCallback)(undefined as OnyxValue, undefined as unknown as TKey); return; } diff --git a/lib/OnyxStore.ts b/lib/OnyxStore.ts index c6fabe6e6..e7adbb41c 100644 --- a/lib/OnyxStore.ts +++ b/lib/OnyxStore.ts @@ -175,7 +175,9 @@ class OnyxStore { } } - // 2. Collection-level routing if this key is a collection member. + // 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) { diff --git a/lib/types.ts b/lib/types.ts index ce507a740..0be5dba2e 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -223,7 +223,7 @@ type BaseConnectOptions = { type DefaultConnectCallback = (value: OnyxEntry, key: TKey) => void; /** Represents the callback function used in `Onyx.connect()` method with a collection key. */ -type CollectionConnectCallback = (value: NonUndefined>, key: TKey, sourceValue?: OnyxValue) => void; +type CollectionConnectCallback = (value: NonUndefined>, key: TKey) => void; /** Represents the options used in `Onyx.connect()` method with a regular key. */ // NOTE: Any changes to this type like adding or removing options must be accounted in OnyxConnectionManager's `generateConnectionID()` method! diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index a76ce36b8..ac6cfd297 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -1,9 +1,6 @@ import {deepEqual} from 'fast-equals'; import {useCallback, useMemo, useRef, useSyncExternalStore} from 'react'; -import OnyxCache from './OnyxCache'; -import OnyxKeys from './OnyxKeys'; import onyxStore from './OnyxStore'; -import OnyxUtils from './OnyxUtils'; import type {OnyxKey, OnyxValue} from './types'; type UseOnyxSelector> = (data: OnyxValue | undefined) => TReturnValue; @@ -25,7 +22,12 @@ type UseOnyxOptions = { selector?: UseOnyxSelector; }; -type FetchStatus = 'loading' | 'loaded'; +/** + * 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 = 'loaded'; type ResultMetadata = { status: FetchStatus; @@ -61,16 +63,16 @@ function createMemoizedSelector(selector: Us }; } +const LOADED_METADATA: ResultMetadata = {status: 'loaded'}; + /** - * Subscribes a React component to an Onyx key. + * 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}]`. Status is `'loaded'` when the cache holds an - * answer for the key (either a value or a confirmed-absent marker), and - * `'loading'` otherwise. For collection keys, `'loaded'` is always true post-init. - * - * If a key isn't in cache (rare with eager-load, but happens for storage writes - * that bypass Onyx), the hook lazy-loads via `OnyxUtils.get()` and transitions - * `loading → loaded` once the read settles. + * 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 selector = options?.selector; @@ -80,53 +82,21 @@ function useOnyx>(key: TKey // looping when consumers pass inline-allocating selectors. const memoizedSelector = useMemo(() => (selector ? createMemoizedSelector(selector) : null), [selector]); - // Flips to `true` after a callback (real change or lazy-load completion) fires for - // the current subscription. Combined with `hasCacheForKey` to decide 'loaded' vs 'loading'. - const hasCallbackFiredRef = useRef(false); - - const subscribe = useCallback( - (onStoreChange: () => void) => { - hasCallbackFiredRef.current = false; - const unsubscribe = onyxStore.subscribe(key, () => { - hasCallbackFiredRef.current = true; - onStoreChange(); - }); - // Lazy-load from storage if cache doesn't have this key. Collection keys are - // treated as always-resolved post-init (an empty collection IS a valid answer). - // After the read settles, notify so the hook transitions to 'loaded'. - if (!OnyxKeys.isCollectionKey(key) && !OnyxCache.hasCacheForKey(key)) { - OnyxUtils.get(key).then(() => { - hasCallbackFiredRef.current = true; - onStoreChange(); - }); - } - return unsubscribe; - }, - [key], - ); - - // resultRef holds the last tuple returned to React. We return the same tuple - // reference when value and status haven't changed so React skips the re-render. - const resultRef = useRef>([undefined, {status: 'loading'}]); + const subscribe = useCallback((onStoreChange: () => void) => onyxStore.subscribe(key, onStoreChange), [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]); 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; - const initDone = OnyxUtils.getDeferredInitTask().isResolved; - const isCollectionKey = OnyxKeys.isCollectionKey(key); - const hasCacheForKey = isCollectionKey || OnyxCache.hasCacheForKey(key); - // 'loaded' when init is done AND (cache has answered for this key OR the - // subscriber callback has fired since (re)subscribing). The callback path - // covers lazy storage reads and writes that race with mount. - const nextStatus: FetchStatus = initDone && (hasCacheForKey || hasCallbackFiredRef.current) ? 'loaded' : 'loading'; - - const [prevValue, prevMeta] = resultRef.current; - if (prevValue === nextValue && prevMeta.status === nextStatus) { + if (resultRef.current[0] === nextValue) { return resultRef.current; } - resultRef.current = [nextValue, {status: nextStatus}]; + resultRef.current = [nextValue, LOADED_METADATA]; return resultRef.current; }, [key, memoizedSelector]); diff --git a/tests/unit/onyxClearWebStorageTest.ts b/tests/unit/onyxClearWebStorageTest.ts index a9629ab3c..6cb4868e2 100644 --- a/tests/unit/onyxClearWebStorageTest.ts +++ b/tests/unit/onyxClearWebStorageTest.ts @@ -240,7 +240,7 @@ describe('Set data while storage is clearing', () => { expect(collectionCallback).toHaveBeenCalledTimes(3); // And it should be called with the expected parameters each time - expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST, undefined); + expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST); expect(collectionCallback).toHaveBeenNthCalledWith( 2, { @@ -250,19 +250,8 @@ describe('Set data while storage is clearing', () => { test_4: 4, }, ONYX_KEYS.COLLECTION.TEST, - { - test_1: 1, - test_2: 2, - test_3: 3, - test_4: 4, - }, ); - expect(collectionCallback).toHaveBeenLastCalledWith({}, ONYX_KEYS.COLLECTION.TEST, { - test_1: undefined, - test_2: undefined, - test_3: undefined, - test_4: undefined, - }); + expect(collectionCallback).toHaveBeenLastCalledWith({}, ONYX_KEYS.COLLECTION.TEST); }) ); }); diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 5ff5d6f96..7b689e605 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -643,21 +643,6 @@ describe('Onyx', () => { }); }); - it('should overwrite an array key nested inside an object when using merge on a collection', () => { - let testKeyValue: unknown; - connection = Onyx.connect({ - key: ONYX_KEYS.COLLECTION.TEST_KEY, - callback: (value) => { - testKeyValue = value; - }, - }); - - Onyx.merge(ONYX_KEYS.COLLECTION.TEST_KEY, {test_1: {something: [1, 2, 3]}}); - return Onyx.merge(ONYX_KEYS.COLLECTION.TEST_KEY, {test_1: {something: [4]}}).then(() => { - expect(testKeyValue).toEqual({test_1: {something: [4]}}); - }); - }); - it('should properly set and merge when using mergeCollection', async () => { const valuesReceived: Record = {}; const mockCallback = jest.fn(); @@ -1065,7 +1050,7 @@ describe('Onyx', () => { .then(() => { // Then we expect the callback to be called only once and the initial stored value to be initialCollectionData expect(mockCallback).toHaveBeenCalledTimes(1); - expect(mockCallback).toHaveBeenCalledWith(initialCollectionData, ONYX_KEYS.COLLECTION.TEST_CONNECT_COLLECTION, undefined); + expect(mockCallback).toHaveBeenCalledWith(initialCollectionData, ONYX_KEYS.COLLECTION.TEST_CONNECT_COLLECTION); }); }); @@ -1091,10 +1076,10 @@ describe('Onyx', () => { 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, undefined); + expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, 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, collectionUpdate); + expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY); }) ); }); @@ -1149,10 +1134,8 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenCalledTimes(2); // AND the value for the second call should be collectionUpdate - expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST_POLICY, undefined); - expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY, { - [`${ONYX_KEYS.COLLECTION.TEST_POLICY}1`]: collectionUpdate.testPolicy_1, - }); + expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST_POLICY); + expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY); }) ); }); @@ -1187,7 +1170,7 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenCalledTimes(2); // And the value for the second call should be collectionUpdate - expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY, {testPolicy_1: collectionUpdate.testPolicy_1}); + expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY); }) // When merge is called again with the same collection not modified @@ -1224,8 +1207,8 @@ 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, undefined); - expect(collectionCallback).toHaveBeenNthCalledWith(2, {[itemKey]: {a: 'a'}}, ONYX_KEYS.COLLECTION.TEST_UPDATE, {[itemKey]: {a: 'a'}}); + expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, 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); @@ -1483,8 +1466,8 @@ describe('Onyx', () => { }) .then(() => { expect(collectionCallback).toHaveBeenCalledTimes(2); - expect(collectionCallback).toHaveBeenNthCalledWith(1, {[cat]: initialValue}, ONYX_KEYS.COLLECTION.ANIMALS, {[cat]: initialValue}); - expect(collectionCallback).toHaveBeenNthCalledWith(2, collectionDiff, ONYX_KEYS.COLLECTION.ANIMALS, {[cat]: initialValue, [dog]: {name: 'Rex'}}); + expect(collectionCallback).toHaveBeenNthCalledWith(1, {[cat]: initialValue}, ONYX_KEYS.COLLECTION.ANIMALS); + expect(collectionCallback).toHaveBeenNthCalledWith(2, collectionDiff, ONYX_KEYS.COLLECTION.ANIMALS); // Cat hasn't changed from its original value, expect only the initial connect callback expect(catCallback).toHaveBeenCalledTimes(1); diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 5fe582f21..d410ecda3 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -50,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'); @@ -94,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'); @@ -236,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'); @@ -269,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'); @@ -335,75 +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'}); @@ -882,89 +716,6 @@ describe('useOnyx', () => { }); }); - 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'); - - Onyx.merge(ONYXKEYS.TEST_KEY, 'test2'); - Onyx.merge(ONYXKEYS.TEST_KEY, 'test3'); - Onyx.merge(ONYXKEYS.TEST_KEY, 'test4'); - - 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('test4'); - expect(result.current[1].status).toEqual('loaded'); - }); - - 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'); - - Onyx.merge(ONYXKEYS.TEST_KEY, 'test2'); - Onyx.merge(ONYXKEYS.TEST_KEY, 'test3'); - Onyx.merge(ONYXKEYS.TEST_KEY, 'test4'); - - const {result} = renderHook(() => - useOnyx(ONYXKEYS.TEST_KEY, { - selector: ((entry: OnyxEntry) => `${entry}_changed`) as UseOnyxSelector, - }), - ); - - 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'); - }); - }); - - 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'); - - const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); - - expect(result1.current[0]).toBeUndefined(); - expect(result1.current[1].status).toEqual('loading'); - - await act(async () => waitForPromisesToResolve()); - - expect(result1.current[0]).toEqual('test'); - expect(result1.current[1].status).toEqual('loaded'); - - const {result: result2} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); - - expect(result2.current[0]).toEqual('test'); - expect(result2.current[1].status).toEqual('loaded'); - }); - - 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'); - - const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); - const {result: result2} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); - - expect(result1.current[0]).toBeUndefined(); - expect(result1.current[1].status).toEqual('loading'); - - expect(result2.current[0]).toBeUndefined(); - expect(result2.current[1].status).toEqual('loading'); - - await act(async () => waitForPromisesToResolve()); - - expect(result1.current[0]).toEqual('test'); - expect(result1.current[1].status).toEqual('loaded'); - - expect(result2.current[0]).toEqual('test'); - expect(result2.current[1].status).toEqual('loaded'); - }); - }); describe('dependencies', () => { it('should return the updated selected value when a external value passed to the dependencies list changes', async () => { @@ -1087,29 +838,6 @@ describe('useOnyx', () => { expect(result.current[1].status).toEqual('loaded'); }); - it('should transition through loading and return value when switching from a skippable key to a valid one', async () => { - // Seed a value for the skippable key — must stay invisible to the hook - await StorageMock.setItem(`${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id`, {id: 'skippable'}); - // Seed the target valid key in storage only (not in cache) so the switch goes through loading - await StorageMock.setItem(`${ONYXKEYS.COLLECTION.TEST_KEY}1`, {id: '1'}); - - const {result, rerender} = renderHook((key: string) => useOnyx(key), {initialProps: `${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id` as string}); - - await act(async () => waitForPromisesToResolve()); - - expect(result.current[0]).toBeUndefined(); - expect(result.current[1].status).toEqual('loaded'); - - // Switch to a valid key whose value is in storage but not in cache — should transition through loading - rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}1`); - - expect(result.current[1].status).toEqual('loading'); - - await act(async () => waitForPromisesToResolve()); - - expect(result.current[0]).toEqual({id: '1'}); - expect(result.current[1].status).toEqual('loaded'); - }); }); describe('RAM-only keys', () => { @@ -1245,122 +973,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 b96fd8c250236e08c3150f11721e6e3da2ecbe35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 22 May 2026 08:10:30 +0100 Subject: [PATCH 4/8] fix(poc): correct empty-collection-post-clear + initial-fire timing in connect wrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Onyx-side fixes that resolve 15 expensify-app CI failures across six categories without behavioral regressions. **1. `OnyxCache.getCollectionData` — empty collection after `Onyx.clear()`** The `storageKeys.size > 0` heuristic was used as a proxy for "init done" to decide whether an empty collection returns the frozen `{}` snapshot or `undefined`. That works at startup but flips back to "not done" after `Onyx.clear()` wipes the storage-keys index, so a known-empty collection post-clear was returning `undefined`. That caused `Onyx.connect({waitForCollectionCallback: true})` callbacks that bail on a nullish value (`if (!actions) return; allReportActions = actions;` in ReportActionsUtils, similar patterns elsewhere) to skip the post-clear notification, leaving module-level caches pointing at pre-clear data. Four DebugUtils GBR tests and the OptionListContextProvider migration test all stemmed from this. Switched to `collectionSnapshots.has(collectionKey)`, which `setCollectionKeys()` seeds during `Onyx.init` and which survives `Onyx.clear()`. Updated the one `onyxCacheTest` assertion that pinned the old "post-init, no keys loaded → undefined" behavior to expect `{}` (the cleaner contract). **2. `Onyx.connect` wrapper — initial-fire timing** The legacy `subscribeToKey` chain (`deferredInitTask.then().then(getAllKeys).then(multiGet).then(sendDataToConnection)`) accidentally deferred initial-fire by ~3 microtask hops past in-flight writes. Test patterns like `tests/utils/TestHelper.getOnyxData` and the ~534 inline `Onyx.disconnect(connection)`-after-first-callback patterns across expensify-app's test suite rely on that timing. The new store-based wrapper had a single `Promise.resolve().then(...)` hop, so callers that `Onyx.connect()` immediately after a fire-and-forget `Onyx.update()` were seeing pre-write state. Added a `scheduleInitialFire` helper that chains three `.then()` hops — known-ugly, but documented as such and chosen over `setTimeout(0)` because Jest test bodies run entirely in microtask land via chained `.then()`s, so a macrotask-deferred initial fire would land after the test's assertions. Same depth applied uniformly to snapshot mode, per-member mode, and single-key mode in `connect()`. One Onyx self-test (`should squash all updates of collection-related keys into a single mergeCollection call`) was updated back to expect a single dedup'd fire — matching the legacy contract that the new timing restores. All 389 Onyx tests pass. All 15 originally failing expensify-app tests now pass with no other regressions in the affected suites. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/Onyx.ts | 48 ++++++++++++++++++++++++------------ lib/OnyxCache.ts | 12 ++++----- tests/unit/onyxCacheTest.tsx | 9 +++++-- tests/unit/onyxTest.ts | 13 +++++----- 4 files changed, 51 insertions(+), 31 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index a555f45d2..3ae188edc 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -123,13 +123,36 @@ function subscribeMembers(collectionKey: TKey, l } /** - * @deprecated Prefer `Onyx.subscribe()` / `Onyx.subscribeMembers()`. Kept as a - * compatibility wrapper for existing call sites; routes to the new primitives + * Kept as a compatibility wrapper for existing call sites; routes to the new primitives * based on the legacy `waitForCollectionCallback` option. * * Returns synchronously with a `Connection` handle. The underlying subscription * wires up after init completes; `disconnect()` works at any point. */ +/** + * 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) + * .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. + * + * 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 scheduleInitialFire(fn: () => void): void { + Promise.resolve() + .then(() => Promise.resolve()) + .then(() => Promise.resolve()) + .then(fn); +} + function connect(connectOptions: ConnectOptions): Connection { const {key, callback, waitForCollectionCallback} = connectOptions; @@ -144,14 +167,9 @@ function connect(connectOptions: ConnectOptions): Co if (OnyxKeys.isCollectionKey(key)) { if (waitForCollectionCallback === true) { // Snapshot mode — listener fires with the whole snapshot per collection change. - // - // The `sourceValue` 3rd-arg has been dropped (anti-pattern: leaked stale state - // through useOnyx, see PR #679). Callback shape is now `(snapshot, key)`. - // - // Legacy preserved on initial fire only: empty collection → `undefined` - // (matches old `sendDataToConnection`). Subsequent fires deliver the actual `{}`. - // - // Dedup: skip if the snapshot reference didn't change. + // Callback shape is `(snapshot, key)`; `sourceValue` has been dropped. + // Legacy semantic preserved on initial fire: empty collection → `undefined`. + // Subsequent fires deliver the actual `{}`. Dedup: skip identical snapshot refs. const NOT_DELIVERED = Symbol('NOT_DELIVERED'); let lastDeliveredSnapshot: unknown = NOT_DELIVERED; const deliverSnapshot = (rawSnapshot: OnyxValue | undefined, k: TKey, isInitialFire: boolean) => { @@ -166,7 +184,7 @@ function connect(connectOptions: ConnectOptions): Co unsubscribeFn = onyxStore.subscribe(key, (value, k) => { deliverSnapshot(value as unknown as OnyxValue, k as TKey, false); }); - Promise.resolve().then(() => { + scheduleInitialFire(() => { if (!active) { return; } @@ -185,7 +203,7 @@ function connect(connectOptions: ConnectOptions): Co (callback as DefaultConnectCallback | undefined)?.(value, memberKey as TKey); }; unsubscribeFn = onyxStore.subscribeMembers(key as CollectionKeyBase, deliverMember); - Promise.resolve().then(() => { + scheduleInitialFire(() => { if (!active || !callback) { return; } @@ -206,7 +224,6 @@ function connect(connectOptions: ConnectOptions): Co } // Non-collection key (or a specific collection member) — single-value subscription. - // Same dedup pattern. const NOT_DELIVERED = Symbol('NOT_DELIVERED'); let lastDelivered: unknown = NOT_DELIVERED; const deliverValue = (value: OnyxValue, k: TKey | undefined) => { @@ -219,7 +236,7 @@ function connect(connectOptions: ConnectOptions): Co unsubscribeFn = onyxStore.subscribe(key, (value, k) => { deliverValue(value, k as TKey); }); - Promise.resolve().then(() => { + scheduleInitialFire(() => { if (!active) { return; } @@ -253,8 +270,7 @@ function connect(connectOptions: ConnectOptions): Co } /** - * @deprecated Prefer `Onyx.subscribe()` / `Onyx.subscribeMembers()`. Identical to - * `connect()` — kept for naming consistency with existing call sites. + * Identical to `connect()` — kept for naming consistency with existing call sites. */ function connectWithoutView(connectOptions: ConnectOptions): Connection { return connect(connectOptions); diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index 725dbfd77..0ba913fc0 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -460,12 +460,12 @@ class OnyxCache { const snapshot = this.collectionSnapshots.get(collectionKey); if (utils.isEmptyObject(snapshot)) { - // We check storageKeys.size (not collection-specific keys) to distinguish - // "init complete, this collection is genuinely empty" from "init not done yet." - // During init, setAllKeys loads ALL keys at once — so if any key exists, - // the full storage picture is loaded and an empty collection is truly empty. - // Returning undefined before init prevents subscribers from seeing a false empty state. - if (this.storageKeys.size > 0) { + // Distinguish "init complete, collection genuinely empty" from "init not done yet." + // `setCollectionKeys()` (called inside `Onyx.init`) seeds every known collection + // with a frozen `{}` entry in `collectionSnapshots`, so the presence of the entry + // is a reliable post-init signal — and unlike `storageKeys.size > 0`, it doesn't + // flip back to "not done" after `Onyx.clear()` wipes the storage-keys index. + if (this.collectionSnapshots.has(collectionKey)) { return FROZEN_EMPTY_COLLECTION; } return undefined; diff --git a/tests/unit/onyxCacheTest.tsx b/tests/unit/onyxCacheTest.tsx index 713232459..8a1daa52a 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/onyxTest.ts b/tests/unit/onyxTest.ts index 618e24f25..2d444a0f9 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -1621,14 +1621,13 @@ describe('Onyx', () => { }, }, ]).then(() => { - // The store-based wrapper delivers two callbacks: an initial fire with - // `undefined` (empty cache at subscribe time) and a second fire with the - // final post-update snapshot. The legacy ConnectionManager's deep-promise - // chain accidentally suppressed the first one; the new wrapper doesn't. - expect(routesCollectionCallback).toHaveBeenCalledTimes(2); - expect(routesCollectionCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.ROUTES); + // 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( - 2, + 1, { [holidayRoute]: { waypoints: { From 9cc6312249fd590585b5295385d4e66dbb37f5d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Wed, 27 May 2026 14:27:26 +0100 Subject: [PATCH 5/8] Remove subscribe and subscribeMembers from public API --- lib/Onyx.ts | 36 ++++-------------------------------- 1 file changed, 4 insertions(+), 32 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 3ae188edc..9dd1159ff 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -32,7 +32,7 @@ import onyxStore from './OnyxStore'; import OnyxMerge from './OnyxMerge'; /** - * Opaque handle returned by `Onyx.connect()` / `Onyx.connectWithoutView()` / `Onyx.subscribe()`. + * Opaque handle returned by `Onyx.connect()` / `Onyx.connectWithoutView()`. * Pass it to `Onyx.disconnect()` to stop receiving callbacks for this subscription. */ type Connection = { @@ -96,35 +96,9 @@ function getState(key: TKey): OnyxValue { } /** - * Subscribe to a single Onyx key, or to a collection key in snapshot mode. - * - * For collection keys, the listener fires with the entire frozen collection snapshot - * every time any member changes. For non-collection keys, the listener fires with the - * value when that key changes. - * - * @returns An unsubscribe function. - */ -function subscribe(key: TKey, listener: (value: OnyxValue, key: TKey) => void): () => void { - return onyxStore.subscribe(key, listener); -} - -/** - * Subscribe to per-member changes on a collection key. The listener fires once per - * changed member with `(memberValue, memberKey)`. - * - * Use this when you care about which specific member changed (e.g. keeping a local - * map indexed by member ID up-to-date). For "always deliver the whole collection", - * use `subscribe()` instead. - * - * @returns An unsubscribe function. - */ -function subscribeMembers(collectionKey: TKey, listener: (value: OnyxValue, memberKey: OnyxKey) => void): () => void { - return onyxStore.subscribeMembers(collectionKey, listener); -} - -/** - * Kept as a compatibility wrapper for existing call sites; routes to the new primitives - * based on the legacy `waitForCollectionCallback` option. + * Subscribe to changes for `key`. Collection keys fire the whole snapshot when any member + * changes; non-collection keys fire when that key changes. Pass `waitForCollectionCallback: false` + * on a collection key to instead fire per changed member with `(memberValue, memberKey)`. * * Returns synchronously with a `Connection` handle. The underlying subscription * wires up after init completes; `disconnect()` works at any point. @@ -711,8 +685,6 @@ function setCollection(collectionKey: TKey, coll const Onyx = { METHOD: OnyxUtils.METHOD, getState, - subscribe, - subscribeMembers, connect, connectWithoutView, disconnect, From d59a04ac83c38598ea1c4ce30e80fc24867e1026 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Wed, 27 May 2026 19:20:31 +0100 Subject: [PATCH 6/8] Remove waitForCollectionCallback --- lib/Onyx.ts | 89 +++++++-------------- lib/OnyxStore.ts | 111 ++++++-------------------- lib/types.ts | 53 ++++-------- tests/unit/collectionHydrationTest.ts | 70 ---------------- tests/unit/onyxTest.ts | 101 ++++++++++++----------- 5 files changed, 117 insertions(+), 307 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 9dd1159ff..48f7a7b3e 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -95,14 +95,6 @@ function getState(key: TKey): OnyxValue { return onyxStore.getState(key); } -/** - * Subscribe to changes for `key`. Collection keys fire the whole snapshot when any member - * changes; non-collection keys fire when that key changes. Pass `waitForCollectionCallback: false` - * on a collection key to instead fire per changed member with `(memberValue, memberKey)`. - * - * Returns synchronously with a `Connection` handle. The underlying subscription - * wires up after init completes; `disconnect()` works at any point. - */ /** * 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. @@ -127,8 +119,19 @@ function scheduleInitialFire(fn: () => void): void { .then(fn); } +/** + * 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)`. + * 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. + * + * Returns synchronously with a `Connection` handle. Disconnecting is idempotent. + */ function connect(connectOptions: ConnectOptions): Connection { - const {key, callback, waitForCollectionCallback} = connectOptions; + const {key, callback} = connectOptions; let active = true; let unsubscribeFn: (() => void) | null = null; @@ -139,60 +142,29 @@ function connect(connectOptions: ConnectOptions): Co } if (OnyxKeys.isCollectionKey(key)) { - if (waitForCollectionCallback === true) { - // Snapshot mode — listener fires with the whole snapshot per collection change. - // Callback shape is `(snapshot, key)`; `sourceValue` has been dropped. - // Legacy semantic preserved on initial fire: empty collection → `undefined`. - // Subsequent fires deliver the actual `{}`. Dedup: skip identical snapshot refs. - const NOT_DELIVERED = Symbol('NOT_DELIVERED'); - let lastDeliveredSnapshot: unknown = NOT_DELIVERED; - const deliverSnapshot = (rawSnapshot: OnyxValue | undefined, k: TKey, isInitialFire: boolean) => { - if (Object.is(lastDeliveredSnapshot, rawSnapshot)) { - return; - } - lastDeliveredSnapshot = rawSnapshot; - const isEmpty = rawSnapshot !== undefined && rawSnapshot !== null && typeof rawSnapshot === 'object' && Object.keys(rawSnapshot as object).length === 0; - const valueToDeliver = isInitialFire && isEmpty ? undefined : rawSnapshot; - (callback as CollectionConnectCallback | undefined)?.(valueToDeliver as NonNullable>, k); - }; - unsubscribeFn = onyxStore.subscribe(key, (value, k) => { - deliverSnapshot(value as unknown as OnyxValue, k as TKey, false); - }); - scheduleInitialFire(() => { - if (!active) { - return; - } - deliverSnapshot(onyxStore.getState(key) as unknown as OnyxValue, key as TKey, true); - }); - return; - } - - // Per-member mode — one callback per changed member. Dedup per member key. - const memberLastDelivered = new Map(); - const deliverMember = (value: OnyxValue, memberKey: OnyxKey) => { - if (memberLastDelivered.has(memberKey) && Object.is(memberLastDelivered.get(memberKey), value)) { + // Collection-root snapshot mode — listener fires with the whole snapshot per + // collection change. Callback shape is `(snapshot, key)`. Legacy semantic + // preserved on initial fire: empty collection → `undefined`. Subsequent fires + // deliver the actual `{}`. Dedup: skip identical snapshot refs. + const NOT_DELIVERED = Symbol('NOT_DELIVERED'); + let lastDeliveredSnapshot: unknown = NOT_DELIVERED; + const deliverSnapshot = (rawSnapshot: OnyxValue | undefined, k: TKey, isInitialFire: boolean) => { + if (Object.is(lastDeliveredSnapshot, rawSnapshot)) { return; } - memberLastDelivered.set(memberKey, value); - (callback as DefaultConnectCallback | undefined)?.(value, memberKey as TKey); + lastDeliveredSnapshot = rawSnapshot; + const isEmpty = rawSnapshot !== undefined && rawSnapshot !== null && typeof rawSnapshot === 'object' && Object.keys(rawSnapshot as object).length === 0; + const valueToDeliver = isInitialFire && isEmpty ? undefined : rawSnapshot; + (callback as CollectionConnectCallback | undefined)?.(valueToDeliver as NonNullable>, k); }; - unsubscribeFn = onyxStore.subscribeMembers(key as CollectionKeyBase, deliverMember); + unsubscribeFn = onyxStore.subscribe(key, (value, k) => { + deliverSnapshot(value as unknown as OnyxValue, k as TKey, false); + }); scheduleInitialFire(() => { - if (!active || !callback) { + if (!active) { return; } - const initial = onyxStore.getState(key) as OnyxCollection | undefined; - const memberKeys = initial ? Object.keys(initial) : []; - if (memberKeys.length === 0) { - // Legacy semantic: when a per-member subscription finds no existing - // members, fire once with (undefined, undefined) so callers can clear - // any prior state. Matches the old `sendDataToConnection(mapping, undefined)`. - (callback as DefaultConnectCallback)(undefined as OnyxValue, undefined as unknown as TKey); - return; - } - for (const memberKey of memberKeys) { - deliverMember(initial?.[memberKey], memberKey); - } + deliverSnapshot(onyxStore.getState(key) as unknown as OnyxValue, key as TKey, true); }); return; } @@ -251,8 +223,7 @@ function connectWithoutView(connectOptions: ConnectOptions } /** - * Disconnects a subscription previously returned by `connect()` / `connectWithoutView()` - * / `subscribe()` / `subscribeMembers()`. + * Disconnects a subscription previously returned by `connect()` / `connectWithoutView()`. */ function disconnect(connection: Connection): void { if (!connection) { diff --git a/lib/OnyxStore.ts b/lib/OnyxStore.ts index e7adbb41c..b74ccc1dc 100644 --- a/lib/OnyxStore.ts +++ b/lib/OnyxStore.ts @@ -9,12 +9,6 @@ import type {CollectionKeyBase, KeyValueMapping, OnyxCollection, OnyxKey, OnyxVa */ type KeyListener = (value: OnyxValue, key: TKey) => void; -/** - * Listener fired once per changed member when any member of a collection changes. - * Replaces today's "collection key without waitForCollectionCallback" delivery mode. - */ -type MemberListener = (value: OnyxValue, memberKey: OnyxKey) => void; - /** * Listener fired when any of a state-listener's declared dep keys changes. */ @@ -27,13 +21,11 @@ type StateListenerEntry = { /** * `OnyxStore` is the listener registry that replaces `OnyxConnectionManager`, - * `OnyxSnapshotCache`, and the subscriber-half of `OnyxUtils`. It owns three + * `OnyxSnapshotCache`, and the subscriber-half of `OnyxUtils`. It owns two * indexes: * * keyListeners — listeners on an exact key (single key, collection root * in snapshot mode, or a specific collection member). - * memberListeners — listeners on "any member of this collection" — fires - * once per changed member, not once per collection change. * stateListeners(ByDep) — listeners that re-evaluate when any of their declared * deps change. Indexed by dep key for O(1) lookup in notify(). * @@ -43,15 +35,12 @@ type StateListenerEntry = { class OnyxStore { private keyListeners: Map>; - private memberListeners: Map>; - private stateListeners: Set; private stateListenersByDep: Map>; constructor() { this.keyListeners = new Map(); - this.memberListeners = new Map(); this.stateListeners = new Set(); this.stateListenersByDep = new Map(); } @@ -94,32 +83,6 @@ class OnyxStore { }; } - /** - * Subscribe to per-member changes on a collection. The listener fires once per - * changed member, with `(memberValue, memberKey)`. Replaces today's "collection - * key without waitForCollectionCallback" delivery mode. - * - * Returns an unsubscribe function. - */ - subscribeMembers(collectionKey: TKey, listener: MemberListener): () => void { - let listeners = this.memberListeners.get(collectionKey); - if (!listeners) { - listeners = new Set(); - this.memberListeners.set(collectionKey, listeners); - } - listeners.add(listener); - return () => { - const set = this.memberListeners.get(collectionKey); - if (!set) { - return; - } - set.delete(listener); - if (set.size === 0) { - this.memberListeners.delete(collectionKey); - } - }; - } - /** * Subscribe to state-tree changes. The listener fires when any of the declared * deps changes. Used by `useOnyxState`. @@ -157,13 +120,12 @@ class OnyxStore { * * Dispatch: * 1. keyListeners.get(key) — exact-key subscribers (always fires) - * 2. If key is a collection member: - * 2a. keyListeners.get(collectionKey) — snapshot subscribers (unless suppressed) - * 2b. memberListeners.get(collectionKey) — per-member subscribers + * 2. If key is a collection member: keyListeners.get(collectionKey) — snapshot + * subscribers for the parent collection (unless suppressed). * 3. State listeners whose deps include `key` or its collection key. * - * `options.suppressCollectionSnapshot` skips step 2a — used by collection-batch - * write paths so each removed/changed member doesn't re-trigger the collection-level + * `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 { @@ -175,25 +137,17 @@ class OnyxStore { } } - // 2. Collection-level routing — only fires when the write is to a member 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) { - if (!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); - } - } - } - const members = this.memberListeners.get(collectionKey); - if (members && members.size > 0) { - for (const listener of members) { - this.safeInvoke(() => listener(value as OnyxValue, key), 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); } } } @@ -212,10 +166,9 @@ class OnyxStore { * * Dispatch: * 1. keyListeners.get(collectionKey) — fires ONCE with the new snapshot. - * 2. memberListeners.get(collectionKey) — fires once per changed member. - * 3. keyListeners.get(memberKey) — fires per changed member where the value + * 2. keyListeners.get(memberKey) — fires per changed member where the value * differs from the previous (for ref-equality on unchanged members). - * 4. State listeners affected by the collection key OR any changed member key, + * 3. State listeners affected by the collection key OR any changed member key, * each fired at most once. */ notifyCollection( @@ -229,10 +182,9 @@ class OnyxStore { } const previous = partialPreviousCollection ?? {}; - // Read the merged snapshot once; reuse for snapshot-mode AND for per-member reads. - // `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 from the previous values during merge). + // 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. @@ -243,18 +195,7 @@ class OnyxStore { } } - // 2. Per-member subscribers fire once per changed member with the merged value. - const members = this.memberListeners.get(collectionKey); - if (members && members.size > 0) { - for (const memberKey of changedKeys) { - const value = snapshot?.[memberKey]; - for (const listener of members) { - this.safeInvoke(() => listener(value as OnyxValue, memberKey), memberKey); - } - } - } - - // 3. Exact-member subscribers fire per changed key (skip if ref unchanged vs previous). + // 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]; @@ -270,7 +211,7 @@ class OnyxStore { } } - // 4. State listeners — each affected entry fires at most once. + // 3. State listeners — each affected entry fires at most once. const fired = new Set(); this.fireStateListenersForDep(collectionKey, fired); for (const memberKey of changedKeys) { @@ -281,24 +222,18 @@ class OnyxStore { /** Wipe all subscriptions. Used by tests and `Onyx.clear()` follow-on. */ clearAll(): void { this.keyListeners.clear(); - this.memberListeners.clear(); this.stateListeners.clear(); this.stateListenersByDep.clear(); } - /** True if there are any subscribers for the given key (exact or member). */ + /** 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) { - if ((this.keyListeners.get(collectionKey)?.size ?? 0) > 0) { - return true; - } - if ((this.memberListeners.get(collectionKey)?.size ?? 0) > 0) { - return true; - } + if (collectionKey && collectionKey !== key && (this.keyListeners.get(collectionKey)?.size ?? 0) > 0) { + return true; } return false; } @@ -329,4 +264,4 @@ class OnyxStore { const onyxStore = new OnyxStore(); export default onyxStore; -export type {KeyListener, MemberListener, StateListenerCallback}; +export type {KeyListener, StateListenerCallback}; diff --git a/lib/types.ts b/lib/types.ts index 0be5dba2e..a8b273475 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -225,48 +225,26 @@ type DefaultConnectCallback = (value: OnyxEntry = (value: NonUndefined>, key: TKey) => void; -/** Represents the options used in `Onyx.connect()` method with a regular key. */ -// NOTE: Any changes to this type like adding or removing options must be accounted in OnyxConnectionManager's `generateConnectionID()` method! -type DefaultConnectOptions = BaseConnectOptions & { - /** The Onyx key to subscribe to. */ - key: TKey; - - /** A function that will be called when the Onyx data we are subscribed changes. */ - callback?: DefaultConnectCallback; - - /** If set to `true`, it will return the entire collection to the callback as a single object. */ - waitForCollectionCallback?: false; -}; - -/** Represents the options used in `Onyx.connect()` method with a collection key. */ -// NOTE: Any changes to this type like adding or removing options must be accounted in OnyxConnectionManager's `generateConnectionID()` method! -type CollectionConnectOptions = BaseConnectOptions & { - /** The Onyx key to subscribe to. */ - key: TKey extends CollectionKeyBase ? TKey : never; - - /** A function that will be called when the Onyx data we are subscribed changes. */ - callback?: CollectionConnectCallback; - - /** If set to `true`, it will return the entire collection to the callback as a single object. */ - waitForCollectionCallback: true; -}; - /** * Represents the options used in `Onyx.connect()` method. - * The type is built from `DefaultConnectOptions`/`CollectionConnectOptions` depending on the `waitForCollectionCallback` property. - * It includes two different forms, depending on whether we are waiting for a collection callback or not. * - * If `waitForCollectionCallback` is `true`, it expects `key` to be a Onyx collection key and `callback` will be triggered with the whole collection - * and will pass `value` as an `OnyxCollection`. + * For a collection root key (e.g. `ONYXKEYS.COLLECTION.REPORT`), the callback fires + * with the entire collection snapshot whenever any member changes (signature + * `(collection, key)`). For any other key, the callback fires with the value at + * that key (signature `(value, key)`). * - * If `waitForCollectionCallback` is `false` or not specified, the `key` can be any Onyx key and `callback` will be triggered with updates of each collection item - * and will pass `value` as an `OnyxEntry`. + * The legacy `waitForCollectionCallback` flag has been removed — collection-root + * subscriptions always deliver snapshots. Per-member dispatch (the old default) + * is no longer supported; consumers that need per-member processing should + * subscribe to the snapshot and diff against the previous value (structural + * sharing makes the per-member ref-check O(1)). */ -// NOTE: Any changes to this type like adding or removing options must be accounted in OnyxConnectionManager's `generateConnectionID()` method! -type ConnectOptions = DefaultConnectOptions | CollectionConnectOptions; +type ConnectOptions = BaseConnectOptions & { + /** The Onyx key to subscribe to. */ + key: TKey; -type CallbackToStateMapping = ConnectOptions & { - subscriptionID: number; + /** A function that will be called when the Onyx data we are subscribed changes. */ + callback?: TKey extends CollectionKeyBase ? CollectionConnectCallback : DefaultConnectCallback; }; /** @@ -447,20 +425,17 @@ export type { BaseConnectOptions, Collection, CollectionConnectCallback, - CollectionConnectOptions, CollectionKey, CollectionKeyBase, ConnectOptions, CustomTypeOptions, DeepRecord, DefaultConnectCallback, - DefaultConnectOptions, ExtractOnyxCollectionValue, GenericFunction, InitOptions, Key, KeyValueMapping, - CallbackToStateMapping, NonNull, NonUndefined, OnyxInputKeyValueMapping, diff --git a/tests/unit/collectionHydrationTest.ts b/tests/unit/collectionHydrationTest.ts index 64de44412..f44b23fe0 100644 --- a/tests/unit/collectionHydrationTest.ts +++ b/tests/unit/collectionHydrationTest.ts @@ -51,43 +51,6 @@ describe('Collection hydration with connect() followed by immediate set()', () = expect(lastCall[`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]).toEqual({id: 1, title: 'Updated Test One'}); }); - test('waitForCollectionCallback=false should deliver all shallow-equal collection members when set() races with hydration', async () => { - // Clear existing storage and set up shallow-equal values for all members - await StorageMock.clear(); - await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`, {status: 'active'}); - await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`, {status: 'active'}); - await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.TEST_KEY}3`, {status: 'active'}); - // Re-init so Onyx picks up the new storage keys - Onyx.init({keys: ONYX_KEYS}); - - const mockCallback = jest.fn(); - - Onyx.connect({ - key: ONYX_KEYS.COLLECTION.TEST_KEY, - waitForCollectionCallback: false, - callback: mockCallback, - }); - - // set() with the same shallow-equal value — this fires keyChanged synchronously, - // populating lastConnectionCallbackData before the hydration multiGet resolves. - Onyx.set(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`, {status: 'active'}); - - await waitForPromisesToResolve(); - - const deliveredKeys = new Set(); - for (const call of mockCallback.mock.calls) { - const [, key] = call; - if (key) { - deliveredKeys.add(key); - } - } - - // ALL three members must be delivered, even though their values are shallow-equal. - expect(deliveredKeys).toContain(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`); - expect(deliveredKeys).toContain(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`); - expect(deliveredKeys).toContain(`${ONYX_KEYS.COLLECTION.TEST_KEY}3`); - }); - test('single key: set() with non-shallow-equal value should not be overwritten by stale hydration', async () => { const mockCallback = jest.fn(); @@ -129,37 +92,4 @@ describe('Collection hydration with connect() followed by immediate set()', () = expect(lastCall[`${ONYX_KEYS.COLLECTION.TEST_KEY}3`]).toEqual({id: 3, title: 'Test Three'}); }); - test('waitForCollectionCallback=false should deliver all collection members from storage', async () => { - const mockCallback = jest.fn(); - - // A component connects to the collection (callback fires per key, not batched). - Onyx.connect({ - key: ONYX_KEYS.COLLECTION.TEST_KEY, - waitForCollectionCallback: false, - callback: mockCallback, - }); - - Onyx.set(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`, {id: 1, title: 'Updated Test One'}); - - await waitForPromisesToResolve(); - - // With waitForCollectionCallback=false, the callback fires per key individually. - // Collect all keys that were delivered across all calls. - const deliveredKeys = new Set(); - for (const call of mockCallback.mock.calls) { - const [, key] = call; - if (key) { - deliveredKeys.add(key); - } - } - - expect(deliveredKeys).toContain(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`); - expect(deliveredKeys).toContain(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`); - expect(deliveredKeys).toContain(`${ONYX_KEYS.COLLECTION.TEST_KEY}3`); - - // Verify the updated value is present (not stale) by finding the last call for key 1 - const key1Calls = mockCallback.mock.calls.filter((call) => call[1] === `${ONYX_KEYS.COLLECTION.TEST_KEY}1`); - const lastKey1Value = key1Calls[key1Calls.length - 1][0]; - expect(lastKey1Value).toEqual({id: 1, title: 'Updated Test One'}); - }); }); diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 2d444a0f9..7d7b554c5 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -644,7 +644,6 @@ describe('Onyx', () => { }); it('should properly set and merge when using mergeCollection', async () => { - const valuesReceived: Record = {}; const mockCallback = jest.fn(); connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, @@ -653,7 +652,6 @@ describe('Onyx', () => { await waitForPromisesToResolve(); mockCallback.mockReset(); - mockCallback.mockImplementation((data) => (valuesReceived[data.ID] = data.value)); // The first time we call mergeCollection we'll be doing a multiSet internally return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { @@ -694,20 +692,25 @@ describe('Onyx', () => { } as GenericCollection), ) .then(() => { - // 3 items on the first mergeCollection + 4 items the next mergeCollection - expect(mockCallback).toHaveBeenCalledTimes(7); - expect(mockCallback).toHaveBeenNthCalledWith(1, {ID: 123, value: 'one'}, 'test_1'); - expect(mockCallback).toHaveBeenNthCalledWith(2, {ID: 234, value: 'two'}, 'test_2'); - expect(mockCallback).toHaveBeenNthCalledWith(3, {ID: 345, value: 'three'}, 'test_3'); - expect(mockCallback).toHaveBeenNthCalledWith(4, {ID: 123, value: 'five'}, 'test_1'); - expect(mockCallback).toHaveBeenNthCalledWith(5, {ID: 234, value: 'four'}, 'test_2'); - expect(mockCallback).toHaveBeenNthCalledWith(6, {ID: 456, value: 'two'}, 'test_4'); - expect(mockCallback).toHaveBeenNthCalledWith(7, {ID: 567, value: 'one'}, 'test_5'); - expect(valuesReceived[123]).toEqual('five'); - expect(valuesReceived[234]).toEqual('four'); - expect(valuesReceived[345]).toEqual('three'); - expect(valuesReceived[456]).toEqual('two'); - expect(valuesReceived[567]).toEqual('one'); + // Snapshot mode: callback fires once per mergeCollection with the full snapshot. + expect(mockCallback).toHaveBeenCalledTimes(2); + expect(mockCallback).toHaveBeenNthCalledWith( + 1, + {test_1: {ID: 123, value: 'one'}, test_2: {ID: 234, value: 'two'}, test_3: {ID: 345, value: 'three'}}, + ONYX_KEYS.COLLECTION.TEST_KEY, + ); + expect(mockCallback).toHaveBeenNthCalledWith( + 2, + { + test_1: {ID: 123, value: 'five'}, + test_2: {ID: 234, value: 'four'}, + // test_3 unchanged (incompatible array merge rejected) + test_3: {ID: 345, value: 'three'}, + test_4: {ID: 456, value: 'two'}, + test_5: {ID: 567, value: 'one'}, + }, + ONYX_KEYS.COLLECTION.TEST_KEY, + ); }); }); @@ -724,10 +727,10 @@ describe('Onyx', () => { }); it('should return full object to callback when calling mergeCollection()', () => { - const valuesReceived: Record = {}; + let lastSnapshot: unknown; connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, - callback: (data, key) => (valuesReceived[key] = data), + callback: (snapshot) => (lastSnapshot = snapshot), }); return Onyx.multiSet({ @@ -751,7 +754,7 @@ describe('Onyx', () => { } as GenericCollection), ) .then(() => { - expect(valuesReceived).toEqual({ + expect(lastSnapshot).toEqual({ test_1: { ID: 123, value: 'one', @@ -897,7 +900,6 @@ describe('Onyx', () => { }); it('should use update data object to merge a collection of keys', () => { - const valuesReceived: Record = {}; const mockCallback = jest.fn(); connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, @@ -907,7 +909,6 @@ describe('Onyx', () => { return waitForPromisesToResolve() .then(() => { mockCallback.mockReset(); - mockCallback.mockImplementation((data) => (valuesReceived[data.ID] = data.value)); // Given the initial Onyx state: {test_1: {existingData: 'test',}, test_2: {existingData: 'test',}} Onyx.multiSet({ @@ -921,8 +922,12 @@ describe('Onyx', () => { return waitForPromisesToResolve(); }) .then(() => { - expect(mockCallback).toHaveBeenNthCalledWith(1, {existingData: 'test'}, 'test_1'); - expect(mockCallback).toHaveBeenNthCalledWith(2, {existingData: 'test'}, 'test_2'); + // 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 return Onyx.update([ @@ -947,36 +952,24 @@ describe('Onyx', () => { ]); }) .then(() => { - /* Then the final Onyx state should be: + // mergeCollection fires the collection snapshot once with all 3 merged members. + expect(mockCallback).toHaveBeenCalledTimes(1); + expect(mockCallback).toHaveBeenCalledWith( { - test_1: { - existingData: 'test' - ID: 123, - value: 'one', - }, - test_2: { - existingData: 'test' - ID: 234, - value: 'two', - }, - test_3: { - ID: 345, - value: 'three', - }, - } - */ - - expect(mockCallback).toHaveBeenNthCalledWith(3, {ID: 123, value: 'one', existingData: 'test'}, 'test_1'); - expect(mockCallback).toHaveBeenNthCalledWith(4, {ID: 234, value: 'two', existingData: 'test'}, 'test_2'); - expect(mockCallback).toHaveBeenNthCalledWith(5, {ID: 345, value: 'three'}, 'test_3'); + 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, + ); }); }); it('should properly set all keys provided in a multiSet called via update', () => { - const valuesReceived: Record = {}; + let lastSnapshot: unknown; connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, - callback: (data, key) => (valuesReceived[key] = data), + callback: (snapshot) => (lastSnapshot = snapshot), }); return Onyx.multiSet({ @@ -1005,7 +998,7 @@ describe('Onyx', () => { ] as unknown as Array>), ) .then(() => { - expect(valuesReceived).toEqual({ + expect(lastSnapshot).toEqual({ test_1: { ID: 123, value: 'one', @@ -1500,9 +1493,10 @@ describe('Onyx', () => { await Onyx.update([{key: cat, value: finalValue, onyxMethod: Onyx.METHOD.MERGE}]); + // Snapshot mode: callback fires with the whole SNAPSHOT-collection snapshot. expect(callback).toBeCalledTimes(2); - expect(callback).toHaveBeenNthCalledWith(1, {data: {[cat]: initialValue}}, snapshot1); - expect(callback).toHaveBeenNthCalledWith(2, {data: {[cat]: finalValue}}, snapshot1); + 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 () => { @@ -1531,9 +1525,14 @@ describe('Onyx', () => { await Onyx.update([{key: cat, value: finalValue, onyxMethod: Onyx.METHOD.MERGE}]); + // Snapshot mode: callback fires with the whole SNAPSHOT-collection snapshot. expect(callback).toBeCalledTimes(2); - expect(callback).toHaveBeenNthCalledWith(1, {data: {[cat]: initialValue}}, snapshot1); - expect(callback).toHaveBeenNthCalledWith(2, {data: {[cat]: {name: 'Kitty', pendingAction: 'delete', pendingFields: {preview: 'delete'}}}}, snapshot1); + 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', () => { From 9f33b83df2c3bb80a2af2f793f1d85345f6e55f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Wed, 27 May 2026 21:48:29 +0100 Subject: [PATCH 7/8] Remove Legacy undefined-on-initial-empty behavior for collection keys --- lib/Onyx.ts | 17 ++++++++--------- tests/unit/onyxClearWebStorageTest.ts | 5 +++-- tests/unit/onyxTest.ts | 19 +++++++++++++------ 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 48f7a7b3e..27b45dfa2 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -143,28 +143,27 @@ 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)`. Legacy semantic - // preserved on initial fire: empty collection → `undefined`. Subsequent fires - // deliver the actual `{}`. Dedup: skip identical snapshot refs. + // 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). const NOT_DELIVERED = Symbol('NOT_DELIVERED'); let lastDeliveredSnapshot: unknown = NOT_DELIVERED; - const deliverSnapshot = (rawSnapshot: OnyxValue | undefined, k: TKey, isInitialFire: boolean) => { + const deliverSnapshot = (rawSnapshot: OnyxValue | undefined, k: TKey) => { if (Object.is(lastDeliveredSnapshot, rawSnapshot)) { return; } lastDeliveredSnapshot = rawSnapshot; - const isEmpty = rawSnapshot !== undefined && rawSnapshot !== null && typeof rawSnapshot === 'object' && Object.keys(rawSnapshot as object).length === 0; - const valueToDeliver = isInitialFire && isEmpty ? undefined : rawSnapshot; - (callback as CollectionConnectCallback | undefined)?.(valueToDeliver as NonNullable>, k); + (callback as CollectionConnectCallback | undefined)?.(rawSnapshot as NonNullable>, k); }; unsubscribeFn = onyxStore.subscribe(key, (value, k) => { - deliverSnapshot(value as unknown as OnyxValue, k as TKey, false); + deliverSnapshot(value as unknown as OnyxValue, k as TKey); }); scheduleInitialFire(() => { if (!active) { return; } - deliverSnapshot(onyxStore.getState(key) as unknown as OnyxValue, key as TKey, true); + deliverSnapshot(onyxStore.getState(key) as unknown as OnyxValue, key as TKey); }); return; } diff --git a/tests/unit/onyxClearWebStorageTest.ts b/tests/unit/onyxClearWebStorageTest.ts index 6cb4868e2..2546d815a 100644 --- a/tests/unit/onyxClearWebStorageTest.ts +++ b/tests/unit/onyxClearWebStorageTest.ts @@ -239,8 +239,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 7d7b554c5..2a005f008 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -1068,8 +1068,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); @@ -1126,8 +1128,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); }) ); @@ -1200,7 +1202,8 @@ 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); @@ -3146,7 +3149,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(); From ec1b88b6033b9e827227f626c00464fee1ccad85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 29 May 2026 07:30:44 +0100 Subject: [PATCH 8/8] Remove `(undefined, undefined)` no-match legacy behavior --- lib/Onyx.ts | 8 +------- tests/unit/onyxTest.ts | 9 ++++++--- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 27b45dfa2..3bd46185f 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -185,13 +185,7 @@ function connect(connectOptions: ConnectOptions): Co if (!active) { return; } - // Legacy semantic: when there's no cached data for the key, fire with - // `(undefined, undefined)` as the "no match" signal — matches what the old - // `sendDataToConnection(mapping, undefined)` did. When data exists, fire - // with `(value, key)`. - const hasCache = cache.hasCacheForKey(key); - const initial = onyxStore.getState(key); - deliverValue(initial, hasCache ? (key as TKey) : (undefined as unknown as TKey)); + deliverValue(onyxStore.getState(key), key); }); }; diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 2a005f008..c3a1f40ba 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -1099,8 +1099,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'); @@ -1207,7 +1209,8 @@ describe('Onyx', () => { 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);