Skip to content

Commit e6e917b

Browse files
committed
fix: re-check subscription state per-key in keysChanged to handle self-disconnect
1 parent f36906c commit e6e917b

2 files changed

Lines changed: 44 additions & 2 deletions

File tree

lib/OnyxUtils.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -588,12 +588,20 @@ function keysChanged<TKey extends CollectionKeyBase>(
588588
continue;
589589
}
590590

591-
// Not using waitForCollectionCallback — notify per changed key
591+
// Not using waitForCollectionCallback — notify per changed key.
592+
// Re-check the subscription on each iteration because the callback may
593+
// synchronously disconnect itself (removing it from callbackToStateMapping),
594+
// in which case we must stop firing further callbacks for this subscriber.
592595
for (const dataKey of changedMemberKeys) {
596+
const currentSubscriber = callbackToStateMapping[subID];
597+
if (!currentSubscriber || typeof currentSubscriber.callback !== 'function') {
598+
break;
599+
}
593600
if (cachedCollection[dataKey] === previousCollection[dataKey]) {
594601
continue;
595602
}
596-
subscriber.callback(cachedCollection[dataKey], dataKey);
603+
const currentSubscriberCallback = currentSubscriber.callback as DefaultConnectCallback<TKey>;
604+
currentSubscriberCallback(cachedCollection[dataKey], dataKey as TKey);
597605
}
598606
} catch (error) {
599607
Logger.logAlert(`[OnyxUtils.keysChanged] Subscriber callback threw an error for key '${collectionKey}': ${error}`);

tests/unit/onyxUtilsTest.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,40 @@ describe('OnyxUtils', () => {
433433
Onyx.disconnect(conn1);
434434
Onyx.disconnect(conn2);
435435
});
436+
437+
it('should stop firing callbacks for a collection subscriber that disconnects itself mid-batch', async () => {
438+
// A collection subscriber (waitForCollectionCallback=false) disconnects itself when
439+
// it receives the first member. Subsequent changed members in the same batch must NOT
440+
// trigger further callbacks for this subscriber.
441+
const callback = jest.fn();
442+
let connection: ReturnType<typeof Onyx.connect>;
443+
444+
callback.mockImplementation(() => {
445+
if (!connection) {
446+
return;
447+
}
448+
449+
Onyx.disconnect(connection);
450+
});
451+
452+
connection = Onyx.connect({
453+
key: ONYXKEYS.COLLECTION.TEST_KEY,
454+
callback,
455+
waitForCollectionCallback: false,
456+
initWithStoredValues: false,
457+
});
458+
await waitForPromisesToResolve();
459+
callback.mockClear();
460+
461+
await Onyx.multiSet({
462+
[`${ONYXKEYS.COLLECTION.TEST_KEY}1`]: {id: 1},
463+
[`${ONYXKEYS.COLLECTION.TEST_KEY}2`]: {id: 2},
464+
[`${ONYXKEYS.COLLECTION.TEST_KEY}3`]: {id: 3},
465+
});
466+
467+
// Despite 3 changed members, callback should fire at most once before disconnect stops it
468+
expect(callback).toHaveBeenCalledTimes(1);
469+
});
436470
});
437471

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

0 commit comments

Comments
 (0)