Skip to content

Commit c16605d

Browse files
committed
Detect member removals in snapshot rebuild when add and remove happen simultaneously
1 parent c1dbb9d commit c16605d

2 files changed

Lines changed: 28 additions & 6 deletions

File tree

lib/OnyxCache.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,6 @@ class OnyxCache {
475475

476476
const members: NonUndefined<OnyxCollection<KeyValueMapping[OnyxKey]>> = {};
477477
let hasMemberChanges = false;
478-
let newMemberCount = 0;
479478

480479
// Use the indexed forward lookup for O(collectionMembers) iteration.
481480
// Falls back to scanning all storageKeys if the index isn't populated yet.
@@ -492,7 +491,6 @@ class OnyxCache {
492491
// and should not be included in the frozen collection snapshot.
493492
if (val !== undefined && val !== null) {
494493
members[key] = val;
495-
newMemberCount++;
496494

497495
// Check if this member's reference changed from the old snapshot
498496
if (!hasMemberChanges && (!previousSnapshot || previousSnapshot[key] !== val)) {
@@ -501,11 +499,16 @@ class OnyxCache {
501499
}
502500
}
503501

504-
// Check if any members were removed (old snapshot had more keys)
502+
// Check if any members were removed from the previous snapshot.
503+
// We can't rely on count comparison alone — if one key is removed and another added,
504+
// the counts match but the snapshot content is different.
505505
if (!hasMemberChanges && previousSnapshot) {
506-
const oldMemberCount = Object.keys(previousSnapshot).length;
507-
if (oldMemberCount !== newMemberCount) {
508-
hasMemberChanges = true;
506+
// eslint-disable-next-line no-restricted-syntax
507+
for (const key in previousSnapshot) {
508+
if (!(key in members)) {
509+
hasMemberChanges = true;
510+
break;
511+
}
509512
}
510513
}
511514

tests/unit/onyxCacheTest.tsx

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -820,6 +820,25 @@ describe('Onyx', () => {
820820
expect(result).toBeUndefined();
821821
});
822822

823+
it('should return a new reference when a member is removed and another added simultaneously', async () => {
824+
await initOnyx();
825+
await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1});
826+
await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}2`, {id: 2});
827+
828+
const before = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION);
829+
830+
// Remove member 1 and add member 3 — count stays the same (2) but content changed
831+
await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, null);
832+
await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}3`, {id: 3});
833+
const after = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION);
834+
835+
expect(before).not.toBe(after);
836+
expect(after).toEqual({
837+
[`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}2`]: {id: 2},
838+
[`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}3`]: {id: 3},
839+
});
840+
});
841+
823842
it('should preserve unchanged member references when a sibling is updated', async () => {
824843
await initOnyx();
825844
const member1Value = {id: 1, name: 'unchanged'};

0 commit comments

Comments
 (0)