From d27c55fd2ad276c46a8aa7721368997b8960ef1f Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Fri, 23 Oct 2026 15:30:07 +0200 Subject: [PATCH 1/2] Fix getState for undefined keys and drop the dead nullish fallback in get and multiGet --- lib/Onyx.ts | 4 ++-- lib/OnyxSubscriptionManager.ts | 2 +- tests/unit/OnyxSubscriptionManagerTest.ts | 25 +++++++++++++++++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 3a05db78a..e1eebe000 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -791,7 +791,7 @@ function exportState(): Promise>> { * @returns The current value, or `undefined` if the key has none. */ function get(key: TKey): Promise> { - return OnyxUtils.afterInit(() => Promise.resolve((onyxSubscriptionManager.getState(key) ?? undefined) as OnyxValue)); + return OnyxUtils.afterInit(() => Promise.resolve(onyxSubscriptionManager.getState(key) as OnyxValue)); } /** @@ -813,7 +813,7 @@ function get(key: TKey): Promise> { */ function multiGet(keys: Keys): Promise<{[Index in keyof Keys]: OnyxValue}> { // map() widens the key tuple to an array, so the per-slot value types survive only through the cast. - return OnyxUtils.afterInit(() => Promise.resolve(keys.map((key) => onyxSubscriptionManager.getState(key) ?? undefined) as {[Index in keyof Keys]: OnyxValue})); + return OnyxUtils.afterInit(() => Promise.resolve(keys.map((key) => onyxSubscriptionManager.getState(key)) as {[Index in keyof Keys]: OnyxValue})); } const Onyx = { diff --git a/lib/OnyxSubscriptionManager.ts b/lib/OnyxSubscriptionManager.ts index d2bb3cc7a..aedab2fbd 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 (OnyxKeys.isRamOnlyKey(key) || !cache.getAllKeys().has(key) || cache.hasNullishStorageKey(key)) { + if (!cache.getAllKeys().has(key) || OnyxKeys.isRamOnlyKey(key) || cache.hasNullishStorageKey(key)) { return; } diff --git a/tests/unit/OnyxSubscriptionManagerTest.ts b/tests/unit/OnyxSubscriptionManagerTest.ts index e892e412b..fd6bed87c 100644 --- a/tests/unit/OnyxSubscriptionManagerTest.ts +++ b/tests/unit/OnyxSubscriptionManagerTest.ts @@ -5,6 +5,7 @@ import cache from '../../lib/OnyxCache'; import * as Logger from '../../lib/Logger'; import OnyxKeys from '../../lib/OnyxKeys'; import StorageMock from '../../lib/storage'; +import createDeferredTask from '../../lib/createDeferredTask'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; // We need access to some internal properties of `onyxSubscriptionManager` during the tests but they are private, @@ -131,6 +132,10 @@ describe('OnyxSubscriptionManager', () => { expect(onyxSubscriptionManager.getState(COLLECTION)).toBe(collectionData); }); + + it('should return undefined without throwing for an undefined key', () => { + expect(onyxSubscriptionManager.getState(undefined as unknown as OnyxKey)).toBeUndefined(); + }); }); describe('collection routing on notifyKey', () => { @@ -397,6 +402,26 @@ describe('OnyxSubscriptionManager', () => { expect(logHmmmSpy).not.toHaveBeenCalled(); }); + it('should report when a merge lands on the lost value before storage answers', async () => { + await StorageMock.setItem(ONYXKEYS.TEST_KEY, {a: 1, b: 2}); + forgetValueButKeepKey(ONYXKEYS.TEST_KEY); + const getItemMock = StorageMock.getItem as jest.Mock; + const readStorage = getItemMock.getMockImplementation() as (key: OnyxKey) => Promise; + const storageAnswer = createDeferredTask(); + getItemMock.mockImplementationOnce((key: OnyxKey) => { + const storedValue = readStorage(key); + return storageAnswer.promise.then(() => storedValue); + }); + + onyxSubscriptionManager.getState(ONYXKEYS.TEST_KEY); + await Onyx.merge(ONYXKEYS.TEST_KEY, {c: 3}); + storageAnswer.resolve(); + await waitForPromisesToResolve(); + + expect(cache.get(ONYXKEYS.TEST_KEY)).toEqual({c: 3}); + 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 5907e388655ca0e97e93710c799ea43d364e2c77 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Fri, 30 Oct 2026 14:57:09 +0100 Subject: [PATCH 2/2] Assert storage in the lost-value merge test and explain the parked read --- tests/unit/OnyxSubscriptionManagerTest.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/unit/OnyxSubscriptionManagerTest.ts b/tests/unit/OnyxSubscriptionManagerTest.ts index fd6bed87c..aa8488237 100644 --- a/tests/unit/OnyxSubscriptionManagerTest.ts +++ b/tests/unit/OnyxSubscriptionManagerTest.ts @@ -405,11 +405,13 @@ describe('OnyxSubscriptionManager', () => { it('should report when a merge lands on the lost value before storage answers', async () => { await StorageMock.setItem(ONYXKEYS.TEST_KEY, {a: 1, b: 2}); forgetValueButKeepKey(ONYXKEYS.TEST_KEY); - const getItemMock = StorageMock.getItem as jest.Mock; - const readStorage = getItemMock.getMockImplementation() as (key: OnyxKey) => Promise; + + // Park the diagnostic's storage read so the merge below lands on the empty cache before it answers. + const getItemMock = jest.mocked(StorageMock.getItem); + const readStorage = getItemMock.getMockImplementation(); const storageAnswer = createDeferredTask(); - getItemMock.mockImplementationOnce((key: OnyxKey) => { - const storedValue = readStorage(key); + getItemMock.mockImplementationOnce((key) => { + const storedValue = readStorage?.(key); return storageAnswer.promise.then(() => storedValue); }); @@ -418,7 +420,9 @@ describe('OnyxSubscriptionManager', () => { storageAnswer.resolve(); await waitForPromisesToResolve(); + // The merge built on nothing, so {a, b} is gone from cache and storage. The log is the only trace. expect(cache.get(ONYXKEYS.TEST_KEY)).toEqual({c: 3}); + expect(await StorageMock.getItem(ONYXKEYS.TEST_KEY)).toEqual({c: 3}); expect(logHmmmSpy).toHaveBeenCalledTimes(1); });