From 93a1478309c5bb2a3132ccc31a40186fc8388df3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 19 Mar 2026 17:02:39 +0000 Subject: [PATCH 1/4] perf: reference equality in notification paths and useOnyx Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/OnyxUtils.ts | 105 ++++++++++++++++++----------------------------- lib/useOnyx.ts | 37 +++++++---------- 2 files changed, 54 insertions(+), 88 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 8723c013d..7053357e2 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1,4 +1,4 @@ -import {deepEqual, shallowEqual} from 'fast-equals'; +import {shallowEqual} from 'fast-equals'; import type {ValueOf} from 'type-fest'; import _ from 'underscore'; import DevTools from './DevTools'; @@ -506,8 +506,8 @@ function getCachedCollection(collectionKey: TKey return filteredCollection; } - // Return a copy to avoid mutations affecting the cache - return {...collectionData}; + // Snapshot is frozen — safe to return by reference + return collectionData; } // Fallback to original implementation if collection data not available @@ -542,78 +542,50 @@ function keysChanged( partialCollection: OnyxCollection, partialPreviousCollection: OnyxCollection | undefined, ): void { - // We prepare the "cached collection" which is the entire collection + the new partial data that - // was merged in via mergeCollection(). const cachedCollection = getCachedCollection(collectionKey); - const previousCollection = partialPreviousCollection ?? {}; + const changedMemberKeys = Object.keys(partialCollection ?? {}); + + // Use indexed lookup instead of scanning all subscribers + 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); + } + } + } - // We are iterating over all subscribers similar to keyChanged(). However, we are looking for subscribers who are subscribing to either a collection key or - // individual collection key member for the collection that is being updated. It is important to note that the collection parameter cane be a PARTIAL collection - // and does not represent all of the combined keys and values for a collection key. It is just the "new" data that was merged in via mergeCollection(). - const stateMappingKeys = Object.keys(callbackToStateMapping); + // Notify collection-level subscribers + for (const subID of collectionSubscriberIDs) { + const subscriber = callbackToStateMapping[subID]; + if (!subscriber || typeof subscriber.callback !== 'function') continue; - for (const stateMappingKey of stateMappingKeys) { - const subscriber = callbackToStateMapping[stateMappingKey]; - if (!subscriber) { - continue; - } + lastConnectionCallbackData.set(subscriber.subscriptionID, cachedCollection); - // Skip iteration if we do not have a collection key or a collection member key on this subscriber - if (!Str.startsWith(subscriber.key, collectionKey)) { + if (subscriber.waitForCollectionCallback) { + subscriber.callback(cachedCollection, subscriber.key, partialCollection); continue; } - /** - * e.g. Onyx.connect({key: ONYXKEYS.COLLECTION.REPORT, callback: ...}); - */ - const isSubscribedToCollectionKey = subscriber.key === collectionKey; - - /** - * e.g. Onyx.connect({key: `${ONYXKEYS.COLLECTION.REPORT}{reportID}`, callback: ...}); - */ - const isSubscribedToCollectionMemberKey = OnyxKeys.isCollectionMemberKey(collectionKey, subscriber.key); - - // Regular Onyx.connect() subscriber found. - if (typeof subscriber.callback === 'function') { - // If they are subscribed to the collection key and using waitForCollectionCallback then we'll - // send the whole cached collection. - if (isSubscribedToCollectionKey) { - lastConnectionCallbackData.set(subscriber.subscriptionID, cachedCollection); - - if (subscriber.waitForCollectionCallback) { - subscriber.callback(cachedCollection, subscriber.key, partialCollection); - continue; - } - - // If they are not using waitForCollectionCallback then we notify the subscriber with - // the new merged data but only for any keys in the partial collection. - const dataKeys = Object.keys(partialCollection ?? {}); - for (const dataKey of dataKeys) { - if (deepEqual(cachedCollection[dataKey], previousCollection[dataKey])) { - continue; - } - - subscriber.callback(cachedCollection[dataKey], dataKey); - } - continue; - } - - // And if the subscriber is specifically only tracking a particular collection member key then we will - // notify them with the cached data for that key only. - if (isSubscribedToCollectionMemberKey) { - if (deepEqual(cachedCollection[subscriber.key], previousCollection[subscriber.key])) { - continue; - } + // Not using waitForCollectionCallback — notify per changed key + for (const dataKey of changedMemberKeys) { + if (cachedCollection[dataKey] === previousCollection[dataKey]) continue; // === instead of deepEqual + subscriber.callback(cachedCollection[dataKey], dataKey); + } + } - const subscriberCallback = subscriber.callback as DefaultConnectCallback; - subscriberCallback(cachedCollection[subscriber.key], subscriber.key as TKey); - lastConnectionCallbackData.set(subscriber.subscriptionID, cachedCollection[subscriber.key]); - continue; - } + // Notify member-level subscribers + for (const subID of memberSubscriberIDs) { + const subscriber = callbackToStateMapping[subID]; + if (!subscriber || typeof subscriber.callback !== 'function') continue; + if (cachedCollection[subscriber.key] === previousCollection[subscriber.key]) continue; // === instead of deepEqual - continue; - } + const subscriberCallback = subscriber.callback as DefaultConnectCallback; + subscriberCallback(cachedCollection[subscriber.key], subscriber.key as TKey); + lastConnectionCallbackData.set(subscriber.subscriptionID, cachedCollection[subscriber.key]); } } @@ -679,7 +651,8 @@ function keyChanged( cachedCollections[subscriber.key] = cachedCollection; } - cachedCollection[key] = value; + // The cache is always updated before keyChanged runs, so the frozen snapshot + // already contains the new value — no need to copy or patch it. lastConnectionCallbackData.set(subscriber.subscriptionID, cachedCollection); subscriber.callback(cachedCollection, subscriber.key, {[key]: value}); continue; diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 49a057813..0fa65c2d8 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -80,17 +80,17 @@ function useOnyx>( // Recompute if input changed, dependencies changed, or first time const dependenciesChanged = !shallowEqual(lastDependencies, currentDependencies); if (!hasComputed || lastInput !== input || dependenciesChanged) { - // Only proceed if we have a valid selector - if (selector) { - const newOutput = selector(input); - - // Deep equality mode: only update if output actually changed - if (!hasComputed || !deepEqual(lastOutput, newOutput) || dependenciesChanged) { - lastInput = input; - lastOutput = newOutput; - lastDependencies = [...currentDependencies]; - hasComputed = true; - } + 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; } } @@ -240,18 +240,11 @@ function useOnyx>( newFetchStatus = 'loading'; } - // Optimized equality checking: - // - Memoized selectors already handle deep equality internally, so we can use fast reference equality - // - Non-selector cases use shallow equality for object reference checks + // Reference equality is sufficient because: + // - Memoized selectors return stable references (deep equality is handled internally) + // - Non-selector values have stable references from frozen cache snapshots // - Normalize null to undefined to ensure consistent comparison (both represent "no value") - let areValuesEqual: boolean; - if (memoizedSelector) { - const normalizedPrevious = previousValueRef.current ?? undefined; - const normalizedNew = newValueRef.current ?? undefined; - areValuesEqual = normalizedPrevious === normalizedNew; - } else { - areValuesEqual = shallowEqual(previousValueRef.current ?? undefined, newValueRef.current); - } + const areValuesEqual = (previousValueRef.current ?? undefined) === (newValueRef.current ?? undefined); // We update the cached value and the result in the following conditions: // We will update the cached value and the result in any of the following situations: From f1e1ba15b0c659b4bf9c3c13f976aea5890307e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 17 Apr 2026 10:51:26 +0100 Subject: [PATCH 2/4] fix: remove cachedCollection mutation in keyChanged that silently fails on frozen snapshots --- lib/OnyxUtils.ts | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 1320d4256..38dc90762 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -640,8 +640,6 @@ function keyChanged( } } - const cachedCollections: Record> = {}; - for (const stateMappingKey of stateMappingKeys) { const subscriber = callbackToStateMapping[stateMappingKey]; if (!subscriber || !OnyxKeys.isKeyMatch(subscriber.key, key) || !canUpdateSubscriber(subscriber)) { @@ -662,14 +660,9 @@ function keyChanged( if (isProcessingCollectionUpdate) { continue; } - let cachedCollection = cachedCollections[subscriber.key]; - - if (!cachedCollection) { - cachedCollection = getCachedCollection(subscriber.key); - cachedCollections[subscriber.key] = cachedCollection; - } - - cachedCollection[key] = value; + // The cache is always updated before keyChanged runs (via broadcastUpdate → cache.set), + // so the frozen snapshot already contains the new value — no patching needed. + const cachedCollection = getCachedCollection(subscriber.key); lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection, matchedKey: subscriber.key}); subscriber.callback(cachedCollection, subscriber.key, {[key]: value}); continue; From 5abe9f855d7ed6d2a72cbe0fea140407e861cee4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 21 Apr 2026 17:55:10 +0100 Subject: [PATCH 3/4] Update docs --- API-INTERNAL.md | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/API-INTERNAL.md b/API-INTERNAL.md index 10b70448f..933d536f5 100644 --- a/API-INTERNAL.md +++ b/API-INTERNAL.md @@ -293,23 +293,6 @@ If the requested key is a collection, it will return an object with all the coll When a collection of keys change, search for any callbacks matching the collection key and trigger those callbacks **Kind**: global function - -* [keysChanged()](#keysChanged) - * [~isSubscribedToCollectionKey](#keysChanged..isSubscribedToCollectionKey) - * [~isSubscribedToCollectionMemberKey](#keysChanged..isSubscribedToCollectionMemberKey) - - - -### keysChanged~isSubscribedToCollectionKey -e.g. Onyx.connect({key: ONYXKEYS.COLLECTION.REPORT, callback: ...}); - -**Kind**: inner constant of [keysChanged](#keysChanged) - - -### keysChanged~isSubscribedToCollectionMemberKey -e.g. Onyx.connect({key: `${ONYXKEYS.COLLECTION.REPORT}{reportID}`, callback: ...}); - -**Kind**: inner constant of [keysChanged](#keysChanged) ## keyChanged() From 863354ea77a4ea99eb6a2740c110a7e988aa0b1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 21 Apr 2026 20:07:15 +0100 Subject: [PATCH 4/4] Return shallowEqual in useOnyx --- lib/useOnyx.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index c1ec5a450..5ef6a3ff7 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -218,11 +218,11 @@ function useOnyx>( newFetchStatus = 'loading'; } - // Reference equality is sufficient because: - // - Memoized selectors return stable references (deep equality is handled internally) - // - Non-selector values have stable references from frozen cache snapshots - // - Normalize null to undefined to ensure consistent comparison (both represent "no value") - const areValuesEqual = (previousValueRef.current ?? undefined) === (newValueRef.current ?? undefined); + // 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); // 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: