From 8c4a4a5b59a03a70867efee346369b4edb657da1 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Fri, 30 Oct 2026 15:47:33 +0100 Subject: [PATCH 1/8] Mark keys removed in another tab or missing in storage as known empty --- lib/Onyx.ts | 4 +++ lib/OnyxUtils.ts | 2 +- tests/unit/OnyxSubscriptionManagerTest.ts | 30 +++++++++++++++++++++++ tests/unit/onyxUtilsTest.ts | 12 +++++++++ tests/unit/useOnyxTest.ts | 25 +++++++++++++++++++ 5 files changed, 72 insertions(+), 1 deletion(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index fd1238388..219fbc881 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -79,6 +79,10 @@ function init({ // Capture the previous cached value before cache.set() so notifyCollection() can diff old vs new per member. const previousValue = isCollectionMember ? cache.get(key) : undefined; cache.set(key, value); + if (value === null || value === undefined) { + // The read found no row, so storage is known empty for this key, not lost from the cache. + cache.addNullishStorageKey(key); + } if (isCollectionMember && collectionKey) { let batch = collectionBatches.get(collectionKey); diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index ed557f585..638cae091 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -307,7 +307,7 @@ function get>(key: TKey): P return cachedValue; } - if (val === undefined) { + if (val === undefined || val === null) { cache.addNullishStorageKey(key); return undefined; } diff --git a/tests/unit/OnyxSubscriptionManagerTest.ts b/tests/unit/OnyxSubscriptionManagerTest.ts index aa8488237..aebccf05c 100644 --- a/tests/unit/OnyxSubscriptionManagerTest.ts +++ b/tests/unit/OnyxSubscriptionManagerTest.ts @@ -426,6 +426,36 @@ describe('OnyxSubscriptionManager', () => { expect(logHmmmSpy).toHaveBeenCalledTimes(1); }); + it('should not read storage for a key another tab removed, and still report a later real miss on its collection', async () => { + type SyncHandler = (pairs: Array<[OnyxKey, unknown]>) => void; + const syncCalls = (StorageMock.keepInstancesSync as jest.Mock).mock.calls as Array<[SyncHandler]>; + const syncHandler = syncCalls[syncCalls.length - 1]?.[0]; + if (!syncHandler) { + throw new Error('Onyx.init did not register a sync handler'); + } + const getItemMock = jest.mocked(StorageMock.getItem); + + await Onyx.set(MEMBER_1, {id: 1}); + await StorageMock.removeItem(MEMBER_1); + getItemMock.mockClear(); + // Another tab removed the key. Its sync read found no row. + syncHandler([[MEMBER_1, undefined]]); + + expect(onyxSubscriptionManager.getState(MEMBER_1)).toBeUndefined(); + await waitForPromisesToResolve(); + expect(getItemMock).not.toHaveBeenCalled(); + expect(logHmmmSpy).not.toHaveBeenCalled(); + + await StorageMock.setItem(MEMBER_2, {id: 2}); + cache.drop(MEMBER_2); + cache.setAllKeys([MEMBER_1, MEMBER_2]); + onyxSubscriptionManager.getState(MEMBER_2); + await waitForPromisesToResolve(); + + expect(logHmmmSpy).toHaveBeenCalledTimes(1); + expect(logHmmmSpy).toHaveBeenCalledWith(expect.stringContaining(COLLECTION)); + }); + it('should not report a key the index does not know, which means init has not loaded it', async () => { await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'from-storage'); cache.drop(ONYXKEYS.TEST_KEY); diff --git a/tests/unit/onyxUtilsTest.ts b/tests/unit/onyxUtilsTest.ts index 2a54d5d0d..d8b013bf5 100644 --- a/tests/unit/onyxUtilsTest.ts +++ b/tests/unit/onyxUtilsTest.ts @@ -99,6 +99,18 @@ describe('OnyxUtils', () => { afterEach(() => jest.clearAllMocks()); + describe('get', () => { + it('should mark a key as known empty when storage returns null for it', async () => { + // The key is indexed but has no cached value, and the provider reports a missing row as null. + OnyxCache.setAllKeys([ONYXKEYS.TEST_KEY]); + + await expect(OnyxUtils.get(ONYXKEYS.TEST_KEY)).resolves.toBeUndefined(); + + expect(OnyxCache.hasNullishStorageKey(ONYXKEYS.TEST_KEY)).toBe(true); + expect(OnyxCache.get(ONYXKEYS.TEST_KEY)).toBeUndefined(); + }); + }); + describe('partialSetCollection', () => { beforeEach(() => { Onyx.clear(); diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 83685255c..46bebebdc 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -219,6 +219,31 @@ describe('useOnyx', () => { expect(result.current[1].status).toEqual('loaded'); }); + it('should report loaded for a key another tab removed while a merge to it is pending', async () => { + type SyncHandler = (pairs: Array<[OnyxKey, unknown]>) => void; + const syncCalls = (StorageMock.keepInstancesSync as jest.Mock).mock.calls as Array<[SyncHandler]>; + const syncHandler = syncCalls[syncCalls.length - 1]?.[0]; + if (!syncHandler) { + throw new Error('Onyx.init did not register a sync handler'); + } + + await Onyx.set(ONYXKEYS.TEST_KEY, 'before'); + await StorageMock.removeItem(ONYXKEYS.TEST_KEY); + syncHandler([[ONYXKEYS.TEST_KEY, undefined]]); + Onyx.merge(ONYXKEYS.TEST_KEY, 'after'); + + const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + + // The removal is a known state, so the hook does not report loading for the pending merge. + expect(result.current[0]).toBeUndefined(); + expect(result.current[1].status).toEqual('loaded'); + + await act(async () => waitForPromisesToResolve()); + + expect(result.current[0]).toEqual('after'); + expect(result.current[1].status).toEqual('loaded'); + }); + it('should initially return undefined and then return cached value after multiple merge operations', async () => { Onyx.merge(ONYXKEYS.TEST_KEY, 'test1'); Onyx.merge(ONYXKEYS.TEST_KEY, 'test2'); From b7751b03215a38f8517c7dd7892a0f57d841eb9c Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Mon, 2 Nov 2026 12:17:03 +0100 Subject: [PATCH 2/8] Drop keys removed in another tab instead of marking them known empty --- lib/Onyx.ts | 7 ++++--- lib/OnyxUtils.ts | 2 +- tests/unit/onyxUtilsTest.ts | 12 ------------ tests/unit/useOnyxTest.ts | 6 +++--- 4 files changed, 8 insertions(+), 19 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 219fbc881..5d2581c7d 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -78,10 +78,11 @@ function init({ // Capture the previous cached value before cache.set() so notifyCollection() can diff old vs new per member. const previousValue = isCollectionMember ? cache.get(key) : undefined; - cache.set(key, value); if (value === null || value === undefined) { - // The read found no row, so storage is known empty for this key, not lost from the cache. - cache.addNullishStorageKey(key); + // Drop like a local removal, so the key leaves the index instead of lingering as an indexed key with no value. + cache.drop(key); + } else { + cache.set(key, value); } if (isCollectionMember && collectionKey) { diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 638cae091..ed557f585 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -307,7 +307,7 @@ function get>(key: TKey): P return cachedValue; } - if (val === undefined || val === null) { + if (val === undefined) { cache.addNullishStorageKey(key); return undefined; } diff --git a/tests/unit/onyxUtilsTest.ts b/tests/unit/onyxUtilsTest.ts index d8b013bf5..2a54d5d0d 100644 --- a/tests/unit/onyxUtilsTest.ts +++ b/tests/unit/onyxUtilsTest.ts @@ -99,18 +99,6 @@ describe('OnyxUtils', () => { afterEach(() => jest.clearAllMocks()); - describe('get', () => { - it('should mark a key as known empty when storage returns null for it', async () => { - // The key is indexed but has no cached value, and the provider reports a missing row as null. - OnyxCache.setAllKeys([ONYXKEYS.TEST_KEY]); - - await expect(OnyxUtils.get(ONYXKEYS.TEST_KEY)).resolves.toBeUndefined(); - - expect(OnyxCache.hasNullishStorageKey(ONYXKEYS.TEST_KEY)).toBe(true); - expect(OnyxCache.get(ONYXKEYS.TEST_KEY)).toBeUndefined(); - }); - }); - describe('partialSetCollection', () => { beforeEach(() => { Onyx.clear(); diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 46bebebdc..939ced1b9 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -219,7 +219,7 @@ describe('useOnyx', () => { expect(result.current[1].status).toEqual('loaded'); }); - it('should report loaded for a key another tab removed while a merge to it is pending', async () => { + it('should keep reporting loading for a key another tab removed while a merge to it is pending', async () => { type SyncHandler = (pairs: Array<[OnyxKey, unknown]>) => void; const syncCalls = (StorageMock.keepInstancesSync as jest.Mock).mock.calls as Array<[SyncHandler]>; const syncHandler = syncCalls[syncCalls.length - 1]?.[0]; @@ -234,9 +234,9 @@ describe('useOnyx', () => { const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); - // The removal is a known state, so the hook does not report loading for the pending merge. + // Same as after a local removal: the pending merge keeps the hook loading until it lands. expect(result.current[0]).toBeUndefined(); - expect(result.current[1].status).toEqual('loaded'); + expect(result.current[1].status).toEqual('loading'); await act(async () => waitForPromisesToResolve()); From 0905b80a7bb2b99d2597ad9e9223605cc72989e7 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Wed, 4 Nov 2026 11:18:34 +0100 Subject: [PATCH 3/8] Log when another tab removes a key this tab is still writing --- lib/Onyx.ts | 3 +++ lib/PendingWrites.ts | 6 ++++++ tests/unit/OnyxSubscriptionManagerTest.ts | 26 +++++++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 5d2581c7d..873dfded0 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -79,6 +79,9 @@ function init({ // Capture the previous cached value before cache.set() so notifyCollection() can diff old vs new per member. const previousValue = isCollectionMember ? cache.get(key) : undefined; if (value === null || value === undefined) { + if (PendingWrites.hasPendingWriteForKey(key)) { + Logger.logHmmm(`Sync removed a key while this tab was writing it: ${key}`); + } // Drop like a local removal, so the key leaves the index instead of lingering as an indexed key with no value. cache.drop(key); } else { diff --git a/lib/PendingWrites.ts b/lib/PendingWrites.ts index 9aac33f4b..f77371adc 100644 --- a/lib/PendingWrites.ts +++ b/lib/PendingWrites.ts @@ -64,6 +64,11 @@ function addWriteListener(listener: (keys: OnyxKey[]) => void): () => void { return () => writeListeners.delete(listener); } +/** True while a write to exactly this key is in flight. Ignores global writes such as clear. */ +function hasPendingWriteForKey(key: OnyxKey): boolean { + return pendingWritesByKey.has(key); +} + /** * In-flight writes that can change the value delivered to a subscriber of `key`: writes to the key * itself, writes to any member when `key` is a collection root, and global writes (clear). @@ -118,6 +123,7 @@ export default { trackPendingGlobalWrite, addWriteListener, getPendingWritesForKey, + hasPendingWriteForKey, scheduleInitialSubscriberNotification, clearPendingWrites, }; diff --git a/tests/unit/OnyxSubscriptionManagerTest.ts b/tests/unit/OnyxSubscriptionManagerTest.ts index aebccf05c..7b56f4ccc 100644 --- a/tests/unit/OnyxSubscriptionManagerTest.ts +++ b/tests/unit/OnyxSubscriptionManagerTest.ts @@ -456,6 +456,32 @@ describe('OnyxSubscriptionManager', () => { expect(logHmmmSpy).toHaveBeenCalledWith(expect.stringContaining(COLLECTION)); }); + it('should log once when another tab removes a key this tab is still writing, and not during a local clear', async () => { + type SyncHandler = (pairs: Array<[OnyxKey, unknown]>) => void; + const syncCalls = (StorageMock.keepInstancesSync as jest.Mock).mock.calls as Array<[SyncHandler]>; + const syncHandler = syncCalls[syncCalls.length - 1]?.[0]; + if (!syncHandler) { + throw new Error('Onyx.init did not register a sync handler'); + } + + await Onyx.set(MEMBER_1, {id: 1}); + const mergePromise = Onyx.merge(MEMBER_1, {id: 2}); + // The write is in flight, so the removal collides with it. + syncHandler([[MEMBER_1, undefined]]); + await mergePromise; + + expect(logHmmmSpy).toHaveBeenCalledTimes(1); + expect(logHmmmSpy).toHaveBeenCalledWith(expect.stringContaining(MEMBER_1)); + + await Onyx.set(MEMBER_2, {id: 2}); + const clearPromise = Onyx.clear(); + // A global write is not a write to this key, so a removal during a local clear stays quiet. + syncHandler([[MEMBER_2, undefined]]); + await clearPromise; + + expect(logHmmmSpy).toHaveBeenCalledTimes(1); + }); + it('should not report a key the index does not know, which means init has not loaded it', async () => { await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'from-storage'); cache.drop(ONYXKEYS.TEST_KEY); From cb8bdd5c6dc7599eb3abb958cb8524c0a8f811a0 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Wed, 4 Nov 2026 11:37:12 +0100 Subject: [PATCH 4/8] Split the sync removal tests and match the sibling sync callback pattern --- tests/unit/OnyxSubscriptionManagerTest.ts | 53 +++++++++++++---------- tests/unit/useOnyxTest.ts | 10 ++--- 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/tests/unit/OnyxSubscriptionManagerTest.ts b/tests/unit/OnyxSubscriptionManagerTest.ts index 7b56f4ccc..bb3e44b71 100644 --- a/tests/unit/OnyxSubscriptionManagerTest.ts +++ b/tests/unit/OnyxSubscriptionManagerTest.ts @@ -426,29 +426,35 @@ describe('OnyxSubscriptionManager', () => { expect(logHmmmSpy).toHaveBeenCalledTimes(1); }); - it('should not read storage for a key another tab removed, and still report a later real miss on its collection', async () => { - type SyncHandler = (pairs: Array<[OnyxKey, unknown]>) => void; - const syncCalls = (StorageMock.keepInstancesSync as jest.Mock).mock.calls as Array<[SyncHandler]>; - const syncHandler = syncCalls[syncCalls.length - 1]?.[0]; - if (!syncHandler) { - throw new Error('Onyx.init did not register a sync handler'); - } + it('should not read storage or report for a key another tab removed', async () => { + const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls.at(-1)?.[0]; + expect(syncCallback).toBeDefined(); const getItemMock = jest.mocked(StorageMock.getItem); await Onyx.set(MEMBER_1, {id: 1}); await StorageMock.removeItem(MEMBER_1); getItemMock.mockClear(); - // Another tab removed the key. Its sync read found no row. - syncHandler([[MEMBER_1, undefined]]); + syncCallback([[MEMBER_1, undefined]]); expect(onyxSubscriptionManager.getState(MEMBER_1)).toBeUndefined(); await waitForPromisesToResolve(); + expect(getItemMock).not.toHaveBeenCalled(); expect(logHmmmSpy).not.toHaveBeenCalled(); + }); + + it('should still report a real miss on a collection after another tab removed one of its members', async () => { + const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls.at(-1)?.[0]; + expect(syncCallback).toBeDefined(); + + await Onyx.set(MEMBER_1, {id: 1}); + await StorageMock.removeItem(MEMBER_1); + syncCallback([[MEMBER_1, undefined]]); + onyxSubscriptionManager.getState(MEMBER_1); + await waitForPromisesToResolve(); await StorageMock.setItem(MEMBER_2, {id: 2}); - cache.drop(MEMBER_2); - cache.setAllKeys([MEMBER_1, MEMBER_2]); + forgetValueButKeepKey(MEMBER_2); onyxSubscriptionManager.getState(MEMBER_2); await waitForPromisesToResolve(); @@ -456,30 +462,29 @@ describe('OnyxSubscriptionManager', () => { expect(logHmmmSpy).toHaveBeenCalledWith(expect.stringContaining(COLLECTION)); }); - it('should log once when another tab removes a key this tab is still writing, and not during a local clear', async () => { - type SyncHandler = (pairs: Array<[OnyxKey, unknown]>) => void; - const syncCalls = (StorageMock.keepInstancesSync as jest.Mock).mock.calls as Array<[SyncHandler]>; - const syncHandler = syncCalls[syncCalls.length - 1]?.[0]; - if (!syncHandler) { - throw new Error('Onyx.init did not register a sync handler'); - } + it('should log once when another tab removes a key this tab is still writing', async () => { + const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls.at(-1)?.[0]; + expect(syncCallback).toBeDefined(); await Onyx.set(MEMBER_1, {id: 1}); const mergePromise = Onyx.merge(MEMBER_1, {id: 2}); - // The write is in flight, so the removal collides with it. - syncHandler([[MEMBER_1, undefined]]); + syncCallback([[MEMBER_1, undefined]]); await mergePromise; expect(logHmmmSpy).toHaveBeenCalledTimes(1); expect(logHmmmSpy).toHaveBeenCalledWith(expect.stringContaining(MEMBER_1)); + }); + + it('should not log a removal from another tab during a local clear', async () => { + const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls.at(-1)?.[0]; + expect(syncCallback).toBeDefined(); - await Onyx.set(MEMBER_2, {id: 2}); + await Onyx.set(MEMBER_1, {id: 1}); const clearPromise = Onyx.clear(); - // A global write is not a write to this key, so a removal during a local clear stays quiet. - syncHandler([[MEMBER_2, undefined]]); + syncCallback([[MEMBER_1, undefined]]); await clearPromise; - expect(logHmmmSpy).toHaveBeenCalledTimes(1); + expect(logHmmmSpy).not.toHaveBeenCalled(); }); it('should not report a key the index does not know, which means init has not loaded it', async () => { diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 939ced1b9..5e11f6755 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -220,16 +220,12 @@ describe('useOnyx', () => { }); it('should keep reporting loading for a key another tab removed while a merge to it is pending', async () => { - type SyncHandler = (pairs: Array<[OnyxKey, unknown]>) => void; - const syncCalls = (StorageMock.keepInstancesSync as jest.Mock).mock.calls as Array<[SyncHandler]>; - const syncHandler = syncCalls[syncCalls.length - 1]?.[0]; - if (!syncHandler) { - throw new Error('Onyx.init did not register a sync handler'); - } + const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls.at(-1)?.[0]; + expect(syncCallback).toBeDefined(); await Onyx.set(ONYXKEYS.TEST_KEY, 'before'); await StorageMock.removeItem(ONYXKEYS.TEST_KEY); - syncHandler([[ONYXKEYS.TEST_KEY, undefined]]); + syncCallback([[ONYXKEYS.TEST_KEY, undefined]]); Onyx.merge(ONYXKEYS.TEST_KEY, 'after'); const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); From d64f6b8b003dc3e681f7defec68ee36b78474a37 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Thu, 5 Nov 2026 12:09:18 +0100 Subject: [PATCH 5/8] Pin that a known-empty key removed in another tab stays known empty --- tests/unit/useOnyxTest.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 5e11f6755..c9d325596 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -5,6 +5,7 @@ import type {UseOnyxSelector} from '../../lib/useOnyx'; import type GenericCollection from '../utils/GenericCollection'; import Onyx, {useOnyx} from '../../lib'; +import cache from '../../lib/OnyxCache'; import {resetDeferredInitTask} from '../../lib/OnyxUtils'; import StorageMock from '../../lib/storage'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; @@ -240,6 +241,29 @@ describe('useOnyx', () => { expect(result.current[1].status).toEqual('loaded'); }); + it('should keep a known-empty key known empty when another tab removes it, like a local removal does', async () => { + const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls.at(-1)?.[0]; + expect(syncCallback).toBeDefined(); + + cache.addNullishStorageKey(ONYXKEYS.TEST_KEY); + await Onyx.merge(ONYXKEYS.TEST_KEY, null); + expect(cache.hasCacheForKey(ONYXKEYS.TEST_KEY)).toBe(true); + + await Onyx.clear(); + cache.addNullishStorageKey(ONYXKEYS.TEST_KEY); + syncCallback([[ONYXKEYS.TEST_KEY, undefined]]); + expect(cache.hasCacheForKey(ONYXKEYS.TEST_KEY)).toBe(true); + + Onyx.merge(ONYXKEYS.TEST_KEY, 'after'); + const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + + expect(result.current[1].status).toEqual('loaded'); + + await act(async () => waitForPromisesToResolve()); + + expect(result.current[0]).toEqual('after'); + }); + it('should initially return undefined and then return cached value after multiple merge operations', async () => { Onyx.merge(ONYXKEYS.TEST_KEY, 'test1'); Onyx.merge(ONYXKEYS.TEST_KEY, 'test2'); From e057b8fb3f577ca48038cb8ffa3253983826a787 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Thu, 5 Nov 2026 13:04:17 +0100 Subject: [PATCH 6/8] Drop the sync removal log and its tests, keep the drop fix only --- lib/Onyx.ts | 5 +-- lib/PendingWrites.ts | 6 ---- tests/unit/OnyxSubscriptionManagerTest.ts | 38 ++++++----------------- tests/unit/useOnyxTest.ts | 24 -------------- 4 files changed, 10 insertions(+), 63 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 873dfded0..4b9230bd9 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -76,12 +76,9 @@ function init({ const collectionKey = OnyxKeys.getCollectionKey(key); const isCollectionMember = !!collectionKey && OnyxKeys.isCollectionMemberKey(collectionKey, key); - // Capture the previous cached value before cache.set() so notifyCollection() can diff old vs new per member. + // Capture the previous cached value before the cache changes so notifyCollection() can diff old vs new per member. const previousValue = isCollectionMember ? cache.get(key) : undefined; if (value === null || value === undefined) { - if (PendingWrites.hasPendingWriteForKey(key)) { - Logger.logHmmm(`Sync removed a key while this tab was writing it: ${key}`); - } // Drop like a local removal, so the key leaves the index instead of lingering as an indexed key with no value. cache.drop(key); } else { diff --git a/lib/PendingWrites.ts b/lib/PendingWrites.ts index f77371adc..9aac33f4b 100644 --- a/lib/PendingWrites.ts +++ b/lib/PendingWrites.ts @@ -64,11 +64,6 @@ function addWriteListener(listener: (keys: OnyxKey[]) => void): () => void { return () => writeListeners.delete(listener); } -/** True while a write to exactly this key is in flight. Ignores global writes such as clear. */ -function hasPendingWriteForKey(key: OnyxKey): boolean { - return pendingWritesByKey.has(key); -} - /** * In-flight writes that can change the value delivered to a subscriber of `key`: writes to the key * itself, writes to any member when `key` is a collection root, and global writes (clear). @@ -123,7 +118,6 @@ export default { trackPendingGlobalWrite, addWriteListener, getPendingWritesForKey, - hasPendingWriteForKey, scheduleInitialSubscriberNotification, clearPendingWrites, }; diff --git a/tests/unit/OnyxSubscriptionManagerTest.ts b/tests/unit/OnyxSubscriptionManagerTest.ts index bb3e44b71..6174e93f0 100644 --- a/tests/unit/OnyxSubscriptionManagerTest.ts +++ b/tests/unit/OnyxSubscriptionManagerTest.ts @@ -354,6 +354,13 @@ describe('OnyxSubscriptionManager', () => { cache.setAllKeys([key]); }; + // The handler Onyx.init registered for changes another tab wrote to storage. + const getSyncCallback = (): ((pairs: Array<[OnyxKey, unknown]>) => void) => { + const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls.at(-1)?.[0] as ((pairs: Array<[OnyxKey, unknown]>) => void) | undefined; + expect(syncCallback).toBeDefined(); + return syncCallback as (pairs: Array<[OnyxKey, unknown]>) => void; + }; + it('should report once when the cache has no value but storage does', async () => { await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'from-storage'); forgetValueButKeepKey(ONYXKEYS.TEST_KEY); @@ -427,8 +434,7 @@ describe('OnyxSubscriptionManager', () => { }); it('should not read storage or report for a key another tab removed', async () => { - const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls.at(-1)?.[0]; - expect(syncCallback).toBeDefined(); + const syncCallback = getSyncCallback(); const getItemMock = jest.mocked(StorageMock.getItem); await Onyx.set(MEMBER_1, {id: 1}); @@ -444,8 +450,7 @@ describe('OnyxSubscriptionManager', () => { }); it('should still report a real miss on a collection after another tab removed one of its members', async () => { - const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls.at(-1)?.[0]; - expect(syncCallback).toBeDefined(); + const syncCallback = getSyncCallback(); await Onyx.set(MEMBER_1, {id: 1}); await StorageMock.removeItem(MEMBER_1); @@ -462,31 +467,6 @@ describe('OnyxSubscriptionManager', () => { expect(logHmmmSpy).toHaveBeenCalledWith(expect.stringContaining(COLLECTION)); }); - it('should log once when another tab removes a key this tab is still writing', async () => { - const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls.at(-1)?.[0]; - expect(syncCallback).toBeDefined(); - - await Onyx.set(MEMBER_1, {id: 1}); - const mergePromise = Onyx.merge(MEMBER_1, {id: 2}); - syncCallback([[MEMBER_1, undefined]]); - await mergePromise; - - expect(logHmmmSpy).toHaveBeenCalledTimes(1); - expect(logHmmmSpy).toHaveBeenCalledWith(expect.stringContaining(MEMBER_1)); - }); - - it('should not log a removal from another tab during a local clear', async () => { - const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls.at(-1)?.[0]; - expect(syncCallback).toBeDefined(); - - await Onyx.set(MEMBER_1, {id: 1}); - const clearPromise = Onyx.clear(); - syncCallback([[MEMBER_1, undefined]]); - await clearPromise; - - expect(logHmmmSpy).not.toHaveBeenCalled(); - }); - it('should not report a key the index does not know, which means init has not loaded it', async () => { await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'from-storage'); cache.drop(ONYXKEYS.TEST_KEY); diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index c9d325596..5e11f6755 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -5,7 +5,6 @@ import type {UseOnyxSelector} from '../../lib/useOnyx'; import type GenericCollection from '../utils/GenericCollection'; import Onyx, {useOnyx} from '../../lib'; -import cache from '../../lib/OnyxCache'; import {resetDeferredInitTask} from '../../lib/OnyxUtils'; import StorageMock from '../../lib/storage'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; @@ -241,29 +240,6 @@ describe('useOnyx', () => { expect(result.current[1].status).toEqual('loaded'); }); - it('should keep a known-empty key known empty when another tab removes it, like a local removal does', async () => { - const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls.at(-1)?.[0]; - expect(syncCallback).toBeDefined(); - - cache.addNullishStorageKey(ONYXKEYS.TEST_KEY); - await Onyx.merge(ONYXKEYS.TEST_KEY, null); - expect(cache.hasCacheForKey(ONYXKEYS.TEST_KEY)).toBe(true); - - await Onyx.clear(); - cache.addNullishStorageKey(ONYXKEYS.TEST_KEY); - syncCallback([[ONYXKEYS.TEST_KEY, undefined]]); - expect(cache.hasCacheForKey(ONYXKEYS.TEST_KEY)).toBe(true); - - Onyx.merge(ONYXKEYS.TEST_KEY, 'after'); - const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); - - expect(result.current[1].status).toEqual('loaded'); - - await act(async () => waitForPromisesToResolve()); - - expect(result.current[0]).toEqual('after'); - }); - it('should initially return undefined and then return cached value after multiple merge operations', async () => { Onyx.merge(ONYXKEYS.TEST_KEY, 'test1'); Onyx.merge(ONYXKEYS.TEST_KEY, 'test2'); From 40d1c56c79faab96cb4149309a5f9ac8161b1a2f Mon Sep 17 00:00:00 2001 From: TaduJR Date: Thu, 5 Nov 2026 17:26:47 +0300 Subject: [PATCH 7/8] Keep keys removed in another tab indexed and only silence the cache-miss check --- lib/Onyx.ts | 9 ++- lib/OnyxCache.ts | 20 ++++++ lib/OnyxSubscriptionManager.ts | 2 +- tests/unit/OnyxSubscriptionManagerTest.ts | 33 ++++++++++ tests/unit/onyxCacheTest.tsx | 27 +++++++++ tests/unit/onyxTest.ts | 74 +++++++++++++++++++++++ tests/unit/useOnyxTest.ts | 23 +++++++ 7 files changed, 182 insertions(+), 6 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 4b9230bd9..6fc64108b 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -76,13 +76,12 @@ function init({ const collectionKey = OnyxKeys.getCollectionKey(key); const isCollectionMember = !!collectionKey && OnyxKeys.isCollectionMemberKey(collectionKey, key); - // Capture the previous cached value before the cache changes so notifyCollection() can diff old vs new per member. + // Capture the previous cached value before cache.set() so notifyCollection() can diff old vs new per member. const previousValue = isCollectionMember ? cache.get(key) : undefined; + cache.set(key, value); + // The key stays indexed because this answer can predate a write of ours still on its way to storage, and only indexed keys get deleted by Onyx.clear and collection sweeps. if (value === null || value === undefined) { - // Drop like a local removal, so the key leaves the index instead of lingering as an indexed key with no value. - cache.drop(key); - } else { - cache.set(key, value); + cache.addRemovedInOtherInstanceKey(key); } if (isCollectionMember && collectionKey) { diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index 3003b5f1f..060a6e17c 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -48,6 +48,9 @@ class OnyxCache { /** A list of keys where a nullish value has been fetched from storage before, but the key still exists in cache */ private nullishStorageKeys: Set; + /** Keys another instance removed, which explains why the cache has no value for them. They still count as uncached, unlike nullish keys. */ + private removedInOtherInstanceKeys: Set; + /** A map of cached values */ private storageMap: Record>; @@ -72,6 +75,7 @@ class OnyxCache { constructor() { this.storageKeys = new Set(); this.nullishStorageKeys = new Set(); + this.removedInOtherInstanceKeys = new Set(); this.storageMap = {}; this.pendingPromises = new Map(); this.collectionSnapshots = new Map(); @@ -87,6 +91,8 @@ class OnyxCache { 'addNullishStorageKey', 'hasNullishStorageKey', 'clearNullishStorageKeys', + 'addRemovedInOtherInstanceKey', + 'hasRemovedInOtherInstanceKey', 'set', 'drop', 'merge', @@ -153,6 +159,16 @@ class OnyxCache { this.nullishStorageKeys = new Set(); } + /** Records that another instance removed this key */ + addRemovedInOtherInstanceKey(key: OnyxKey): void { + this.removedInOtherInstanceKeys.add(key); + } + + /** Whether another instance removed this key and it hasn't changed here since */ + hasRemovedInOtherInstanceKey(key: OnyxKey): boolean { + return this.removedInOtherInstanceKeys.has(key); + } + /** Check whether cache has data for the given key */ hasCacheForKey(key: OnyxKey): boolean { return this.storageMap[key] !== undefined || this.hasNullishStorageKey(key); @@ -173,6 +189,7 @@ class OnyxCache { // When a key is explicitly set in cache, we can remove it from the list of nullish keys, // since it will either be set to a non nullish value or removed from the cache completely. this.nullishStorageKeys.delete(key); + this.removedInOtherInstanceKeys.delete(key); const collectionKey = OnyxKeys.getCollectionKey(key); const oldValue = this.storageMap[key]; @@ -210,6 +227,7 @@ class OnyxCache { } this.storageKeys.delete(key); + this.removedInOtherInstanceKeys.delete(key); OnyxKeys.deregisterMemberKey(key); } @@ -249,6 +267,7 @@ class OnyxCache { } } else { this.nullishStorageKeys.delete(key); + this.removedInOtherInstanceKeys.delete(key); const existing = this.storageMap[key]; @@ -320,6 +339,7 @@ class OnyxCache { } } else { this.nullishStorageKeys.delete(key); + this.removedInOtherInstanceKeys.delete(key); // Per-key merge instead of spreading the entire storageMap const existing = this.storageMap[key]; diff --git a/lib/OnyxSubscriptionManager.ts b/lib/OnyxSubscriptionManager.ts index aedab2fbd..81bddac3f 100644 --- a/lib/OnyxSubscriptionManager.ts +++ b/lib/OnyxSubscriptionManager.ts @@ -19,7 +19,7 @@ type GenericListener = (value: unknown, key: OnyxKey) => void; const reportedCacheMisses = new Set(); function reportCacheMissIfStorageHasValue(key: OnyxKey): void { - if (!cache.getAllKeys().has(key) || OnyxKeys.isRamOnlyKey(key) || cache.hasNullishStorageKey(key)) { + if (!cache.getAllKeys().has(key) || OnyxKeys.isRamOnlyKey(key) || cache.hasNullishStorageKey(key) || cache.hasRemovedInOtherInstanceKey(key)) { return; } diff --git a/tests/unit/OnyxSubscriptionManagerTest.ts b/tests/unit/OnyxSubscriptionManagerTest.ts index 6174e93f0..b4895b721 100644 --- a/tests/unit/OnyxSubscriptionManagerTest.ts +++ b/tests/unit/OnyxSubscriptionManagerTest.ts @@ -467,6 +467,39 @@ describe('OnyxSubscriptionManager', () => { expect(logHmmmSpy).toHaveBeenCalledWith(expect.stringContaining(COLLECTION)); }); + it('should not report when another tab removes a key and writes it back before this tab reads it', async () => { + // Given another tab removed a key and this tab applied that removal + const syncCallback = getSyncCallback(); + await Onyx.set(ONYXKEYS.TEST_KEY, 'first'); + await StorageMock.removeItem(ONYXKEYS.TEST_KEY); + syncCallback([[ONYXKEYS.TEST_KEY, undefined]]); + + // When that tab writes the key back, and this tab reads it before the sync for that write arrives + await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'second'); + onyxSubscriptionManager.getState(ONYXKEYS.TEST_KEY); + await waitForPromisesToResolve(); + + // Then nothing is reported, because nothing was lost + expect(logHmmmSpy).not.toHaveBeenCalled(); + }); + + it('should report a key lost after another tab removed it and it got a value again', async () => { + // Given a key another tab removed, which then got a value again + const syncCallback = getSyncCallback(); + await Onyx.set(ONYXKEYS.TEST_KEY, 'first'); + await StorageMock.removeItem(ONYXKEYS.TEST_KEY); + syncCallback([[ONYXKEYS.TEST_KEY, undefined]]); + await Onyx.set(ONYXKEYS.TEST_KEY, 'second'); + + // When the cache then has no value for it while storage still does + jest.spyOn(cache, 'get').mockReturnValueOnce(undefined); + expect(onyxSubscriptionManager.getState(ONYXKEYS.TEST_KEY)).toBeUndefined(); + await waitForPromisesToResolve(); + + // Then this is a real miss, and the old removal doesn't hide it + expect(logHmmmSpy).toHaveBeenCalledTimes(1); + }); + it('should not report a key the index does not know, which means init has not loaded it', async () => { await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'from-storage'); cache.drop(ONYXKEYS.TEST_KEY); diff --git a/tests/unit/onyxCacheTest.tsx b/tests/unit/onyxCacheTest.tsx index 5d6e78344..47ccd4aaf 100644 --- a/tests/unit/onyxCacheTest.tsx +++ b/tests/unit/onyxCacheTest.tsx @@ -217,6 +217,33 @@ describe('Onyx', () => { }); }); + describe('keys removed in another instance', () => { + it('should not count as cached, unlike a nullish key', () => { + // Given a key another instance removed + cache.addRemovedInOtherInstanceKey('mockKey'); + + // Then it is marked, and reads still treat the key as having nothing cached + expect(cache.hasRemovedInOtherInstanceKey('mockKey')).toBe(true); + expect(cache.hasCacheForKey('mockKey')).toBe(false); + }); + + it.each<[string, (onyxCache: typeof OnyxCache) => void]>([ + ['set', (onyxCache) => onyxCache.set('mockKey', 'value')], + ['merge', (onyxCache) => onyxCache.merge({mockKey: {a: 1}})], + ['hydrate', (onyxCache) => onyxCache.hydrate({mockKey: 'value'})], + ['drop', (onyxCache) => onyxCache.drop('mockKey')], + ])('should forget the mark on %s, because the removal no longer explains a missing value', (_name, change) => { + // Given a key another instance removed + cache.addRemovedInOtherInstanceKey('mockKey'); + + // When the key changes state here + change(cache); + + // Then the old removal is forgotten + expect(cache.hasRemovedInOtherInstanceKey('mockKey')).toBe(false); + }); + }); + describe('merge', () => { it('should create the value in cache when it does not exist', () => { // Given empty cache diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 7e465feaf..8ccefb6e7 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -4616,6 +4616,80 @@ describe('RAM-only keys should not read from storage', () => { }); }); +// A sync read can start before this instance's write reaches storage and answer after it. The answer reports the key as removed, while storage ends up holding the write. +describe('a stale removal synced from another instance', () => { + const COLLECTION = ONYX_KEYS.COLLECTION.TEST_KEY; + const MEMBER_1 = `${COLLECTION}1`; + const MEMBER_2 = `${COLLECTION}2`; + + const writeThenReceiveStaleRemoval = async (key: OnyxKey, value: unknown) => { + await Onyx.set(key, value); + const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls.at(-1)?.[0]; + syncCallback([[key, undefined]]); + }; + + beforeEach(async () => { + Object.assign(OnyxUtils.getDeferredInitTask(), createDeferredTask()); + Onyx.init({keys: ONYX_KEYS, shouldSyncMultipleInstances: true}); + await act(async () => waitForPromisesToResolve()); + + // Another key keeps the index non-empty, as in any running app, because getAllKeys lists storage when the index is empty and that would hide a missed key. + await Onyx.set(ONYX_KEYS.OTHER_TEST, 'other'); + }); + + afterEach(async () => { + await Onyx.clear(); + await StorageMock.clear(); + }); + + it('should still delete the key on Onyx.clear', async () => { + // Given a stored value whose key this instance was told is removed + await writeThenReceiveStaleRemoval(ONYX_KEYS.TEST_KEY, 'mine'); + + // When this instance clears, as it does on sign-out + await Onyx.clear(); + + // Then the value doesn't survive into the next session + expect(await StorageMock.getItem(ONYX_KEYS.TEST_KEY)).toBeNull(); + }); + + it('should merge into the stored value on mergeCollection instead of overwriting it', async () => { + // Given a stored member whose key this instance was told is removed + await writeThenReceiveStaleRemoval(MEMBER_1, {a: 1, b: 2}); + + // When a collection merge touches that member + await Onyx.mergeCollection(COLLECTION, {[MEMBER_1]: {c: 3}} as GenericCollection); + + // Then the stored fields are kept + expect(await StorageMock.getItem(MEMBER_1)).toEqual({a: 1, b: 2, c: 3}); + }); + + it.each<[string, () => Promise]>([ + ['multiSet', () => Onyx.multiSet({[MEMBER_1]: null})], + ['mergeCollection', () => Onyx.mergeCollection(COLLECTION, {[MEMBER_1]: null} as GenericCollection)], + ])('should delete the stored value when %s writes null', async (_name, writeNull) => { + // Given a stored member whose key this instance was told is removed + await writeThenReceiveStaleRemoval(MEMBER_1, {a: 1}); + + // When a later write removes it + await writeNull(); + + // Then the row doesn't come back on the next load + expect(await StorageMock.getItem(MEMBER_1)).toBeNull(); + }); + + it('should delete a member that setCollection leaves out', async () => { + // Given a stored member whose key this instance was told is removed + await writeThenReceiveStaleRemoval(MEMBER_1, {a: 1}); + + // When setCollection replaces the collection without it + await Onyx.setCollection(COLLECTION, {[MEMBER_2]: {b: 2}} as GenericCollection); + + // Then the row doesn't come back on the next load + expect(await StorageMock.getItem(MEMBER_1)).toBeNull(); + }); +}); + describe('get() should prefer cache over stale storage', () => { let cache: typeof OnyxCache; diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 5e11f6755..1dd74103d 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -5,6 +5,7 @@ import type {UseOnyxSelector} from '../../lib/useOnyx'; import type GenericCollection from '../utils/GenericCollection'; import Onyx, {useOnyx} from '../../lib'; +import cache from '../../lib/OnyxCache'; import {resetDeferredInitTask} from '../../lib/OnyxUtils'; import StorageMock from '../../lib/storage'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; @@ -240,6 +241,28 @@ describe('useOnyx', () => { expect(result.current[1].status).toEqual('loaded'); }); + it('should keep reporting loading for a known-empty key another tab removed while a merge to it is pending', async () => { + const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls.at(-1)?.[0]; + expect(syncCallback).toBeDefined(); + + // Given a key this tab knows is empty, which another tab then removes + cache.addNullishStorageKey(ONYXKEYS.TEST_KEY); + syncCallback([[ONYXKEYS.TEST_KEY, undefined]]); + + // When a merge to it is pending as a subscriber mounts + Onyx.merge(ONYXKEYS.TEST_KEY, 'after'); + const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + + // Then the subscriber waits for the merge instead of seeing a loaded key with no value + expect(result.current[0]).toBeUndefined(); + expect(result.current[1].status).toEqual('loading'); + + await act(async () => waitForPromisesToResolve()); + + expect(result.current[0]).toEqual('after'); + expect(result.current[1].status).toEqual('loaded'); + }); + it('should initially return undefined and then return cached value after multiple merge operations', async () => { Onyx.merge(ONYXKEYS.TEST_KEY, 'test1'); Onyx.merge(ONYXKEYS.TEST_KEY, 'test2'); From f62fa782b6350af37df64d9dc97be7834f3b5315 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Tue, 10 Nov 2026 10:06:08 +0100 Subject: [PATCH 8/8] Apply review nits: forgetEmptyMarks helper, explicit sync in tests, clear keeps marks --- lib/Onyx.ts | 2 +- lib/OnyxCache.ts | 15 +++++++++------ tests/unit/OnyxSubscriptionManagerTest.ts | 18 ++++++++++++++++++ tests/unit/useOnyxTest.ts | 1 + 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 6fc64108b..6c79a8c26 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -78,8 +78,8 @@ function init({ // Capture the previous cached value before cache.set() so notifyCollection() can diff old vs new per member. const previousValue = isCollectionMember ? cache.get(key) : undefined; - cache.set(key, value); // The key stays indexed because this answer can predate a write of ours still on its way to storage, and only indexed keys get deleted by Onyx.clear and collection sweeps. + cache.set(key, value); if (value === null || value === undefined) { cache.addRemovedInOtherInstanceKey(key); } diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index 060a6e17c..96df4627e 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -169,6 +169,12 @@ class OnyxCache { return this.removedInOtherInstanceKeys.has(key); } + /** A write here replaces what the cache knew about the key being empty, including another instance's removal */ + private forgetEmptyMarks(key: OnyxKey): void { + this.nullishStorageKeys.delete(key); + this.removedInOtherInstanceKeys.delete(key); + } + /** Check whether cache has data for the given key */ hasCacheForKey(key: OnyxKey): boolean { return this.storageMap[key] !== undefined || this.hasNullishStorageKey(key); @@ -188,8 +194,7 @@ class OnyxCache { // When a key is explicitly set in cache, we can remove it from the list of nullish keys, // since it will either be set to a non nullish value or removed from the cache completely. - this.nullishStorageKeys.delete(key); - this.removedInOtherInstanceKeys.delete(key); + this.forgetEmptyMarks(key); const collectionKey = OnyxKeys.getCollectionKey(key); const oldValue = this.storageMap[key]; @@ -266,8 +271,7 @@ class OnyxCache { affectedCollections.add(collectionKey); } } else { - this.nullishStorageKeys.delete(key); - this.removedInOtherInstanceKeys.delete(key); + this.forgetEmptyMarks(key); const existing = this.storageMap[key]; @@ -338,8 +342,7 @@ class OnyxCache { affectedCollections.add(collectionKey); } } else { - this.nullishStorageKeys.delete(key); - this.removedInOtherInstanceKeys.delete(key); + this.forgetEmptyMarks(key); // Per-key merge instead of spreading the entire storageMap const existing = this.storageMap[key]; diff --git a/tests/unit/OnyxSubscriptionManagerTest.ts b/tests/unit/OnyxSubscriptionManagerTest.ts index b4895b721..bbecc4ad5 100644 --- a/tests/unit/OnyxSubscriptionManagerTest.ts +++ b/tests/unit/OnyxSubscriptionManagerTest.ts @@ -28,6 +28,7 @@ const MEMBER_2 = `${COLLECTION}2`; Onyx.init({ keys: ONYXKEYS, + shouldSyncMultipleInstances: true, }); beforeEach(() => Onyx.clear()); @@ -500,6 +501,23 @@ describe('OnyxSubscriptionManager', () => { expect(logHmmmSpy).toHaveBeenCalledTimes(1); }); + it('should not report a key whose removal and re-add by another tab straddle a clear that keeps it', async () => { + // Given a key another tab removed + const syncCallback = getSyncCallback(); + await Onyx.set(ONYXKEYS.TEST_KEY, 'first'); + await StorageMock.removeItem(ONYXKEYS.TEST_KEY); + syncCallback([[ONYXKEYS.TEST_KEY, undefined]]); + + // When that tab writes it back, and this tab clears while keeping the key before that sync arrives + await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'second'); + await Onyx.clear([ONYXKEYS.TEST_KEY]); + onyxSubscriptionManager.getState(ONYXKEYS.TEST_KEY); + await waitForPromisesToResolve(); + + // Then the pending write-back isn't reported as a lost value + expect(logHmmmSpy).not.toHaveBeenCalled(); + }); + it('should not report a key the index does not know, which means init has not loaded it', async () => { await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'from-storage'); cache.drop(ONYXKEYS.TEST_KEY); diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 1dd74103d..9df4debba 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -26,6 +26,7 @@ Onyx.init({ keys: ONYXKEYS, skippableCollectionMemberIDs: ['skippable-id'], ramOnlyKeys: [ONYXKEYS.RAM_ONLY_KEY, ONYXKEYS.COLLECTION.RAM_ONLY_COLLECTION, ONYXKEYS.RAM_ONLY_WITH_INITIAL_VALUE], + shouldSyncMultipleInstances: true, }); beforeEach(async () => {