[WIP] Fix cross-tab sync for collection-root subscribers

lib/Onyx.ts

@@ -21,6 +21,9 @@ import type {
     OnyxInput,
     OnyxMethodMap,
     SetOptions,
+    OnyxCollection,
+    NonUndefined,
+    OnyxEntry,
 } from './types';
 import OnyxUtils from './OnyxUtils';
 import OnyxKeys from './OnyxKeys';
@@ -50,21 +53,54 @@ function init({
     OnyxKeys.setRamOnlyKeys(new Set<OnyxKey>(ramOnlyKeys));
 
     if (shouldSyncMultipleInstances) {
-        Storage.keepInstancesSync?.((key, value) => {
-            // RAM-only keys should never sync from storage as they may have stale persisted data
-            // from before the key was migrated to RAM-only.
-            if (OnyxKeys.isRamOnlyKey(key)) {
-                return;
-            }
+        // Cross-tab sync (InstanceSync) hands us the full batch of key/value pairs that changed together in
+        // a single write. We process it synchronously, grouping collection members so each affected
+        // collection is notified once (mirroring the local mergeCollection batching) instead of
+        // re-delivering the whole collection per member.
+        Storage.keepInstancesSync?.((pairs) => {
+            const individual: Array<[OnyxKey, OnyxEntry<KeyValueMapping[OnyxKey]>]> = [];
+            const collectionBatches = new Map<string, {partial: NonUndefined<OnyxCollection<KeyValueMapping[OnyxKey]>>; previous: NonUndefined<OnyxCollection<KeyValueMapping[OnyxKey]>>}>();
+
+            for (const [key, value] of pairs) {
+                // RAM-only keys should never sync from storage as they may have stale persisted data
+                // from before the key was migrated to RAM-only.
+                if (OnyxKeys.isRamOnlyKey(key)) {
+                    continue;
+                }
+
+                const collectionKey = OnyxKeys.getCollectionKey(key);
+                const isCollectionMember = !!collectionKey && OnyxKeys.isCollectionMemberKey(collectionKey, key);
 
-            cache.set(key, value);
+                // Capture the previous cached value BEFORE cache.set() so keysChanged() can diff old vs new per member.
+                const previousValue = isCollectionMember ? cache.get(key) : undefined;
+                cache.set(key, value);
+
+                if (isCollectionMember && collectionKey) {
+                    let batch = collectionBatches.get(collectionKey);
+                    if (!batch) {
+                        batch = {partial: {}, previous: {}};
+                        collectionBatches.set(collectionKey, batch);
+                    }
+                    batch.partial[key] = value;
+                    // Keep the earliest previous value in case the same member appears twice in one batch.
+                    if (!(key in batch.previous)) {
+                        batch.previous[key] = previousValue;
+                    }
+                } else {
+                    individual.push([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);
+            // Non-collection keys: notify individually, matching keyChanged() semantics for exact keys.
+            for (const [key, value] of individual) {
+                OnyxUtils.keyChanged(key, value);
+            }
 
-            OnyxUtils.keyChanged(key, value as OnyxValue<typeof key>, undefined, isKeyCollectionMember);
+            // One keysChanged() per collection notifies the collection-root subscriber once and lets
+            // keysChanged() decide which individual member subscribers actually changed.
+            for (const [collectionKey, {partial, previous}] of collectionBatches) {
+                OnyxUtils.keysChanged(collectionKey, partial, previous);
+            }
         });
     }
 

lib/storage/InstanceSync/index.web.ts

@@ -3,36 +3,101 @@
  * when using LocalStorage APIs in the browser. These events are great because multiple tabs can listen for when
  * data changes and then stay up-to-date with everything happening in Onyx.
  */
+import * as Logger from '../../Logger';
 import type {OnyxKey} from '../../types';
 import NoopProvider from '../providers/NoopProvider';
-import type {StorageKeyList, OnStorageKeyChanged} from '../providers/types';
+import type {StorageKeyList, OnStorageKeysChanged} from '../providers/types';
 import type StorageProvider from '../providers/types';
 
 const SYNC_ONYX = 'SYNC_ONYX';
 
+// localStorage stores values as UTF-16 (~2 bytes/char). The per-origin quota isn't fixed by the spec —
+// it's user-agent dependent and commonly ~5MB — so we keep each SYNC_ONYX payload conservatively small
+// (and pair it with a try/catch in emitSyncEvent). This way a large key batch (e.g. Onyx.clear() on a
+// heavy account, or a bulk import) is split across several events instead of throwing QuotaExceededError.
+// See https://developer.mozilla.org/en-US/docs/Web/API/Web_Storage_API/Using_the_Web_Storage_API
+const MAX_SYNC_PAYLOAD_LENGTH = 1_000_000;
+
 /**
- * Raise an event through `localStorage` to let other tabs know a value changed
- * @param {String} onyxKey
+ * Parses the SYNC_ONYX storage event value.
+ * The payload is a JSON array of the changed keys (a batch). I fall backs to treating the raw
+ * value as a single key for backwards compatibility with the previous one-key-per-event format.
  */
-function raiseStorageSyncEvent(onyxKey: OnyxKey) {
-    global.localStorage.setItem(SYNC_ONYX, onyxKey);
-    global.localStorage.removeItem(SYNC_ONYX);
+function parseSyncOnyxStorageEventValue(value: string): StorageKeyList {
+    let onyxKeys: StorageKeyList;
+    try {
+        const parsed = JSON.parse(value) as StorageKeyList | string;
+        onyxKeys = Array.isArray(parsed) ? parsed : [value];
+    } catch {
+        onyxKeys = [value];
+    }
+
+    return onyxKeys;
+}
+
+/**
+ * Emit a single SYNC_ONYX storage event. Wrapped so a failed cross-tab signal
+ * degrades gracefully — other tabs simply miss this update until their next organic sync/reload — instead
+ * of throwing an uncaught rejection in the writing tab.
+ */
+function emitSyncEvent(value: string) {
+    try {
+        global.localStorage.setItem(SYNC_ONYX, value);
+        global.localStorage.removeItem(SYNC_ONYX);
+    } catch (error) {
+        Logger.logAlert(`[InstanceSync] Failed to raise storage sync event: ${error}`);
+    }
 }
 
+/**
+ * Raise cross-tab event(s) for a batch of changed keys. Sending keys together (instead of one event per
+ * key) preserves the write's batch boundary across tabs, so the receiving tab notifies collection
+ * subscribers once for the whole batch — matching the local mergeCollection behavior — instead of
+ * re-delivering the whole collection once per member (O(N^2), which can crash the tab). Large batches are
+ * chunked so no single payload approaches the localStorage quota.
+ */
 function raiseStorageSyncManyKeysEvent(onyxKeys: StorageKeyList) {
+    if (onyxKeys.length === 0) {
+        return;
+    }
+
+    let chunk: StorageKeyList = [];
+    let chunkLength = 2; // accounts for the surrounding `[]`
     for (const onyxKey of onyxKeys) {
-        raiseStorageSyncEvent(onyxKey);
+        const keyLength = onyxKey.length + 3; // quotes + comma separator
+        if (chunk.length > 0 && chunkLength + keyLength > MAX_SYNC_PAYLOAD_LENGTH) {
+            emitSyncEvent(JSON.stringify(chunk));
+            chunk = [];
+            chunkLength = 2;
+        }
+        chunk.push(onyxKey);
+        chunkLength += keyLength;
     }
+    if (chunk.length > 0) {
+        emitSyncEvent(JSON.stringify(chunk));
+    }
+}
+
+/**
+ * Raise an event through `localStorage` to let other tabs know a single key changed.
+ *
+ * This intentionally emits the raw key (the legacy, pre-batching format) rather than a JSON array, so a
+ * tab still running the previous bundle during a deploy keeps receiving single-key updates (a new message,
+ * a pin, a rename, etc.). Only multi-key writes use the batched JSON-array format; the receiver here
+ * understands both. The mixed-version gap is therefore limited to bulk collection writes, which resolve on reload.
+ */
+function raiseStorageSyncEvent(onyxKey: OnyxKey) {
+    emitSyncEvent(onyxKey);
 }
 
 let storage = NoopProvider;
 
 const InstanceSync = {
     shouldBeUsed: true,
     /**
-     * @param {Function} onStorageKeyChanged Storage synchronization mechanism keeping all opened tabs in sync
+     * @param {Function} onStorageKeysChanged Storage synchronization mechanism keeping all opened tabs in sync
      */
-    init: (onStorageKeyChanged: OnStorageKeyChanged, store: StorageProvider<unknown>) => {
+    init: (onStorageKeysChanged: OnStorageKeysChanged, store: StorageProvider<unknown>) => {
         storage = store;
 
         // This listener will only be triggered by events coming from other tabs
@@ -42,9 +107,9 @@ const InstanceSync = {
                 return;
             }
 
-            const onyxKey = event.newValue;
+            const onyxKeys = parseSyncOnyxStorageEventValue(event.newValue);
 
-            storage.getItem(onyxKey).then((value) => onStorageKeyChanged(onyxKey, value));
+            storage.multiGet(onyxKeys).then((pairs) => onStorageKeysChanged(pairs));
         });
     },
     setItem: raiseStorageSyncEvent,

lib/storage/index.ts

@@ -182,14 +182,14 @@ const storage: Storage = {
     getDatabaseSize: () => tryOrDegradePerformance(() => provider.getDatabaseSize()),
 
     /**
-     * @param onStorageKeyChanged - Storage synchronization mechanism keeping all opened tabs in sync (web only)
+     * @param onStorageKeysChanged - Storage synchronization mechanism keeping all opened tabs in sync (web only)
      */
-    keepInstancesSync(onStorageKeyChanged) {
+    keepInstancesSync(onStorageKeysChanged) {
         // If InstanceSync shouldn't be used, it means we're on a native platform and we don't need to keep instances in sync
         if (!InstanceSync.shouldBeUsed) return;
 
         shouldKeepInstancesSync = true;
-        InstanceSync.init(onStorageKeyChanged, this);
+        InstanceSync.init(onStorageKeysChanged, this);
     },
 };
 

lib/storage/providers/types.ts

@@ -10,7 +10,8 @@ type DatabaseSize = {
     usageDetails?: Record<string, number>;
 };
 
-type OnStorageKeyChanged = <TKey extends OnyxKey>(key: TKey, value: OnyxValue<TKey>) => void;
+/** Called with the full batch of key/value pairs that changed together in a single cross-tab sync event. */
+type OnStorageKeysChanged = (pairs: StorageKeyValuePair[]) => void;
 
 type StorageProvider<TStore> = {
     store: TStore;
@@ -90,8 +91,8 @@ type StorageProvider<TStore> = {
     /**
      * @param onStorageKeyChanged Storage synchronization mechanism keeping all opened tabs in sync
      */
-    keepInstancesSync?: (onStorageKeyChanged: OnStorageKeyChanged) => void;
+    keepInstancesSync?: (onStorageKeysChanged: OnStorageKeysChanged) => void;
 };
 
 export default StorageProvider;
-export type {StorageKeyList, StorageKeyValuePair, OnStorageKeyChanged};
+export type {StorageKeyList, StorageKeyValuePair, OnStorageKeysChanged};

tests/unit/onyxTest.ts

@@ -3317,7 +3317,7 @@ describe('RAM-only keys should not read from storage', () => {
         await act(async () => waitForPromisesToResolve());
 
         // Simulate another tab syncing a stale RAM-only key value
-        syncCallback(ONYX_KEYS.RAM_ONLY_TEST_KEY, 'synced_stale_value');
+        syncCallback([[ONYX_KEYS.RAM_ONLY_TEST_KEY, 'synced_stale_value']]);
         await act(async () => waitForPromisesToResolve());
 
         // The RAM-only key should NOT have been updated from the sync
@@ -3334,7 +3334,7 @@ describe('RAM-only keys should not read from storage', () => {
         });
         await act(async () => waitForPromisesToResolve());
 
-        syncCallback(ONYX_KEYS.OTHER_TEST, 'synced_normal_value');
+        syncCallback([[ONYX_KEYS.OTHER_TEST, 'synced_normal_value']]);
         await act(async () => waitForPromisesToResolve());
 
         expect(normalValue).toEqual('synced_normal_value');
@@ -3343,6 +3343,97 @@ describe('RAM-only keys should not read from storage', () => {
         Onyx.disconnect(connection2);
     });
 
+    it('should notify collection-root and collection member subscribers when a collection member syncs from another instance', async () => {
+        Onyx.init({
+            keys: ONYX_KEYS,
+            shouldSyncMultipleInstances: true,
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls.at(-1)?.[0];
+        expect(syncCallback).toBeDefined();
+
+        await Onyx.setCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {
+            [`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]: {name: 'entry 1'},
+            [`${ONYX_KEYS.COLLECTION.TEST_KEY}2`]: {name: 'entry 2'},
+            [`${ONYX_KEYS.COLLECTION.TEST_KEY}3`]: {name: 'entry 3'},
+        } as GenericCollection);
+
+        let collection: GenericCollection = {};
+        const collectionConn = Onyx.connect({
+            key: ONYX_KEYS.COLLECTION.TEST_KEY,
+            waitForCollectionCallback: true,
+            callback: (value) => {
+                collection = value as GenericCollection;
+            },
+        });
+
+        let collectionMember2: unknown;
+        const collectionMember2Conn = Onyx.connect({
+            key: `${ONYX_KEYS.COLLECTION.TEST_KEY}2`,
+            callback: (value) => {
+                collectionMember2 = value;
+            },
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        // Another tab writes a collection member; the storage-sync batch must notify the collection-root subscriber.
+        syncCallback([[`${ONYX_KEYS.COLLECTION.TEST_KEY}2`, {name: 'entry 2 changed'}]]);
+        await act(async () => waitForPromisesToResolve());
+
+        // The collection-root subscriber must receive the whole collection including the synced member.
+        expect(Object.keys(collection).length).toBe(3);
+        expect(collection[`${ONYX_KEYS.COLLECTION.TEST_KEY}2`]).toEqual({name: 'entry 2 changed'});
+
+        // The collection member subscriber must receive the synced data.
+        expect(collectionMember2).toEqual({name: 'entry 2 changed'});
+
+        Onyx.disconnect(collectionConn);
+        Onyx.disconnect(collectionMember2Conn);
+    });
+
+    it('should notify a collection-root subscriber once when multiple members sync from another instance', async () => {
+        Onyx.init({
+            keys: ONYX_KEYS,
+            shouldSyncMultipleInstances: true,
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls.at(-1)?.[0];
+        expect(syncCallback).toBeDefined();
+
+        await Onyx.setCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {
+            [`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]: {name: 'entry 1'},
+            [`${ONYX_KEYS.COLLECTION.TEST_KEY}2`]: {name: 'entry 2'},
+            [`${ONYX_KEYS.COLLECTION.TEST_KEY}3`]: {name: 'entry 3'},
+        } as GenericCollection);
+
+        const collectionCallback = jest.fn();
+        const connection = Onyx.connect({
+            key: ONYX_KEYS.COLLECTION.TEST_KEY,
+            waitForCollectionCallback: true,
+            callback: collectionCallback,
+        });
+        await act(async () => waitForPromisesToResolve());
+        collectionCallback.mockClear();
+
+        // Another tab writes two members; storage sync delivers them as one batch.
+        syncCallback([
+            [`${ONYX_KEYS.COLLECTION.TEST_KEY}1`, {name: 'entry 1 changed'}],
+            [`${ONYX_KEYS.COLLECTION.TEST_KEY}3`, {name: 'entry 3 changed'}],
+        ]);
+        await waitForPromisesToResolve();
+
+        // The batch produces a single collection-root notification carrying all members.
+        expect(collectionCallback).toHaveBeenCalledTimes(1);
+        const collection = collectionCallback.mock.calls[0][0] as Record<string, unknown>;
+        expect(collection[`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]).toEqual({name: 'entry 1 changed'});
+        expect(collection[`${ONYX_KEYS.COLLECTION.TEST_KEY}2`]).toEqual({name: 'entry 2'});
+        expect(collection[`${ONYX_KEYS.COLLECTION.TEST_KEY}3`]).toEqual({name: 'entry 3 changed'});
+
+        Onyx.disconnect(connection);
+    });
+
     it('should serve RAM-only keys from cache and normal keys from storage in multiGet', async () => {
         const ramOnlyMember = `${ONYX_KEYS.COLLECTION.RAM_ONLY_COLLECTION}1`;
         const normalMember = `${ONYX_KEYS.COLLECTION.TEST_KEY}1`;