Skip to content

Commit 283159e

Browse files
committed
fix: process non-collection keys inline in multiSetWithRetry to preserve callback semantics
1 parent 6430168 commit 283159e

2 files changed

Lines changed: 53 additions & 8 deletions

File tree

lib/OnyxUtils.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1379,7 +1379,6 @@ function multiSetWithRetry(data: OnyxMultiSetInput, retryAttempt?: number): Prom
13791379
// collection, `partial` holds the new values being set and `previous` holds the cached values
13801380
// from before the set, which keysChanged() uses to skip subscribers whose value didn't change.
13811381
const collectionBatches = new Map<string, {partial: Record<string, OnyxValue<OnyxKey>>; previous: Record<string, OnyxValue<OnyxKey>>}>();
1382-
const nonCollectionPairs: Array<[string, OnyxValue<OnyxKey>]> = [];
13831382

13841383
for (const [key, value] of keyValuePairsToSet) {
13851384
// When we use multiSet to set a key we want to clear the current delta changes from Onyx.merge that were queued
@@ -1403,9 +1402,11 @@ function multiSetWithRetry(data: OnyxMultiSetInput, retryAttempt?: number): Prom
14031402
batch.partial[key] = value;
14041403
batch.previous[key] = previousValue;
14051404
} else {
1406-
// Non-collection keys are notified individually — no batching.
1405+
// Non-collection keys are notified inline (cache.set + keyChanged in iteration order)
1406+
// so re-entrant callbacks (e.g. Onyx.set inside a callback) see consistent cache
1407+
// and subscriber state, matching the original per-key notification semantics.
14071408
cache.set(key, value);
1408-
nonCollectionPairs.push([key, value]);
1409+
keyChanged(key, value);
14091410
}
14101411
}
14111412

@@ -1415,11 +1416,6 @@ function multiSetWithRetry(data: OnyxMultiSetInput, retryAttempt?: number): Prom
14151416
keysChanged(collectionKey as CollectionKeyBase, batch.partial, batch.previous);
14161417
}
14171418

1418-
// Non-collection keys go through the regular per-key notification path.
1419-
for (const [key, value] of nonCollectionPairs) {
1420-
keyChanged(key, value);
1421-
}
1422-
14231419
const keyValuePairsToStore = keyValuePairsToSet.filter((keyValuePair) => {
14241420
const [key] = keyValuePair;
14251421
// Filter out the RAM-only key value pairs, as they should not be saved to storage

tests/unit/onyxUtilsTest.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ const testMergeChanges: GenericDeepRecord[] = [
7474

7575
const ONYXKEYS = {
7676
TEST_KEY: 'test',
77+
TEST_KEY_2: 'test2',
7778
COLLECTION: {
7879
TEST_KEY: 'test_',
7980
TEST_LEVEL_KEY: 'test_level_',
@@ -467,6 +468,54 @@ describe('OnyxUtils', () => {
467468
// Despite 3 changed members, callback should fire at most once before disconnect stops it
468469
expect(callback).toHaveBeenCalledTimes(1);
469470
});
471+
472+
it('should keep cache and subscriber state consistent when a non-collection callback writes to another payload key', async () => {
473+
// A subscriber for keyA synchronously calls Onyx.set() on keyB during its callback.
474+
// After multiSet completes, the cache must reflect the multiSet's value for keyB
475+
// (multiSet wins), and the keyB subscriber's last seen value must equal the cache.
476+
await Onyx.multiSet({[ONYXKEYS.TEST_KEY]: 'initialA', [ONYXKEYS.TEST_KEY_2]: 'initialB'});
477+
478+
const callbackA = jest.fn((value: unknown) => {
479+
if (value !== 'newA') {
480+
return;
481+
}
482+
483+
// While processing the new value of keyA, write to keyB.
484+
// keyB is later in the same multiSet payload — multiSet should win.
485+
Onyx.set(ONYXKEYS.TEST_KEY_2, 'callbackB');
486+
});
487+
const callbackB = jest.fn();
488+
489+
const connA = Onyx.connect({
490+
key: ONYXKEYS.TEST_KEY,
491+
callback: callbackA,
492+
initWithStoredValues: false,
493+
});
494+
const connB = Onyx.connect({
495+
key: ONYXKEYS.TEST_KEY_2,
496+
callback: callbackB,
497+
initWithStoredValues: false,
498+
});
499+
await waitForPromisesToResolve();
500+
callbackA.mockClear();
501+
callbackB.mockClear();
502+
503+
await Onyx.multiSet({
504+
[ONYXKEYS.TEST_KEY]: 'newA',
505+
[ONYXKEYS.TEST_KEY_2]: 'multiSetB',
506+
});
507+
508+
// Cache reflects multiSet's payload value for keyB (the multiSet's later cache.set wins)
509+
expect(OnyxCache.get(ONYXKEYS.TEST_KEY_2)).toBe('multiSetB');
510+
511+
expect(callbackB.mock.calls.length).toBe(2);
512+
expect(callbackB.mock.calls.at(0)?.[0]).toBe('callbackB');
513+
// keyB subscriber's last received value matches the cache (no stale callback)
514+
expect(callbackB.mock.calls.at(1)?.[0]).toBe('multiSetB');
515+
516+
Onyx.disconnect(connA);
517+
Onyx.disconnect(connB);
518+
});
470519
});
471520

472521
describe('keysChanged', () => {

0 commit comments

Comments
 (0)