Skip to content

[WIP][Structural Sharing] PR 4: Reference equality in notification and subscription paths #774

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
fabioh8010 wants to merge 6 commits into
base: main
Choose a base branch
from
+69 −114
Draft

[WIP][Structural Sharing] PR 4: Reference equality in notification and subscription paths #774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
93a1478
perf: reference equality in notification paths and useOnyx
fabioh8010 Mar 19, 2026
dcceeeb
Merge branch 'main' into feature/structural-sharing-cache-pr-4
fabioh8010 Apr 16, 2026
f1e1ba1
fix: remove cachedCollection mutation in keyChanged that silently fai…
fabioh8010 Apr 17, 2026
8d76faf
Merge branch 'main' into feature/structural-sharing-cache-pr-4
fabioh8010 Apr 21, 2026
5abe9f8
Update docs
fabioh8010 Apr 21, 2026
863354e
Return shallowEqual in useOnyx
fabioh8010 Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions API-INTERNAL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

<a name="keysChanged..isSubscribedToCollectionKey"></a>

### keysChanged~isSubscribedToCollectionKey
e.g. Onyx.connect({key: ONYXKEYS.COLLECTION.REPORT, callback: ...});

**Kind**: inner constant of [<code>keysChanged</code>](#keysChanged)
<a name="keysChanged..isSubscribedToCollectionMemberKey"></a>

### keysChanged~isSubscribedToCollectionMemberKey
e.g. Onyx.connect({key: `${ONYXKEYS.COLLECTION.REPORT}{reportID}`, callback: ...});

**Kind**: inner constant of [<code>keysChanged</code>](#keysChanged)
<a name="keyChanged"></a>

## keyChanged()
Expand Down
127 changes: 53 additions & 74 deletions lib/OnyxUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -510,8 +510,8 @@ function getCachedCollection<TKey extends CollectionKeyBase>(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
Expand Down Expand Up @@ -546,81 +546,67 @@ function keysChanged<TKey extends CollectionKeyBase>(
partialCollection: OnyxCollection<KeyValueMapping[TKey]>,
partialPreviousCollection: OnyxCollection<KeyValueMapping[TKey]> | 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 ?? {};

// 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);

for (const stateMappingKey of stateMappingKeys) {
const subscriber = callbackToStateMapping[stateMappingKey];
if (!subscriber) {
continue;
const changedMemberKeys = Object.keys(partialCollection ?? {});

// 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);
}
}
}

// Skip iteration if we do not have a collection key or a collection member key on this subscriber
if (!Str.startsWith(subscriber.key, collectionKey)) {
// Notify collection-level subscribers
for (const subID of collectionSubscriberIDs) {
const subscriber = callbackToStateMapping[subID];
if (!subscriber || typeof subscriber.callback !== 'function') {
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') {
try {
// 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, {value: cachedCollection, matchedKey: subscriber.key});

if (subscriber.waitForCollectionCallback) {
subscriber.callback(cachedCollection, subscriber.key, partialCollection);
continue;
}
try {
lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection, matchedKey: subscriber.key});

// 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;
}
if (subscriber.waitForCollectionCallback) {
subscriber.callback(cachedCollection, subscriber.key, partialCollection);
continue;
}

subscriber.callback(cachedCollection[dataKey], dataKey);
}
// Not using waitForCollectionCallback — notify per changed key
for (const dataKey of changedMemberKeys) {
if (cachedCollection[dataKey] === previousCollection[dataKey]) {
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep deep-equality check for collection member diffs

This === comparison assumes unchanged collection members keep the same reference, but setCollectionWithRetry() and partialSetCollection() populate updates via cache.set(...), which replaces object references even when values are deep-equal. In those paths, unchanged members are now treated as changed and subscriber callbacks fire unnecessarily, which can trigger extra side effects and render work compared to prior behavior.

Useful? React with 👍 / 👎.

continue;
}
subscriber.callback(cachedCollection[dataKey], dataKey);
}
} catch (error) {
Logger.logAlert(`[OnyxUtils.keysChanged] Subscriber callback threw an error for key '${collectionKey}': ${error}`);
}
}

// 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;
}
// 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;
}

const subscriberCallback = subscriber.callback as DefaultConnectCallback<TKey>;
subscriberCallback(cachedCollection[subscriber.key], subscriber.key as TKey);
lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection[subscriber.key], matchedKey: subscriber.key});
continue;
}
if (cachedCollection[subscriber.key] === previousCollection[subscriber.key]) {
continue;
}

continue;
} catch (error) {
Logger.logAlert(`[OnyxUtils.keysChanged] Subscriber callback threw an error for key '${collectionKey}': ${error}`);
}
try {
const subscriberCallback = subscriber.callback as DefaultConnectCallback<TKey>;
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}`);
}
}
}
Expand Down Expand Up @@ -660,8 +646,6 @@ function keyChanged<TKey extends OnyxKey>(
}
}

const cachedCollections: Record<string, ReturnType<typeof getCachedCollection>> = {};

for (const stateMappingKey of stateMappingKeys) {
const subscriber = callbackToStateMapping[stateMappingKey];
if (!subscriber || !OnyxKeys.isKeyMatch(subscriber.key, key) || !canUpdateSubscriber(subscriber)) {
Expand All @@ -682,14 +666,9 @@ function keyChanged<TKey extends OnyxKey>(
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;
Expand Down
39 changes: 16 additions & 23 deletions lib/useOnyx.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,17 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
// 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;
}
}

Expand Down Expand Up @@ -218,18 +218,11 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
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
// - 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);
}
// 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:
Expand Down
Loading