From e75f5f4d1be1b876662b6922df01a755a3a73f00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 25 Jun 2026 17:07:00 +0100 Subject: [PATCH 1/7] Fix cross-tab sync for collection-root subscribers --- lib/Onyx.ts | 60 +++++++++++++---- lib/storage/InstanceSync/index.web.ts | 48 ++++++++++---- lib/storage/index.ts | 6 +- lib/storage/providers/types.ts | 7 +- tests/unit/onyxTest.ts | 95 ++++++++++++++++++++++++++- 5 files changed, 184 insertions(+), 32 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index a8f8f2d4f..3a81beea7 100644 --- a/lib/Onyx.ts +++ b/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(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]> = []; + const collectionBatches = new Map>; previous: NonUndefined>}>(); + + 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, 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); + } }); } diff --git a/lib/storage/InstanceSync/index.web.ts b/lib/storage/InstanceSync/index.web.ts index cb1a3c5bb..6ebb94d3e 100644 --- a/lib/storage/InstanceSync/index.web.ts +++ b/lib/storage/InstanceSync/index.web.ts @@ -5,24 +5,48 @@ */ 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'; /** - * 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; } +/** + * Raise a single cross-tab event for a batch of changed keys. Sending them together (instead of one + * event per key) preserves the write's batch boundary across tabs, so the receiving tab can notify + * collection subscribers once for the whole batch — matching the local mergeCollection behavior — + * instead of re-delivering the whole collection once per member (which is O(N^2) and can crash the tab). + */ function raiseStorageSyncManyKeysEvent(onyxKeys: StorageKeyList) { - for (const onyxKey of onyxKeys) { - raiseStorageSyncEvent(onyxKey); + if (onyxKeys.length === 0) { + return; } + global.localStorage.setItem(SYNC_ONYX, JSON.stringify(onyxKeys)); + global.localStorage.removeItem(SYNC_ONYX); +} + +/** + * Raise an event through `localStorage` to let other tabs know a value changed. + * @param {String} onyxKey + */ +function raiseStorageSyncEvent(onyxKey: OnyxKey) { + raiseStorageSyncManyKeysEvent([onyxKey]); } let storage = NoopProvider; @@ -30,9 +54,9 @@ 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) => { + init: (onStorageKeysChanged: OnStorageKeysChanged, store: StorageProvider) => { storage = store; // This listener will only be triggered by events coming from other tabs @@ -42,9 +66,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, diff --git a/lib/storage/index.ts b/lib/storage/index.ts index 6dcd8e0bd..4b999216b 100644 --- a/lib/storage/index.ts +++ b/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); }, }; diff --git a/lib/storage/providers/types.ts b/lib/storage/providers/types.ts index 046b531b0..92a9a7c5a 100644 --- a/lib/storage/providers/types.ts +++ b/lib/storage/providers/types.ts @@ -10,7 +10,8 @@ type DatabaseSize = { usageDetails?: Record; }; -type OnStorageKeyChanged = (key: TKey, value: OnyxValue) => 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 = { store: TStore; @@ -90,8 +91,8 @@ type StorageProvider = { /** * @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}; diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 5ff5d6f96..fb36c05f1 100644 --- a/tests/unit/onyxTest.ts +++ b/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; + 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`; From 05130ea48dadc10ad18914ac2775bd8d598c430f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 26 Jun 2026 15:36:01 +0100 Subject: [PATCH 2/7] Handle legacy pre-batching storage sync events --- lib/storage/InstanceSync/index.web.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/storage/InstanceSync/index.web.ts b/lib/storage/InstanceSync/index.web.ts index 6ebb94d3e..363ddef08 100644 --- a/lib/storage/InstanceSync/index.web.ts +++ b/lib/storage/InstanceSync/index.web.ts @@ -42,11 +42,16 @@ function raiseStorageSyncManyKeysEvent(onyxKeys: StorageKeyList) { } /** - * Raise an event through `localStorage` to let other tabs know a value changed. - * @param {String} onyxKey + * 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) { - raiseStorageSyncManyKeysEvent([onyxKey]); + global.localStorage.setItem(SYNC_ONYX, onyxKey); + global.localStorage.removeItem(SYNC_ONYX); } let storage = NoopProvider; From 57a1564c6bf93768a9fe7a861a4f4d4b024fea69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 26 Jun 2026 16:45:06 +0100 Subject: [PATCH 3/7] Split large sync events into chunks --- lib/storage/InstanceSync/index.web.ts | 52 ++++++++++++++++++++++----- 1 file changed, 44 insertions(+), 8 deletions(-) diff --git a/lib/storage/InstanceSync/index.web.ts b/lib/storage/InstanceSync/index.web.ts index 363ddef08..3c02e3e48 100644 --- a/lib/storage/InstanceSync/index.web.ts +++ b/lib/storage/InstanceSync/index.web.ts @@ -3,6 +3,7 @@ * 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, OnStorageKeysChanged} from '../providers/types'; @@ -10,6 +11,13 @@ 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; + /** * 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 @@ -28,17 +36,46 @@ function parseSyncOnyxStorageEventValue(value: string): StorageKeyList { } /** - * Raise a single cross-tab event for a batch of changed keys. Sending them together (instead of one - * event per key) preserves the write's batch boundary across tabs, so the receiving tab can notify - * collection subscribers once for the whole batch — matching the local mergeCollection behavior — - * instead of re-delivering the whole collection once per member (which is O(N^2) and can crash the tab). + * 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; } - global.localStorage.setItem(SYNC_ONYX, JSON.stringify(onyxKeys)); - global.localStorage.removeItem(SYNC_ONYX); + + let chunk: StorageKeyList = []; + let chunkLength = 2; // accounts for the surrounding `[]` + for (const onyxKey of onyxKeys) { + 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)); + } } /** @@ -50,8 +87,7 @@ function raiseStorageSyncManyKeysEvent(onyxKeys: StorageKeyList) { * understands both. The mixed-version gap is therefore limited to bulk collection writes, which resolve on reload. */ function raiseStorageSyncEvent(onyxKey: OnyxKey) { - global.localStorage.setItem(SYNC_ONYX, onyxKey); - global.localStorage.removeItem(SYNC_ONYX); + emitSyncEvent(onyxKey); } let storage = NoopProvider; From f4587d6582aa172395732a1ce729d466ffab92c7 Mon Sep 17 00:00:00 2001 From: eliran goshen Date: Tue, 28 Jul 2026 12:52:54 +0200 Subject: [PATCH 4/7] Drop removed waitForCollectionCallback option from sync tests Collection-root callbacks are now automatic for collection keys, so the option no longer exists on ConnectOptions. Co-Authored-By: Claude Fable 5 --- tests/unit/onyxTest.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index db14f6f9d..b32897ba7 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -3317,7 +3317,6 @@ describe('RAM-only keys should not read from storage', () => { let collection: GenericCollection = {}; const collectionConn = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, - waitForCollectionCallback: true, callback: (value) => { collection = value as GenericCollection; }, @@ -3366,7 +3365,6 @@ describe('RAM-only keys should not read from storage', () => { const collectionCallback = jest.fn(); const connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, - waitForCollectionCallback: true, callback: collectionCallback, }); await act(async () => waitForPromisesToResolve()); From 6c0cf38530faa4384cbd00c0fada950406e506bf Mon Sep 17 00:00:00 2001 From: eliran goshen Date: Thu, 30 Jul 2026 10:23:01 +0200 Subject: [PATCH 5/7] Add tests for member deletion sync, event chunking, and legacy raw-key parsing Co-Authored-By: Claude Fable 5 --- tests/unit/onyxTest.ts | 46 ++++++++ tests/unit/storage/instanceSyncWebTest.ts | 121 ++++++++++++++++++++++ 2 files changed, 167 insertions(+) create mode 100644 tests/unit/storage/instanceSyncWebTest.ts diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index b32897ba7..0021fcd8e 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -3387,6 +3387,52 @@ describe('RAM-only keys should not read from storage', () => { Onyx.disconnect(connection); }); + it('should notify subscribers with undefined when a collection member is removed in 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'}, + } as GenericCollection); + + let collection: GenericCollection = {}; + const collectionConn = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_KEY, + callback: (value) => { + collection = value as GenericCollection; + }, + }); + + let collectionMember2: unknown = 'initial'; + const collectionMember2Conn = Onyx.connect({ + key: `${ONYX_KEYS.COLLECTION.TEST_KEY}2`, + callback: (value) => { + collectionMember2 = value; + }, + }); + await act(async () => waitForPromisesToResolve()); + + // Another tab removes member 2; the removed key reads back as undefined from storage. + syncCallback([[`${ONYX_KEYS.COLLECTION.TEST_KEY}2`, undefined]]); + await act(async () => waitForPromisesToResolve()); + + // The member subscriber must be told the member is gone. + expect(collectionMember2).toBeUndefined(); + + // The collection-root subscriber must receive the collection without the removed member. + expect(collection).toEqual({[`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]: {name: 'entry 1'}}); + + Onyx.disconnect(collectionConn); + Onyx.disconnect(collectionMember2Conn); + }); + 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`; diff --git a/tests/unit/storage/instanceSyncWebTest.ts b/tests/unit/storage/instanceSyncWebTest.ts new file mode 100644 index 000000000..c27d5a85f --- /dev/null +++ b/tests/unit/storage/instanceSyncWebTest.ts @@ -0,0 +1,121 @@ +import InstanceSync from '../../../lib/storage/InstanceSync/index.web'; +import type StorageProvider from '../../../lib/storage/providers/types'; +import waitForPromisesToResolve from '../../utils/waitForPromisesToResolve'; + +const SYNC_ONYX = 'SYNC_ONYX'; + +// Mirrors MAX_SYNC_PAYLOAD_LENGTH in lib/storage/InstanceSync/index.web.ts (not exported). +const MAX_SYNC_PAYLOAD_LENGTH = 1_000_000; + +/** Returns the SYNC_ONYX payloads written to localStorage, in write order. */ +function getSyncPayloads(setItemSpy: jest.SpyInstance): string[] { + return setItemSpy.mock.calls.filter(([key]) => key === SYNC_ONYX).map(([, value]) => value as string); +} + +describe('InstanceSync (web)', () => { + let setItemSpy: jest.SpyInstance; + + beforeEach(() => { + setItemSpy = jest.spyOn(Storage.prototype, 'setItem'); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe('event payload chunking', () => { + it('emits a single event when the batch fits within the payload limit', () => { + const keys = ['test_1', 'test_2', 'test_3']; + InstanceSync.multiSet(keys); + + const payloads = getSyncPayloads(setItemSpy); + expect(payloads).toHaveLength(1); + expect(JSON.parse(payloads[0])).toEqual(keys); + }); + + it('splits a batch larger than the payload limit into multiple events without losing keys', () => { + // Three keys of ~400k chars each: the first two fit in one payload, the third starts a new one. + const keys = [1, 2, 3].map((i) => `test_${i}_${'x'.repeat(400_000)}`); + InstanceSync.multiSet(keys); + + const payloads = getSyncPayloads(setItemSpy); + expect(payloads).toHaveLength(2); + for (const payload of payloads) { + expect(payload.length).toBeLessThanOrEqual(MAX_SYNC_PAYLOAD_LENGTH); + } + + // All keys must arrive, in order, with no duplicates. + const receivedKeys = payloads.flatMap((payload) => JSON.parse(payload) as string[]); + expect(receivedKeys).toEqual(keys); + }); + + it('emits an oversized single key as its own event instead of dropping it', () => { + const hugeKey = `test_huge_${'x'.repeat(MAX_SYNC_PAYLOAD_LENGTH + 1000)}`; + InstanceSync.multiSet([hugeKey]); + + const payloads = getSyncPayloads(setItemSpy); + expect(payloads).toHaveLength(1); + expect(JSON.parse(payloads[0])).toEqual([hugeKey]); + }); + + it('emits no event for an empty batch', () => { + InstanceSync.multiSet([]); + + expect(getSyncPayloads(setItemSpy)).toHaveLength(0); + }); + }); + + describe('storage event parsing', () => { + let onStorageKeysChanged: jest.Mock; + let multiGet: jest.Mock; + let storageEventHandler: (event: {key: string | null; newValue: string | null}) => void; + + beforeEach(() => { + onStorageKeysChanged = jest.fn(); + multiGet = jest.fn((keys: string[]) => Promise.resolve(keys.map((key) => [key, `value_of_${key}`]))); + + const addEventListenerSpy = jest.spyOn(global, 'addEventListener').mockImplementation(() => undefined); + InstanceSync.init(onStorageKeysChanged, {multiGet} as unknown as StorageProvider); + + const storageCall = addEventListenerSpy.mock.calls.find(([type]) => type === 'storage'); + storageEventHandler = storageCall?.[1] as unknown as typeof storageEventHandler; + expect(storageEventHandler).toBeDefined(); + }); + + it('parses a JSON-array payload as a batch of keys', async () => { + storageEventHandler({key: SYNC_ONYX, newValue: JSON.stringify(['test_1', 'test_2'])}); + await waitForPromisesToResolve(); + + expect(multiGet).toHaveBeenCalledWith(['test_1', 'test_2']); + expect(onStorageKeysChanged).toHaveBeenCalledWith([ + ['test_1', 'value_of_test_1'], + ['test_2', 'value_of_test_2'], + ]); + }); + + it('treats a legacy raw-key payload as a single key for backwards compatibility', async () => { + storageEventHandler({key: SYNC_ONYX, newValue: 'test_1'}); + await waitForPromisesToResolve(); + + expect(multiGet).toHaveBeenCalledWith(['test_1']); + expect(onStorageKeysChanged).toHaveBeenCalledWith([['test_1', 'value_of_test_1']]); + }); + + it('treats a raw key that parses as non-array JSON as a single key', async () => { + // A key like "123" is valid JSON but not an array; the raw string must be kept as the key. + storageEventHandler({key: SYNC_ONYX, newValue: '123'}); + await waitForPromisesToResolve(); + + expect(multiGet).toHaveBeenCalledWith(['123']); + }); + + it('ignores storage events that are not SYNC_ONYX', async () => { + storageEventHandler({key: 'someOtherKey', newValue: 'test_1'}); + storageEventHandler({key: SYNC_ONYX, newValue: null}); + await waitForPromisesToResolve(); + + expect(multiGet).not.toHaveBeenCalled(); + expect(onStorageKeysChanged).not.toHaveBeenCalled(); + }); + }); +}); From 303e1b64d08d70dc92d6d3be81d555f353f5913f Mon Sep 17 00:00:00 2001 From: eliran goshen Date: Mon, 3 Aug 2026 10:09:42 +0200 Subject: [PATCH 6/7] Address review NABs: alphabetize type imports, comment spacing, doc typo Co-Authored-By: Claude Fable 5 --- lib/Onyx.ts | 15 ++++++++------- lib/storage/InstanceSync/index.web.ts | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 1c51b5cd2..1b7685ad8 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -8,22 +8,22 @@ import type { ConnectOptions, InitOptions, KeyValueMapping, - OnyxInputKeyValueMapping, MixedOperationsQueue, + NonUndefined, + OnyxCollection, + OnyxEntry, + OnyxInput, + OnyxInputKeyValueMapping, OnyxKey, OnyxMergeCollectionInput, - OnyxSetCollectionInput, OnyxMergeInput, + OnyxMethodMap, OnyxMultiSetInput, + OnyxSetCollectionInput, OnyxSetInput, OnyxUpdate, OnyxValue, - OnyxInput, - OnyxMethodMap, SetOptions, - OnyxCollection, - NonUndefined, - OnyxEntry, } from './types'; import OnyxUtils from './OnyxUtils'; import OnyxKeys from './OnyxKeys'; @@ -82,6 +82,7 @@ function init({ 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; diff --git a/lib/storage/InstanceSync/index.web.ts b/lib/storage/InstanceSync/index.web.ts index 3c02e3e48..b79491cab 100644 --- a/lib/storage/InstanceSync/index.web.ts +++ b/lib/storage/InstanceSync/index.web.ts @@ -20,7 +20,7 @@ const MAX_SYNC_PAYLOAD_LENGTH = 1_000_000; /** * 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 + * The payload is a JSON array of the changed keys (a batch). It falls back to treating the raw * value as a single key for backwards compatibility with the previous one-key-per-event format. */ function parseSyncOnyxStorageEventValue(value: string): StorageKeyList { From cbfe68395affb7aa907792e5c2afb8f8513c2e45 Mon Sep 17 00:00:00 2001 From: eliran goshen Date: Mon, 3 Aug 2026 10:13:22 +0200 Subject: [PATCH 7/7] Fix stale @param name in keepInstancesSync doc Co-Authored-By: Claude Fable 5 --- lib/storage/providers/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/storage/providers/types.ts b/lib/storage/providers/types.ts index 953da12ca..90ada2368 100644 --- a/lib/storage/providers/types.ts +++ b/lib/storage/providers/types.ts @@ -98,7 +98,7 @@ type StorageProvider = { classifyError: (error: unknown) => ValueOf; /** - * @param onStorageKeyChanged Storage synchronization mechanism keeping all opened tabs in sync + * @param onStorageKeysChanged Storage synchronization mechanism keeping all opened tabs in sync */ keepInstancesSync?: (onStorageKeysChanged: OnStorageKeysChanged) => void; };