Skip to content

Commit 1d12330

Browse files
committed
fix: prefer cache over stale storage in get() to prevent concurrent write race
1 parent bcfee2c commit 1d12330

2 files changed

Lines changed: 59 additions & 0 deletions

File tree

lib/OnyxUtils.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,12 @@ function get<TKey extends OnyxKey, TValue extends OnyxValue<TKey>>(key: TKey): P
290290
}
291291
}
292292

293+
// Prefer cache over stale storage if a concurrent write populated it during the read.
294+
const cachedValue = cache.get(key) as TValue;
295+
if (cachedValue !== undefined) {
296+
return cachedValue;
297+
}
298+
293299
if (val === undefined) {
294300
cache.addNullishStorageKey(key);
295301
return undefined;

tests/unit/onyxTest.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3477,3 +3477,56 @@ describe('RAM-only keys should not read from storage', () => {
34773477
Onyx.disconnect(connection);
34783478
});
34793479
});
3480+
3481+
describe('get() should prefer cache over stale storage', () => {
3482+
let cache: typeof OnyxCache;
3483+
3484+
beforeEach(() => {
3485+
Object.assign(OnyxUtils.getDeferredInitTask(), createDeferredTask());
3486+
cache = require('../../lib/OnyxCache').default;
3487+
Onyx.init({keys: ONYX_KEYS});
3488+
});
3489+
3490+
afterEach(() => {
3491+
jest.restoreAllMocks();
3492+
return Onyx.clear();
3493+
});
3494+
3495+
it('should preserve data from Onyx.update when a concurrent Onyx.merge fires before cache is written', async () => {
3496+
const member1 = `${ONYX_KEYS.COLLECTION.TEST_KEY}1`;
3497+
const member2 = `${ONYX_KEYS.COLLECTION.TEST_KEY}2`;
3498+
3499+
// Delay getItem for member1 to simulate slow Native storage (returns null before the write lands)
3500+
const getItemMock = StorageMock.getItem as jest.Mock;
3501+
const originalGetItem = getItemMock.getMockImplementation()!;
3502+
getItemMock.mockImplementation((key: OnyxKey) => {
3503+
if (key === member1) {
3504+
return new Promise<undefined>((resolve) => {
3505+
setTimeout(() => resolve(undefined), 50);
3506+
});
3507+
}
3508+
return originalGetItem(key);
3509+
});
3510+
3511+
// 2+ collection keys get batched into mergeCollectionWithPatches (deferred cache write)
3512+
const updatePromise = Onyx.update([
3513+
{onyxMethod: Onyx.METHOD.MERGE, key: member1, value: {isOptimistic: true, name: 'first'}},
3514+
{onyxMethod: Onyx.METHOD.MERGE, key: member2, value: {isOptimistic: true, name: 'second'}},
3515+
]);
3516+
3517+
// Concurrent merge fires before cache write — its get() hits the delayed storage mock
3518+
const mergePromise = Onyx.merge(member1, {lastVisitTime: '2025-01-01'});
3519+
3520+
await act(async () => {
3521+
await updatePromise;
3522+
await mergePromise;
3523+
await new Promise<void>((resolve) => {
3524+
setTimeout(resolve, 100);
3525+
});
3526+
});
3527+
3528+
const value = cache.get(member1);
3529+
expect(value).toHaveProperty('isOptimistic', true);
3530+
expect(value).toHaveProperty('lastVisitTime', '2025-01-01');
3531+
});
3532+
});

0 commit comments

Comments
 (0)