Skip to content

[WIP] Remove waitForCollectionCallback from Onyx #799

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 1 commit into
base: main
Choose a base branch
from
Draft

[WIP] Remove waitForCollectionCallback from Onyx #799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 0 additions & 2 deletions lib/Onyx.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ function init({
* @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
Expand All @@ -119,7 +118,6 @@ function connect<TKey extends OnyxKey>(connectOptions: ConnectOptions<TKey>): Co
* @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
Expand Down
29 changes: 11 additions & 18 deletions lib/OnyxConnectionManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ 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 type {CollectionConnectCallback, DefaultConnectCallback, OnyxKey, OnyxValue} from './types';
import onyxSnapshotCache from './OnyxSnapshotCache';

type ConnectCallback = DefaultConnectCallback<OnyxKey> | CollectionConnectCallback<OnyxKey>;
Expand Down Expand Up @@ -48,11 +48,6 @@ type ConnectionMetadata = {
* The value that triggered the last update
*/
sourceValue?: OnyxValue<OnyxKey>;

/**
* Whether the subscriber is waiting for the collection callback to be fired.
*/
waitForCollectionCallback?: boolean;
};

/**
Expand Down Expand Up @@ -115,21 +110,20 @@ class OnyxConnectionManager {
* according to their purpose and effect they produce in the Onyx connection.
*/
private generateConnectionID<TKey extends OnyxKey>(connectOptions: ConnectOptions<TKey>): string {
const {key, reuseConnection, waitForCollectionCallback} = connectOptions;
const {key, reuseConnection} = connectOptions;

// The current session ID is appended to the connection ID so we can have different connections
// after an `Onyx.clear()` operation.
let suffix = `,sessionID=${this.sessionID}`;

// We will generate a unique ID 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))) {
// We will generate a unique ID when `reuseConnection` is `false`, which means the subscriber
// explicitly wants the connection to not be reused. Collection-root subscriptions are now always
// snapshot mode, so they can be reused like any other connection.
if (reuseConnection === false) {
suffix += `,uniqueID=${Str.guid()}`;
}

return `onyxKey=${key},waitForCollectionCallback=${waitForCollectionCallback ?? false}${suffix}`;
return `onyxKey=${key}${suffix}`;
}

/**
Expand All @@ -142,7 +136,7 @@ class OnyxConnectionManager {
}

for (const callback of connection.callbacks.values()) {
if (connection.waitForCollectionCallback) {
if (OnyxKeys.isCollectionKey(connection.onyxKey)) {
(callback as CollectionConnectCallback<OnyxKey>)(connection.cachedCallbackValue as Record<string, unknown>, connection.cachedCallbackKey as OnyxKey, connection.sourceValue);
} else {
(callback as DefaultConnectCallback<OnyxKey>)(connection.cachedCallbackValue, connection.cachedCallbackKey as OnyxKey);
Expand Down Expand Up @@ -180,15 +174,14 @@ class OnyxConnectionManager {

subscriptionID = OnyxUtils.subscribeToKey({
...connectOptions,
callback: callback as DefaultConnectCallback<TKey>,
});
callback,
} as ConnectOptions<TKey>);

connectionMetadata = {
subscriptionID,
onyxKey: connectOptions.key,
isConnectionMade: false,
callbacks: new Map(),
waitForCollectionCallback: connectOptions.waitForCollectionCallback,
};

this.connectionsMap.set(connectionID, connectionMetadata);
Expand All @@ -204,7 +197,7 @@ class OnyxConnectionManager {
// 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<OnyxKey>).callback?.(connectionMetadata.cachedCallbackValue, connectionMetadata.cachedCallbackKey as OnyxKey);
(connectOptions.callback as DefaultConnectCallback<OnyxKey> | undefined)?.(connectionMetadata.cachedCallbackValue, connectionMetadata.cachedCallbackKey as OnyxKey);
});
}

Expand Down
67 changes: 19 additions & 48 deletions lib/OnyxUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ import OnyxKeys from './OnyxKeys';
import * as Str from './Str';
import Storage from './storage';
import type {
CollectionConnectCallback,
CollectionKeyBase,
ConnectOptions,
DeepRecord,
DefaultConnectCallback,
DefaultConnectOptions,
KeyValueMapping,
CallbackToStateMapping,
MultiMergeReplaceNullPatches,
Expand Down Expand Up @@ -597,28 +597,9 @@ function keysChanged<TKey extends CollectionKeyBase>(
}

try {
// Collection-root subscribers always receive the whole collection snapshot.
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<TKey>;
currentSubscriberCallback(cachedCollection[dataKey], dataKey as TKey);
}
subscriber.callback(cachedCollection, subscriber.key, partialCollection);
} catch (error) {
Logger.logAlert(`[OnyxUtils.keysChanged] Subscriber callback threw an error for key '${collectionKey}': ${error}`);
}
Expand Down Expand Up @@ -696,9 +677,10 @@ function keyChanged<TKey extends OnyxKey>(
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 (OnyxKeys.isCollectionKey(subscriber.key)) {
// Collection-root subscribers always receive the whole collection snapshot.
// Skip individual key changes during collection updates to prevent duplicate
// callbacks - the collection update will handle this properly.
if (isProcessingCollectionUpdate) {
Comment on lines +680 to 684

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Don't skip synced collection-member updates

On web with multiple tabs, InstanceSync raises storage events per member key and Onyx.ts calls keyChanged(..., isKeyCollectionMember) with true for those collection-member updates. With this broadened collection-root branch, the receiving tab hits the isProcessingCollectionUpdate continue, and no keysChanged() runs afterward in that tab, so Onyx.connect({key: COLLECTION...}) subscribers never receive the required collection snapshot when another tab changes the collection. Please allow the storage-sync path to notify collection-root subscribers instead of treating it like a local batched collection update.

Useful? React with 👍 / 👎.

continue;
}
Expand All @@ -710,7 +692,7 @@ function keyChanged<TKey extends OnyxKey>(
cachedCollections[subscriber.key] = cachedCollection;
}
lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection, matchedKey: subscriber.key});
subscriber.callback(cachedCollection, subscriber.key, {[key]: value});
(subscriber.callback as CollectionConnectCallback<OnyxKey>)(cachedCollection, subscriber.key, {[key]: value});
continue;
}

Expand Down Expand Up @@ -741,10 +723,10 @@ function sendDataToConnection<TKey extends OnyxKey>(mapping: CallbackToStateMapp
}

// Always read the latest value from cache to avoid stale or duplicate data.
// For collection subscribers with waitForCollectionCallback, read the full collection.
// For collection-root subscribers, read the full collection (snapshot mode).
// For individual key subscribers, read just that key's value.
let value: OnyxValue<TKey> | undefined;
if (OnyxKeys.isCollectionKey(mapping.key) && mapping.waitForCollectionCallback) {
if (OnyxKeys.isCollectionKey(mapping.key)) {
const collection = getCachedCollection(mapping.key);
value = Object.keys(collection).length > 0 ? (collection as OnyxValue<TKey>) : undefined;
} else {
Expand All @@ -762,7 +744,7 @@ function sendDataToConnection<TKey extends OnyxKey>(mapping: CallbackToStateMapp
return;
}

(mapping as DefaultConnectOptions<TKey>).callback?.(value, matchedKey as TKey);
(mapping.callback as DefaultConnectCallback<TKey> | undefined)?.(value, matchedKey as TKey);
}

/**
Expand Down Expand Up @@ -1143,30 +1125,19 @@ function subscribeToKey<TKey extends OnyxKey>(connectOptions: ConnectOptions<TKe
cache.addNullishStorageKey(mapping.key);
}

const matchedKey = OnyxKeys.isCollectionKey(mapping.key) && mapping.waitForCollectionCallback ? mapping.key : undefined;
const matchedKey = OnyxKeys.isCollectionKey(mapping.key) ? mapping.key : undefined;

// Here we cannot use batching because the nullish value is expected to be set immediately for default props
// or they will be undefined.
sendDataToConnection(mapping, matchedKey);
return;
}

// When using a callback subscriber 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.
// When using a callback subscriber, a subscription to a collection key combines all matching
// member values into a single object and makes one call with the whole collection snapshot.
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);
}
});
getCollectionDataAndSendAsObject(matchingKeys, mapping);
return;
}

Expand Down Expand Up @@ -1416,7 +1387,7 @@ function multiSetWithRetry(data: OnyxMultiSetInput, retryAttempt?: number): Prom
// and subscriber state, matching the original per-key notification semantics.
cache.set(key, value);
// Skip subscriber notification on retry — already notified on attempt 0.
// waitForCollectionCallback subscribers re-fire on every keyChanged by contract.
// Collection-root subscribers re-fire on every keyChanged by contract.
if (!retryAttempt) {
keyChanged(key, value);
}
Expand Down Expand Up @@ -1506,7 +1477,7 @@ function setCollectionWithRetry<TKey extends CollectionKeyBase>({collectionKey,
for (const [key, value] of keyValuePairs) cache.set(key, value);

// Skip subscriber notification on retry — already notified on attempt 0.
// waitForCollectionCallback subscribers re-fire on every keysChanged by contract.
// Collection-root subscribers re-fire on every keysChanged by contract.
if (!retryAttempt) {
keysChanged(collectionKey, mutableCollection, previousCollection);
}
Expand Down Expand Up @@ -1652,7 +1623,7 @@ function mergeCollectionWithPatches<TKey extends CollectionKeyBase>(
const previousCollection = getCachedCollection(collectionKey, existingKeys);
cache.merge(finalMergedCollection);
// Skip subscriber notification on retry — already notified on attempt 0.
// waitForCollectionCallback subscribers re-fire on every keysChanged by contract.
// Collection-root subscribers re-fire on every keysChanged by contract.
if (!retryAttempt) {
keysChanged(collectionKey, finalMergedCollection, previousCollection);
}
Expand Down Expand Up @@ -1739,7 +1710,7 @@ function partialSetCollection<TKey extends CollectionKeyBase>({collectionKey, co
for (const [key, value] of keyValuePairs) cache.set(key, value);

// Skip subscriber notification on retry — already notified on attempt 0.
// waitForCollectionCallback subscribers re-fire on every keysChanged by contract.
// Collection-root subscribers re-fire on every keysChanged by contract.
if (!retryAttempt) {
keysChanged(collectionKey, mutableCollection, previousCollection);
}
Expand Down
61 changes: 26 additions & 35 deletions lib/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,45 +225,38 @@ type DefaultConnectCallback<TKey extends OnyxKey> = (value: OnyxEntry<KeyValueMa
/** Represents the callback function used in `Onyx.connect()` method with a collection key. */
type CollectionConnectCallback<TKey extends OnyxKey> = (value: NonUndefined<OnyxCollection<KeyValueMapping[TKey]>>, key: TKey, sourceValue?: OnyxValue<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<TKey extends OnyxKey> = 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<TKey>;

/** 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<TKey extends OnyxKey> = 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<TKey>;

/** 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
* for collection-root subscribers) is no longer supported; consumers that need
* per-member processing should subscribe to a single collection member key.
*/
// NOTE: Any changes to this type like adding or removing options must be accounted in OnyxConnectionManager's `generateConnectionID()` method!
type ConnectOptions<TKey extends OnyxKey> = DefaultConnectOptions<TKey> | CollectionConnectOptions<TKey>;
type ConnectOptions<TKey extends OnyxKey> = BaseConnectOptions & {
/** The Onyx key to subscribe to. */
key: TKey;

/**
* A function that will be called when the Onyx data we are subscribed changes.
*
* The value is a conditional *parameter* (collection snapshot vs. entry) inside a single
* function type — rather than a union of two distinct callback types — so that callers using a
* generic or union `TKey` still get an assignable, non-`any` callback. Collection snapshots stay
* `NonUndefined`.
*/
callback?: (
value: TKey extends CollectionKeyBase ? NonUndefined<OnyxCollection<KeyValueMapping[TKey]>> : OnyxEntry<KeyValueMapping[TKey]>,
key: TKey,
sourceValue?: OnyxValue<TKey>,
) => void;
};

type CallbackToStateMapping<TKey extends OnyxKey> = ConnectOptions<TKey> & {
subscriptionID: number;
Expand Down Expand Up @@ -448,14 +441,12 @@ export type {
BaseConnectOptions,
Collection,
CollectionConnectCallback,
CollectionConnectOptions,
CollectionKey,
CollectionKeyBase,
ConnectOptions,
CustomTypeOptions,
DeepRecord,
DefaultConnectCallback,
DefaultConnectOptions,
ExtractOnyxCollectionValue,
GenericFunction,
InitOptions,
Expand Down
Loading
Loading