From 217e7c3aae9a7b00460bb5bc96d017fad0de2508 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 11 Jun 2026 14:48:04 +0100 Subject: [PATCH 1/8] Add OnyxStore as a standalone subscription registry (inert) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce lib/OnyxStore.ts: a single listener registry (keyListeners Map>) with subscribe / notifyKey / notifyCollection / getState / hasListenersForKey / clearAll. Built on the existing structural-sharing cache (cache.getCollectionData frozen snapshots). This module is inert — nothing imports it yet. The subscription and notification paths (Onyx.connect, useOnyx, OnyxUtils.notify*) are wired onto it in a later change. Adding it alone has zero behavioral impact. Includes tests/unit/OnyxStoreTest.ts (20 tests) covering exact-key and collection-snapshot routing, ref-equality member skips, hasListenersForKey, clearAll, and listener error isolation. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/OnyxStore.ts | 186 +++++++++++++++++++++++++ tests/unit/OnyxStoreTest.ts | 270 ++++++++++++++++++++++++++++++++++++ 2 files changed, 456 insertions(+) create mode 100644 lib/OnyxStore.ts create mode 100644 tests/unit/OnyxStoreTest.ts diff --git a/lib/OnyxStore.ts b/lib/OnyxStore.ts new file mode 100644 index 000000000..449c0751f --- /dev/null +++ b/lib/OnyxStore.ts @@ -0,0 +1,186 @@ +import cache from './OnyxCache'; +import OnyxKeys from './OnyxKeys'; +import * as Logger from './Logger'; +import type {CollectionKeyBase, KeyValueMapping, OnyxCollection, OnyxKey, OnyxValue} from './types'; + +/** + * Listener fired when an exact key's value changes. For collection root keys this is the + * snapshot-mode listener: receives the frozen collection snapshot every time a member changes. + */ +type KeyListener = (value: OnyxValue, key: TKey) => void; + +/** + * `OnyxStore` is a single listener registry for Onyx reads/subscriptions. It replaces the + * connection manager's several per-subscription bookkeeping structures with one index: + * + * keyListeners — listeners on an exact key (a single key, a collection root in snapshot + * mode, or a specific collection member). + * + * Write paths call `notifyKey()` (single-key write) or `notifyCollection()` (batch collection + * update from `mergeCollection`/`setCollection`/`clear`). + * + * NOTE: This module is introduced inert — nothing calls it yet. The subscription/notification + * paths (`Onyx.connect`, `useOnyx`, `OnyxUtils.notify*`) are wired onto it in a later change. + */ +class OnyxStore { + private keyListeners: Map>; + + constructor() { + this.keyListeners = new Map(); + } + + /** + * Sync, cache-only read. Returns the frozen collection snapshot for collection + * keys, the cached value for single keys, or `undefined` if not in cache. + */ + getState(key: TKey): OnyxValue { + if (OnyxKeys.isCollectionKey(key)) { + return cache.getCollectionData(key) as OnyxValue; + } + return cache.get(key) as OnyxValue; + } + + /** + * Subscribe to an exact key. For collection root keys this is "snapshot mode" — + * the listener fires with the frozen collection snapshot whenever any member + * changes. For collection member keys or regular keys, the listener fires when + * that specific key's value changes. + * + * Returns an unsubscribe function. + */ + subscribe(key: TKey, listener: KeyListener): () => void { + let listeners = this.keyListeners.get(key); + if (!listeners) { + listeners = new Set(); + this.keyListeners.set(key, listeners); + } + listeners.add(listener as unknown as KeyListener); + return () => { + const set = this.keyListeners.get(key); + if (!set) { + return; + } + set.delete(listener as unknown as KeyListener); + if (set.size === 0) { + this.keyListeners.delete(key); + } + }; + } + + /** + * Notify of a single-key write. + * + * Dispatch: + * 1. keyListeners.get(key) — exact-key subscribers (always fires) + * 2. If key is a collection member: keyListeners.get(collectionKey) — snapshot + * subscribers for the parent collection (unless suppressed). + * + * `options.suppressCollectionSnapshot` skips step 2 — used by collection-batch + * write paths so each member-write doesn't re-trigger the collection-level + * snapshot listeners; the outer `notifyCollection()` fires those once. + */ + notifyKey(key: TKey, value: OnyxValue, options?: {suppressCollectionSnapshot?: boolean}): void { + // 1. Exact-key listeners + const exact = this.keyListeners.get(key); + if (exact && exact.size > 0) { + for (const listener of exact) { + this.safeInvoke(() => listener(value as OnyxValue, key), key); + } + } + + // 2. Collection-level snapshot routing — only fires when the write is to a member key. + // Direct writes to a collection root (e.g. `Onyx.merge(COLLECTION_KEY, ...)`) are + // an unsupported anti-pattern — treat them as opaque single-key writes. + const collectionKey = OnyxKeys.getCollectionKey(key); + const isCollectionMemberWrite = collectionKey !== undefined && collectionKey !== key; + if (isCollectionMemberWrite && !options?.suppressCollectionSnapshot) { + const snapshotListeners = this.keyListeners.get(collectionKey); + if (snapshotListeners && snapshotListeners.size > 0) { + const snapshot = cache.getCollectionData(collectionKey); + for (const listener of snapshotListeners) { + this.safeInvoke(() => listener(snapshot as OnyxValue, collectionKey), collectionKey); + } + } + } + } + + /** + * Notify of a collection-level batch update. Used by `mergeCollection`, + * `setCollection`, and `clear`'s collection path. + * + * Dispatch: + * 1. keyListeners.get(collectionKey) — fires ONCE with the new snapshot. + * 2. keyListeners.get(memberKey) — fires per changed member where the value + * differs from the previous (for ref-equality on unchanged members). + */ + notifyCollection( + collectionKey: TKey, + partialCollection: OnyxCollection, + partialPreviousCollection?: OnyxCollection, + ): void { + const changedKeys = Object.keys(partialCollection ?? {}); + if (changedKeys.length === 0) { + return; + } + const previous = partialPreviousCollection ?? {}; + + // Read the merged snapshot once. `cache.getCollectionData()` returns the post-merge + // frozen object, which is what listeners should see (not the raw `partialCollection` + // input, which is just the delta and lacks fields preserved during merge). + const snapshot = cache.getCollectionData(collectionKey); + + // 1. Snapshot subscribers fire once with the new snapshot. + const snapshotListeners = this.keyListeners.get(collectionKey); + if (snapshotListeners && snapshotListeners.size > 0) { + for (const listener of snapshotListeners) { + this.safeInvoke(() => listener(snapshot as OnyxValue, collectionKey), collectionKey); + } + } + + // 2. Exact-member subscribers fire per changed key (skip if ref unchanged vs previous). + for (const memberKey of changedKeys) { + const value = snapshot?.[memberKey]; + const prev = previous[memberKey]; + if (value === prev) { + continue; + } + const exact = this.keyListeners.get(memberKey); + if (!exact || exact.size === 0) { + continue; + } + for (const listener of exact) { + this.safeInvoke(() => listener(value as OnyxValue, memberKey), memberKey); + } + } + } + + /** Wipe all subscriptions. Used by tests and `Onyx.clear()` follow-on. */ + clearAll(): void { + this.keyListeners.clear(); + } + + /** True if there are any subscribers for the given key (exact or parent collection). */ + hasListenersForKey(key: OnyxKey): boolean { + if ((this.keyListeners.get(key)?.size ?? 0) > 0) { + return true; + } + const collectionKey = OnyxKeys.getCollectionKey(key); + if (collectionKey && collectionKey !== key && (this.keyListeners.get(collectionKey)?.size ?? 0) > 0) { + return true; + } + return false; + } + + private safeInvoke(fn: () => void, contextKey: OnyxKey): void { + try { + fn(); + } catch (error) { + Logger.logAlert(`[OnyxStore] Listener threw an error for key '${contextKey}': ${error}`); + } + } +} + +const onyxStore = new OnyxStore(); + +export default onyxStore; +export type {KeyListener}; diff --git a/tests/unit/OnyxStoreTest.ts b/tests/unit/OnyxStoreTest.ts new file mode 100644 index 000000000..e1eb3d1b3 --- /dev/null +++ b/tests/unit/OnyxStoreTest.ts @@ -0,0 +1,270 @@ +import type {OnyxKey} from '../../lib'; +import Onyx from '../../lib'; +import onyxStore from '../../lib/OnyxStore'; +import cache from '../../lib/OnyxCache'; +import * as Logger from '../../lib/Logger'; + +// We need access to some internal properties of `onyxStore` during the tests but they are private, +// so this workaround allows us to have access to them. The maps are created once in the constructor +// and only ever `.clear()`ed (never reassigned), so capturing the references here stays valid. +// eslint-disable-next-line dot-notation +const keyListeners = onyxStore['keyListeners']; + +const ONYXKEYS = { + TEST_KEY: 'test', + OTHER_TEST: 'otherTest', + COLLECTION: { + TEST_KEY: 'test_', + }, +}; + +const COLLECTION = ONYXKEYS.COLLECTION.TEST_KEY; +const MEMBER_1 = `${COLLECTION}1`; +const MEMBER_2 = `${COLLECTION}2`; + +Onyx.init({ + keys: ONYXKEYS, +}); + +beforeEach(() => Onyx.clear()); + +describe('OnyxStore', () => { + // Always start from a clean registry. + beforeEach(() => { + onyxStore.clearAll(); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe('subscribe / notifyKey', () => { + it('should fire the listener with (value, key) on notifyKey', () => { + const callback = jest.fn(); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback); + + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'hello'); + + expect(callback).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenCalledWith('hello', ONYXKEYS.TEST_KEY); + }); + + it('should fire all listeners registered on the same key', () => { + const callback1 = jest.fn(); + const callback2 = jest.fn(); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback1); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback2); + + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 1); + + expect(callback1).toHaveBeenCalledTimes(1); + expect(callback2).toHaveBeenCalledTimes(1); + }); + + it('should not fire the listener after it unsubscribes', () => { + const callback = jest.fn(); + const unsubscribe = onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback); + + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'first'); + unsubscribe(); + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'second'); + + expect(callback).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenLastCalledWith('first', ONYXKEYS.TEST_KEY); + }); + + it('should only unsubscribe the specific listener, leaving others intact', () => { + const callback1 = jest.fn(); + const callback2 = jest.fn(); + const unsubscribe1 = onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback1); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback2); + + unsubscribe1(); + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 1); + + expect(callback1).not.toHaveBeenCalled(); + expect(callback2).toHaveBeenCalledTimes(1); + }); + + it('should delete the key entry from the internal map once the last listener unsubscribes', () => { + const unsubscribe = onyxStore.subscribe(ONYXKEYS.TEST_KEY, jest.fn()); + expect(keyListeners.has(ONYXKEYS.TEST_KEY)).toBeTruthy(); + + unsubscribe(); + + expect(keyListeners.has(ONYXKEYS.TEST_KEY)).toBeFalsy(); + }); + + it('should be a no-op to notify a key with no listeners', () => { + expect(() => onyxStore.notifyKey('keyWithNoListeners' as OnyxKey, 'x')).not.toThrow(); + }); + + it('should be idempotent when unsubscribing more than once', () => { + const callback = jest.fn(); + const unsubscribe = onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback); + + unsubscribe(); + expect(() => unsubscribe()).not.toThrow(); + + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 1); + expect(callback).not.toHaveBeenCalled(); + }); + }); + + describe('collection-snapshot routing on notifyKey', () => { + it('should fire the collection-root snapshot listener with the cache snapshot when a member is written', () => { + const snapshot = {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}; + const getCollectionData = jest.spyOn(cache, 'getCollectionData').mockReturnValue(snapshot); + + const callback = jest.fn(); + onyxStore.subscribe(COLLECTION, callback); + + onyxStore.notifyKey(MEMBER_1, {id: 1}); + + expect(getCollectionData).toHaveBeenCalledWith(COLLECTION); + expect(callback).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenCalledWith(snapshot, COLLECTION); + }); + + it('should fire both the exact-member listener and the collection-root snapshot listener', () => { + const snapshot = {[MEMBER_1]: {id: 1}}; + jest.spyOn(cache, 'getCollectionData').mockReturnValue(snapshot); + + const memberCallback = jest.fn(); + const snapshotCallback = jest.fn(); + onyxStore.subscribe(MEMBER_1, memberCallback); + onyxStore.subscribe(COLLECTION, snapshotCallback); + + onyxStore.notifyKey(MEMBER_1, {id: 1}); + + expect(memberCallback).toHaveBeenCalledWith({id: 1}, MEMBER_1); + expect(snapshotCallback).toHaveBeenCalledWith(snapshot, COLLECTION); + }); + + it('should skip the collection-root listener but still fire the exact-member listener when suppressCollectionSnapshot is set', () => { + const getCollectionData = jest.spyOn(cache, 'getCollectionData').mockReturnValue({}); + + const memberCallback = jest.fn(); + const snapshotCallback = jest.fn(); + onyxStore.subscribe(MEMBER_1, memberCallback); + onyxStore.subscribe(COLLECTION, snapshotCallback); + + onyxStore.notifyKey(MEMBER_1, {id: 1}, {suppressCollectionSnapshot: true}); + + expect(memberCallback).toHaveBeenCalledTimes(1); + expect(snapshotCallback).not.toHaveBeenCalled(); + // The snapshot is never read when suppressed. + expect(getCollectionData).not.toHaveBeenCalled(); + }); + + it('should not perform collection routing for a non-member single key', () => { + const getCollectionData = jest.spyOn(cache, 'getCollectionData'); + const callback = jest.fn(); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback); + + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'x'); + + expect(callback).toHaveBeenCalledTimes(1); + expect(getCollectionData).not.toHaveBeenCalled(); + }); + }); + + describe('notifyCollection', () => { + it('should fire the snapshot listener once with the cache snapshot', () => { + const snapshot = {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}; + jest.spyOn(cache, 'getCollectionData').mockReturnValue(snapshot); + + const callback = jest.fn(); + onyxStore.subscribe(COLLECTION, callback); + + onyxStore.notifyCollection(COLLECTION, {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}); + + expect(callback).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenCalledWith(snapshot, COLLECTION); + }); + + it('should fire exact-member listeners only for members whose value reference changed', () => { + const shared = {id: 2}; // same reference in snapshot and previous → should be skipped + const snapshot = {[MEMBER_1]: {id: 1}, [MEMBER_2]: shared}; + jest.spyOn(cache, 'getCollectionData').mockReturnValue(snapshot); + + const member1Callback = jest.fn(); + const member2Callback = jest.fn(); + onyxStore.subscribe(MEMBER_1, member1Callback); + onyxStore.subscribe(MEMBER_2, member2Callback); + + onyxStore.notifyCollection( + COLLECTION, + {[MEMBER_1]: {id: 1}, [MEMBER_2]: shared}, + {[MEMBER_2]: shared}, // previous: member 2 unchanged by reference + ); + + expect(member1Callback).toHaveBeenCalledWith({id: 1}, MEMBER_1); + expect(member2Callback).not.toHaveBeenCalled(); + }); + + it('should be a no-op when the partial collection is empty', () => { + const callback = jest.fn(); + onyxStore.subscribe(COLLECTION, callback); + + onyxStore.notifyCollection(COLLECTION, {}); + + expect(callback).not.toHaveBeenCalled(); + }); + }); + + describe('hasListenersForKey', () => { + it('should return true for an exact-key subscriber', () => { + onyxStore.subscribe(ONYXKEYS.TEST_KEY, jest.fn()); + expect(onyxStore.hasListenersForKey(ONYXKEYS.TEST_KEY)).toBeTruthy(); + }); + + it('should return true for a member key when its parent collection has a subscriber', () => { + onyxStore.subscribe(COLLECTION, jest.fn()); + expect(onyxStore.hasListenersForKey(MEMBER_1)).toBeTruthy(); + }); + + it('should return false when there are no relevant subscribers', () => { + expect(onyxStore.hasListenersForKey('someUnwatchedKey')).toBeFalsy(); + }); + + it('should return false after the last listener unsubscribes', () => { + const unsubscribe = onyxStore.subscribe(ONYXKEYS.TEST_KEY, jest.fn()); + unsubscribe(); + expect(onyxStore.hasListenersForKey(ONYXKEYS.TEST_KEY)).toBeFalsy(); + }); + }); + + describe('clearAll', () => { + it('should wipe key and collection subscriptions', () => { + const keyCallback = jest.fn(); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, keyCallback); + + onyxStore.clearAll(); + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'x'); + + expect(keyCallback).not.toHaveBeenCalled(); + expect(onyxStore.hasListenersForKey(ONYXKEYS.TEST_KEY)).toBeFalsy(); + }); + }); + + describe('listener error isolation', () => { + it('should log a throwing listener and still fire the other listeners', () => { + const logAlertSpy = jest.spyOn(Logger, 'logAlert').mockImplementation(() => { + /* empty */ + }); + const throwingCallback = jest.fn(() => { + throw new Error('boom'); + }); + const healthyCallback = jest.fn(); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, throwingCallback); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, healthyCallback); + + expect(() => onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'x')).not.toThrow(); + + expect(throwingCallback).toHaveBeenCalledTimes(1); + expect(healthyCallback).toHaveBeenCalledTimes(1); + expect(logAlertSpy).toHaveBeenCalled(); + }); + }); +}); From 4f99ddf0151e88a4603b8719a6ffb659afea88cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 28 Aug 2026 16:59:41 +0100 Subject: [PATCH 2/8] Simplify comments and remove snapshot wording --- lib/OnyxStore.ts | 78 ++++++++++++++++++------------------- tests/unit/OnyxStoreTest.ts | 48 +++++++++++------------ 2 files changed, 61 insertions(+), 65 deletions(-) diff --git a/lib/OnyxStore.ts b/lib/OnyxStore.ts index 449c0751f..e1cd33ceb 100644 --- a/lib/OnyxStore.ts +++ b/lib/OnyxStore.ts @@ -4,23 +4,20 @@ import * as Logger from './Logger'; import type {CollectionKeyBase, KeyValueMapping, OnyxCollection, OnyxKey, OnyxValue} from './types'; /** - * Listener fired when an exact key's value changes. For collection root keys this is the - * snapshot-mode listener: receives the frozen collection snapshot every time a member changes. + * Listener fired when an exact key's value changes. For a collection root key this is the + * collection listener: it receives the frozen collection object every time a member changes. */ type KeyListener = (value: OnyxValue, key: TKey) => void; /** - * `OnyxStore` is a single listener registry for Onyx reads/subscriptions. It replaces the - * connection manager's several per-subscription bookkeeping structures with one index: + * `OnyxStore` is a single listener registry for Onyx reads/subscriptions. One index backs + * every subscription: * - * keyListeners — listeners on an exact key (a single key, a collection root in snapshot - * mode, or a specific collection member). + * keyListeners: exact-key listeners (a single key, a collection root in collection mode, + * or a specific collection member). * * Write paths call `notifyKey()` (single-key write) or `notifyCollection()` (batch collection * update from `mergeCollection`/`setCollection`/`clear`). - * - * NOTE: This module is introduced inert — nothing calls it yet. The subscription/notification - * paths (`Onyx.connect`, `useOnyx`, `OnyxUtils.notify*`) are wired onto it in a later change. */ class OnyxStore { private keyListeners: Map>; @@ -30,7 +27,7 @@ class OnyxStore { } /** - * Sync, cache-only read. Returns the frozen collection snapshot for collection + * Sync, cache-only read. Returns the frozen collection object for collection * keys, the cached value for single keys, or `undefined` if not in cache. */ getState(key: TKey): OnyxValue { @@ -41,10 +38,9 @@ class OnyxStore { } /** - * Subscribe to an exact key. For collection root keys this is "snapshot mode" — - * the listener fires with the frozen collection snapshot whenever any member - * changes. For collection member keys or regular keys, the listener fires when - * that specific key's value changes. + * Subscribe to an exact key. For a collection root key this is "collection mode": the + * listener fires with the frozen collection object whenever any member changes. For a + * collection member key or a regular key, the listener fires when that key's value changes. * * Returns an unsubscribe function. */ @@ -71,15 +67,15 @@ class OnyxStore { * Notify of a single-key write. * * Dispatch: - * 1. keyListeners.get(key) — exact-key subscribers (always fires) - * 2. If key is a collection member: keyListeners.get(collectionKey) — snapshot - * subscribers for the parent collection (unless suppressed). + * 1. keyListeners.get(key): exact-key subscribers (always fires). + * 2. If key is a collection member, keyListeners.get(collectionKey): collection + * listeners for the parent collection (unless suppressed). * - * `options.suppressCollectionSnapshot` skips step 2 — used by collection-batch - * write paths so each member-write doesn't re-trigger the collection-level - * snapshot listeners; the outer `notifyCollection()` fires those once. + * `options.suppressCollectionNotify` skips step 2. Collection-batch write paths set + * it so each member write doesn't re-trigger the collection-level listeners; + * the outer `notifyCollection()` fires those once. */ - notifyKey(key: TKey, value: OnyxValue, options?: {suppressCollectionSnapshot?: boolean}): void { + notifyKey(key: TKey, value: OnyxValue, options?: {suppressCollectionNotify?: boolean}): void { // 1. Exact-key listeners const exact = this.keyListeners.get(key); if (exact && exact.size > 0) { @@ -88,17 +84,17 @@ class OnyxStore { } } - // 2. Collection-level snapshot routing — only fires when the write is to a member key. - // Direct writes to a collection root (e.g. `Onyx.merge(COLLECTION_KEY, ...)`) are - // an unsupported anti-pattern — treat them as opaque single-key writes. + // 2. Collection-level routing. Only fires when the write is to a member key. + // Direct writes to a collection root (e.g. `Onyx.merge(COLLECTION_KEY, ...)`) are an + // unsupported anti-pattern; treat them as opaque single-key writes. const collectionKey = OnyxKeys.getCollectionKey(key); const isCollectionMemberWrite = collectionKey !== undefined && collectionKey !== key; - if (isCollectionMemberWrite && !options?.suppressCollectionSnapshot) { - const snapshotListeners = this.keyListeners.get(collectionKey); - if (snapshotListeners && snapshotListeners.size > 0) { - const snapshot = cache.getCollectionData(collectionKey); - for (const listener of snapshotListeners) { - this.safeInvoke(() => listener(snapshot as OnyxValue, collectionKey), collectionKey); + if (isCollectionMemberWrite && !options?.suppressCollectionNotify) { + const collectionListeners = this.keyListeners.get(collectionKey); + if (collectionListeners && collectionListeners.size > 0) { + const collectionData = cache.getCollectionData(collectionKey); + for (const listener of collectionListeners) { + this.safeInvoke(() => listener(collectionData as OnyxValue, collectionKey), collectionKey); } } } @@ -109,9 +105,9 @@ class OnyxStore { * `setCollection`, and `clear`'s collection path. * * Dispatch: - * 1. keyListeners.get(collectionKey) — fires ONCE with the new snapshot. - * 2. keyListeners.get(memberKey) — fires per changed member where the value - * differs from the previous (for ref-equality on unchanged members). + * 1. keyListeners.get(collectionKey): fires once with the new collection object. + * 2. keyListeners.get(memberKey): fires per changed member whose value differs from + * the previous, preserving ref-equality on unchanged members. */ notifyCollection( collectionKey: TKey, @@ -124,22 +120,22 @@ class OnyxStore { } const previous = partialPreviousCollection ?? {}; - // Read the merged snapshot once. `cache.getCollectionData()` returns the post-merge + // Read the merged collection once. `cache.getCollectionData()` returns the post-merge // frozen object, which is what listeners should see (not the raw `partialCollection` // input, which is just the delta and lacks fields preserved during merge). - const snapshot = cache.getCollectionData(collectionKey); + const collectionData = cache.getCollectionData(collectionKey); - // 1. Snapshot subscribers fire once with the new snapshot. - const snapshotListeners = this.keyListeners.get(collectionKey); - if (snapshotListeners && snapshotListeners.size > 0) { - for (const listener of snapshotListeners) { - this.safeInvoke(() => listener(snapshot as OnyxValue, collectionKey), collectionKey); + // 1. Collection listeners fire once with the new collection object. + const collectionListeners = this.keyListeners.get(collectionKey); + if (collectionListeners && collectionListeners.size > 0) { + for (const listener of collectionListeners) { + this.safeInvoke(() => listener(collectionData as OnyxValue, collectionKey), collectionKey); } } // 2. Exact-member subscribers fire per changed key (skip if ref unchanged vs previous). for (const memberKey of changedKeys) { - const value = snapshot?.[memberKey]; + const value = collectionData?.[memberKey]; const prev = previous[memberKey]; if (value === prev) { continue; diff --git a/tests/unit/OnyxStoreTest.ts b/tests/unit/OnyxStoreTest.ts index e1eb3d1b3..be54297c1 100644 --- a/tests/unit/OnyxStoreTest.ts +++ b/tests/unit/OnyxStoreTest.ts @@ -111,10 +111,10 @@ describe('OnyxStore', () => { }); }); - describe('collection-snapshot routing on notifyKey', () => { - it('should fire the collection-root snapshot listener with the cache snapshot when a member is written', () => { - const snapshot = {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}; - const getCollectionData = jest.spyOn(cache, 'getCollectionData').mockReturnValue(snapshot); + describe('collection routing on notifyKey', () => { + it('should fire the collection-root listener with the cache collection object when a member is written', () => { + const collectionData = {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}; + const getCollectionData = jest.spyOn(cache, 'getCollectionData').mockReturnValue(collectionData); const callback = jest.fn(); onyxStore.subscribe(COLLECTION, callback); @@ -123,37 +123,37 @@ describe('OnyxStore', () => { expect(getCollectionData).toHaveBeenCalledWith(COLLECTION); expect(callback).toHaveBeenCalledTimes(1); - expect(callback).toHaveBeenCalledWith(snapshot, COLLECTION); + expect(callback).toHaveBeenCalledWith(collectionData, COLLECTION); }); - it('should fire both the exact-member listener and the collection-root snapshot listener', () => { - const snapshot = {[MEMBER_1]: {id: 1}}; - jest.spyOn(cache, 'getCollectionData').mockReturnValue(snapshot); + it('should fire both the exact-member listener and the collection-root listener', () => { + const collectionData = {[MEMBER_1]: {id: 1}}; + jest.spyOn(cache, 'getCollectionData').mockReturnValue(collectionData); const memberCallback = jest.fn(); - const snapshotCallback = jest.fn(); + const collectionCallback = jest.fn(); onyxStore.subscribe(MEMBER_1, memberCallback); - onyxStore.subscribe(COLLECTION, snapshotCallback); + onyxStore.subscribe(COLLECTION, collectionCallback); onyxStore.notifyKey(MEMBER_1, {id: 1}); expect(memberCallback).toHaveBeenCalledWith({id: 1}, MEMBER_1); - expect(snapshotCallback).toHaveBeenCalledWith(snapshot, COLLECTION); + expect(collectionCallback).toHaveBeenCalledWith(collectionData, COLLECTION); }); - it('should skip the collection-root listener but still fire the exact-member listener when suppressCollectionSnapshot is set', () => { + it('should skip the collection-root listener but still fire the exact-member listener when suppressCollectionNotify is set', () => { const getCollectionData = jest.spyOn(cache, 'getCollectionData').mockReturnValue({}); const memberCallback = jest.fn(); - const snapshotCallback = jest.fn(); + const collectionCallback = jest.fn(); onyxStore.subscribe(MEMBER_1, memberCallback); - onyxStore.subscribe(COLLECTION, snapshotCallback); + onyxStore.subscribe(COLLECTION, collectionCallback); - onyxStore.notifyKey(MEMBER_1, {id: 1}, {suppressCollectionSnapshot: true}); + onyxStore.notifyKey(MEMBER_1, {id: 1}, {suppressCollectionNotify: true}); expect(memberCallback).toHaveBeenCalledTimes(1); - expect(snapshotCallback).not.toHaveBeenCalled(); - // The snapshot is never read when suppressed. + expect(collectionCallback).not.toHaveBeenCalled(); + // The collection object is never read when suppressed. expect(getCollectionData).not.toHaveBeenCalled(); }); @@ -170,9 +170,9 @@ describe('OnyxStore', () => { }); describe('notifyCollection', () => { - it('should fire the snapshot listener once with the cache snapshot', () => { - const snapshot = {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}; - jest.spyOn(cache, 'getCollectionData').mockReturnValue(snapshot); + it('should fire the collection listener once with the cache collection object', () => { + const collectionData = {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}; + jest.spyOn(cache, 'getCollectionData').mockReturnValue(collectionData); const callback = jest.fn(); onyxStore.subscribe(COLLECTION, callback); @@ -180,13 +180,13 @@ describe('OnyxStore', () => { onyxStore.notifyCollection(COLLECTION, {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}); expect(callback).toHaveBeenCalledTimes(1); - expect(callback).toHaveBeenCalledWith(snapshot, COLLECTION); + expect(callback).toHaveBeenCalledWith(collectionData, COLLECTION); }); it('should fire exact-member listeners only for members whose value reference changed', () => { - const shared = {id: 2}; // same reference in snapshot and previous → should be skipped - const snapshot = {[MEMBER_1]: {id: 1}, [MEMBER_2]: shared}; - jest.spyOn(cache, 'getCollectionData').mockReturnValue(snapshot); + const shared = {id: 2}; // same reference in collection and previous, should be skipped + const collectionData = {[MEMBER_1]: {id: 1}, [MEMBER_2]: shared}; + jest.spyOn(cache, 'getCollectionData').mockReturnValue(collectionData); const member1Callback = jest.fn(); const member2Callback = jest.fn(); From 28179abee48eef566cb54eb8c9d5da4cbe502b5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 3 Sep 2026 16:12:49 +0100 Subject: [PATCH 3/8] Simplify comments and types --- lib/OnyxStore.ts | 72 +++++++++++++++++++++++++++++++----------------- 1 file changed, 47 insertions(+), 25 deletions(-) diff --git a/lib/OnyxStore.ts b/lib/OnyxStore.ts index e1cd33ceb..d9e14af8f 100644 --- a/lib/OnyxStore.ts +++ b/lib/OnyxStore.ts @@ -1,33 +1,45 @@ +import type {CollectionKeyBase, KeyValueMapping, OnyxCollection, OnyxKey, OnyxValue} from './types'; + +import * as Logger from './Logger'; import cache from './OnyxCache'; import OnyxKeys from './OnyxKeys'; -import * as Logger from './Logger'; -import type {CollectionKeyBase, KeyValueMapping, OnyxCollection, OnyxKey, OnyxValue} from './types'; /** - * Listener fired when an exact key's value changes. For a collection root key this is the - * collection listener: it receives the frozen collection object every time a member changes. + * Listener fired when an exact key's value changes. */ type KeyListener = (value: OnyxValue, key: TKey) => void; +/** + * Storage form of a listener, value erased so one Map can hold listeners for every key type. + */ +type StoredListener = (value: unknown, key: OnyxKey) => void; + +type NotifyKeyOptions = { + /** + * Skips collection-level routing. Collection-batch write paths set it so each member write + * doesn't re-trigger the collection-level listeners; the outer `notifyCollection()` fires those once. + */ + suppressCollectionNotify?: boolean; +}; + /** * `OnyxStore` is a single listener registry for Onyx reads/subscriptions. One index backs * every subscription: * - * keyListeners: exact-key listeners (a single key, a collection root in collection mode, + * keyListeners: exact-key listeners (a single key, a collection object, * or a specific collection member). * - * Write paths call `notifyKey()` (single-key write) or `notifyCollection()` (batch collection - * update from `mergeCollection`/`setCollection`/`clear`). + * Write paths call `notifyKey()` (single-key write) or `notifyCollection()` (batch collection update). */ class OnyxStore { - private keyListeners: Map>; + private keyListeners: Map>; constructor() { this.keyListeners = new Map(); } /** - * Sync, cache-only read. Returns the frozen collection object for collection + * Returns the frozen collection object for collection * keys, the cached value for single keys, or `undefined` if not in cache. */ getState(key: TKey): OnyxValue { @@ -50,13 +62,17 @@ class OnyxStore { listeners = new Set(); this.keyListeners.set(key, listeners); } - listeners.add(listener as unknown as KeyListener); + + listeners.add(listener as StoredListener); + return () => { const set = this.keyListeners.get(key); if (!set) { return; } - set.delete(listener as unknown as KeyListener); + + set.delete(listener as StoredListener); + if (set.size === 0) { this.keyListeners.delete(key); } @@ -69,18 +85,14 @@ class OnyxStore { * Dispatch: * 1. keyListeners.get(key): exact-key subscribers (always fires). * 2. If key is a collection member, keyListeners.get(collectionKey): collection - * listeners for the parent collection (unless suppressed). - * - * `options.suppressCollectionNotify` skips step 2. Collection-batch write paths set - * it so each member write doesn't re-trigger the collection-level listeners; - * the outer `notifyCollection()` fires those once. + * listeners for the parent collection (unless `options.suppressCollectionNotify`). */ - notifyKey(key: TKey, value: OnyxValue, options?: {suppressCollectionNotify?: boolean}): void { + notifyKey(key: TKey, value: OnyxValue, options?: NotifyKeyOptions): void { // 1. Exact-key listeners const exact = this.keyListeners.get(key); if (exact && exact.size > 0) { for (const listener of exact) { - this.safeInvoke(() => listener(value as OnyxValue, key), key); + this.safeInvoke(() => listener(value, key), key); } } @@ -94,15 +106,14 @@ class OnyxStore { if (collectionListeners && collectionListeners.size > 0) { const collectionData = cache.getCollectionData(collectionKey); for (const listener of collectionListeners) { - this.safeInvoke(() => listener(collectionData as OnyxValue, collectionKey), collectionKey); + this.safeInvoke(() => listener(collectionData, collectionKey), collectionKey); } } } } /** - * Notify of a collection-level batch update. Used by `mergeCollection`, - * `setCollection`, and `clear`'s collection path. + * Notify of a collection-level batch update. * * Dispatch: * 1. keyListeners.get(collectionKey): fires once with the new collection object. @@ -129,7 +140,7 @@ class OnyxStore { const collectionListeners = this.keyListeners.get(collectionKey); if (collectionListeners && collectionListeners.size > 0) { for (const listener of collectionListeners) { - this.safeInvoke(() => listener(collectionData as OnyxValue, collectionKey), collectionKey); + this.safeInvoke(() => listener(collectionData, collectionKey), collectionKey); } } @@ -140,33 +151,44 @@ class OnyxStore { if (value === prev) { continue; } + const exact = this.keyListeners.get(memberKey); if (!exact || exact.size === 0) { continue; } + for (const listener of exact) { - this.safeInvoke(() => listener(value as OnyxValue, memberKey), memberKey); + this.safeInvoke(() => listener(value, memberKey), memberKey); } } } - /** Wipe all subscriptions. Used by tests and `Onyx.clear()` follow-on. */ + /** + * Wipe all subscriptions. Used by tests and `Onyx.clear()` follow-on. + */ clearAll(): void { this.keyListeners.clear(); } - /** True if there are any subscribers for the given key (exact or parent collection). */ + /** + * True if there are any subscribers for the given key (exact or parent collection). + */ hasListenersForKey(key: OnyxKey): boolean { if ((this.keyListeners.get(key)?.size ?? 0) > 0) { return true; } + const collectionKey = OnyxKeys.getCollectionKey(key); if (collectionKey && collectionKey !== key && (this.keyListeners.get(collectionKey)?.size ?? 0) > 0) { return true; } + return false; } + /** + * Runs a listener, catching and logging any throw so one failing listener can't stop the rest. + */ private safeInvoke(fn: () => void, contextKey: OnyxKey): void { try { fn(); From 844571b1f7592975cb06c4ba21d8f184b307d11e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 3 Sep 2026 17:07:39 +0100 Subject: [PATCH 4/8] fix: snapshot listener sets before dispatch so subscription changes during a notify only affect later ones --- lib/OnyxStore.ts | 8 +++--- tests/unit/OnyxStoreTest.ts | 49 +++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/lib/OnyxStore.ts b/lib/OnyxStore.ts index d9e14af8f..629350257 100644 --- a/lib/OnyxStore.ts +++ b/lib/OnyxStore.ts @@ -91,7 +91,7 @@ class OnyxStore { // 1. Exact-key listeners const exact = this.keyListeners.get(key); if (exact && exact.size > 0) { - for (const listener of exact) { + for (const listener of [...exact]) { this.safeInvoke(() => listener(value, key), key); } } @@ -105,7 +105,7 @@ class OnyxStore { const collectionListeners = this.keyListeners.get(collectionKey); if (collectionListeners && collectionListeners.size > 0) { const collectionData = cache.getCollectionData(collectionKey); - for (const listener of collectionListeners) { + for (const listener of [...collectionListeners]) { this.safeInvoke(() => listener(collectionData, collectionKey), collectionKey); } } @@ -139,7 +139,7 @@ class OnyxStore { // 1. Collection listeners fire once with the new collection object. const collectionListeners = this.keyListeners.get(collectionKey); if (collectionListeners && collectionListeners.size > 0) { - for (const listener of collectionListeners) { + for (const listener of [...collectionListeners]) { this.safeInvoke(() => listener(collectionData, collectionKey), collectionKey); } } @@ -157,7 +157,7 @@ class OnyxStore { continue; } - for (const listener of exact) { + for (const listener of [...exact]) { this.safeInvoke(() => listener(value, memberKey), memberKey); } } diff --git a/tests/unit/OnyxStoreTest.ts b/tests/unit/OnyxStoreTest.ts index be54297c1..4e2cf2d90 100644 --- a/tests/unit/OnyxStoreTest.ts +++ b/tests/unit/OnyxStoreTest.ts @@ -248,6 +248,55 @@ describe('OnyxStore', () => { }); }); + describe('subscription mutation during dispatch', () => { + it('should fire a listener that unsubscribes and re-subscribes itself during dispatch only once', () => { + let unsubscribe: () => void = jest.fn(); + const callback = jest.fn(() => { + unsubscribe(); + unsubscribe = onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback); + }); + unsubscribe = onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback); + + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'x'); + + expect(callback).toHaveBeenCalledTimes(1); + }); + + it('should not deliver the in-flight notification to a listener added during dispatch', () => { + const lateCallback = jest.fn(); + const firstCallback = jest.fn(() => { + onyxStore.subscribe(ONYXKEYS.TEST_KEY, lateCallback); + }); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, firstCallback); + + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'first'); + expect(lateCallback).not.toHaveBeenCalled(); + + // It receives later notifications normally. + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'second'); + expect(lateCallback).toHaveBeenCalledTimes(1); + expect(lateCallback).toHaveBeenCalledWith('second', ONYXKEYS.TEST_KEY); + }); + + it('should still fire a sibling unsubscribed during dispatch this round, but not on later notifications', () => { + const siblingCallback = jest.fn(); + let unsubscribeSibling: () => void = jest.fn(); + const firstCallback = jest.fn(() => { + unsubscribeSibling(); + }); + onyxStore.subscribe(ONYXKEYS.TEST_KEY, firstCallback); + unsubscribeSibling = onyxStore.subscribe(ONYXKEYS.TEST_KEY, siblingCallback); + + // The sibling was registered when dispatch began, so the snapshot still fires it. + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'first'); + expect(siblingCallback).toHaveBeenCalledTimes(1); + + // Now unsubscribed, it does not fire again. + onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'second'); + expect(siblingCallback).toHaveBeenCalledTimes(1); + }); + }); + describe('listener error isolation', () => { it('should log a throwing listener and still fire the other listeners', () => { const logAlertSpy = jest.spyOn(Logger, 'logAlert').mockImplementation(() => { From 46e135e6dd322318a22b4cc7032a8ba6f06b4e1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 7 Sep 2026 14:12:29 +0100 Subject: [PATCH 5/8] Rename OnyxStore to OnyxSubscriptionManager Co-Authored-By: Claude Code --- ...nyxStore.ts => OnyxSubscriptionManager.ts} | 20 ++- ...Test.ts => OnyxSubscriptionManagerTest.ts} | 124 +++++++++--------- 2 files changed, 71 insertions(+), 73 deletions(-) rename lib/{OnyxStore.ts => OnyxSubscriptionManager.ts} (92%) rename tests/unit/{OnyxStoreTest.ts => OnyxSubscriptionManagerTest.ts} (65%) diff --git a/lib/OnyxStore.ts b/lib/OnyxSubscriptionManager.ts similarity index 92% rename from lib/OnyxStore.ts rename to lib/OnyxSubscriptionManager.ts index 629350257..09072b4da 100644 --- a/lib/OnyxStore.ts +++ b/lib/OnyxSubscriptionManager.ts @@ -23,15 +23,13 @@ type NotifyKeyOptions = { }; /** - * `OnyxStore` is a single listener registry for Onyx reads/subscriptions. One index backs - * every subscription: - * - * keyListeners: exact-key listeners (a single key, a collection object, - * or a specific collection member). - * - * Write paths call `notifyKey()` (single-key write) or `notifyCollection()` (batch collection update). + * OnyxSubscriptionManager is a registry for Onyx subscriptions. + * Subscriptions are stored in `keyListeners`, a flat map keyed by OnyxKey. + * Subscribers are notified on a per-key basis: + * - `notifyKey` for individual keys or collection members + * - `notifyCollection` for batch updates to collections */ -class OnyxStore { +class OnyxSubscriptionManager { private keyListeners: Map>; constructor() { @@ -193,12 +191,12 @@ class OnyxStore { try { fn(); } catch (error) { - Logger.logAlert(`[OnyxStore] Listener threw an error for key '${contextKey}': ${error}`); + Logger.logAlert(`[OnyxSubscriptionManager] Listener threw an error for key '${contextKey}': ${error}`); } } } -const onyxStore = new OnyxStore(); +const onyxSubscriptionManager = new OnyxSubscriptionManager(); -export default onyxStore; +export default onyxSubscriptionManager; export type {KeyListener}; diff --git a/tests/unit/OnyxStoreTest.ts b/tests/unit/OnyxSubscriptionManagerTest.ts similarity index 65% rename from tests/unit/OnyxStoreTest.ts rename to tests/unit/OnyxSubscriptionManagerTest.ts index 4e2cf2d90..cba0e4a2e 100644 --- a/tests/unit/OnyxStoreTest.ts +++ b/tests/unit/OnyxSubscriptionManagerTest.ts @@ -1,14 +1,14 @@ import type {OnyxKey} from '../../lib'; import Onyx from '../../lib'; -import onyxStore from '../../lib/OnyxStore'; +import onyxSubscriptionManager from '../../lib/OnyxSubscriptionManager'; import cache from '../../lib/OnyxCache'; import * as Logger from '../../lib/Logger'; -// We need access to some internal properties of `onyxStore` during the tests but they are private, +// We need access to some internal properties of `onyxSubscriptionManager` during the tests but they are private, // so this workaround allows us to have access to them. The maps are created once in the constructor // and only ever `.clear()`ed (never reassigned), so capturing the references here stays valid. // eslint-disable-next-line dot-notation -const keyListeners = onyxStore['keyListeners']; +const keyListeners = onyxSubscriptionManager['keyListeners']; const ONYXKEYS = { TEST_KEY: 'test', @@ -28,10 +28,10 @@ Onyx.init({ beforeEach(() => Onyx.clear()); -describe('OnyxStore', () => { +describe('OnyxSubscriptionManager', () => { // Always start from a clean registry. beforeEach(() => { - onyxStore.clearAll(); + onyxSubscriptionManager.clearAll(); }); afterEach(() => { @@ -41,9 +41,9 @@ describe('OnyxStore', () => { describe('subscribe / notifyKey', () => { it('should fire the listener with (value, key) on notifyKey', () => { const callback = jest.fn(); - onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback); + onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, callback); - onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'hello'); + onyxSubscriptionManager.notifyKey(ONYXKEYS.TEST_KEY, 'hello'); expect(callback).toHaveBeenCalledTimes(1); expect(callback).toHaveBeenCalledWith('hello', ONYXKEYS.TEST_KEY); @@ -52,10 +52,10 @@ describe('OnyxStore', () => { it('should fire all listeners registered on the same key', () => { const callback1 = jest.fn(); const callback2 = jest.fn(); - onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback1); - onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback2); + onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, callback1); + onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, callback2); - onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 1); + onyxSubscriptionManager.notifyKey(ONYXKEYS.TEST_KEY, 1); expect(callback1).toHaveBeenCalledTimes(1); expect(callback2).toHaveBeenCalledTimes(1); @@ -63,11 +63,11 @@ describe('OnyxStore', () => { it('should not fire the listener after it unsubscribes', () => { const callback = jest.fn(); - const unsubscribe = onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback); + const unsubscribe = onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, callback); - onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'first'); + onyxSubscriptionManager.notifyKey(ONYXKEYS.TEST_KEY, 'first'); unsubscribe(); - onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'second'); + onyxSubscriptionManager.notifyKey(ONYXKEYS.TEST_KEY, 'second'); expect(callback).toHaveBeenCalledTimes(1); expect(callback).toHaveBeenLastCalledWith('first', ONYXKEYS.TEST_KEY); @@ -76,18 +76,18 @@ describe('OnyxStore', () => { it('should only unsubscribe the specific listener, leaving others intact', () => { const callback1 = jest.fn(); const callback2 = jest.fn(); - const unsubscribe1 = onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback1); - onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback2); + const unsubscribe1 = onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, callback1); + onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, callback2); unsubscribe1(); - onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 1); + onyxSubscriptionManager.notifyKey(ONYXKEYS.TEST_KEY, 1); expect(callback1).not.toHaveBeenCalled(); expect(callback2).toHaveBeenCalledTimes(1); }); it('should delete the key entry from the internal map once the last listener unsubscribes', () => { - const unsubscribe = onyxStore.subscribe(ONYXKEYS.TEST_KEY, jest.fn()); + const unsubscribe = onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, jest.fn()); expect(keyListeners.has(ONYXKEYS.TEST_KEY)).toBeTruthy(); unsubscribe(); @@ -96,17 +96,17 @@ describe('OnyxStore', () => { }); it('should be a no-op to notify a key with no listeners', () => { - expect(() => onyxStore.notifyKey('keyWithNoListeners' as OnyxKey, 'x')).not.toThrow(); + expect(() => onyxSubscriptionManager.notifyKey('keyWithNoListeners' as OnyxKey, 'x')).not.toThrow(); }); it('should be idempotent when unsubscribing more than once', () => { const callback = jest.fn(); - const unsubscribe = onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback); + const unsubscribe = onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, callback); unsubscribe(); expect(() => unsubscribe()).not.toThrow(); - onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 1); + onyxSubscriptionManager.notifyKey(ONYXKEYS.TEST_KEY, 1); expect(callback).not.toHaveBeenCalled(); }); }); @@ -117,9 +117,9 @@ describe('OnyxStore', () => { const getCollectionData = jest.spyOn(cache, 'getCollectionData').mockReturnValue(collectionData); const callback = jest.fn(); - onyxStore.subscribe(COLLECTION, callback); + onyxSubscriptionManager.subscribe(COLLECTION, callback); - onyxStore.notifyKey(MEMBER_1, {id: 1}); + onyxSubscriptionManager.notifyKey(MEMBER_1, {id: 1}); expect(getCollectionData).toHaveBeenCalledWith(COLLECTION); expect(callback).toHaveBeenCalledTimes(1); @@ -132,10 +132,10 @@ describe('OnyxStore', () => { const memberCallback = jest.fn(); const collectionCallback = jest.fn(); - onyxStore.subscribe(MEMBER_1, memberCallback); - onyxStore.subscribe(COLLECTION, collectionCallback); + onyxSubscriptionManager.subscribe(MEMBER_1, memberCallback); + onyxSubscriptionManager.subscribe(COLLECTION, collectionCallback); - onyxStore.notifyKey(MEMBER_1, {id: 1}); + onyxSubscriptionManager.notifyKey(MEMBER_1, {id: 1}); expect(memberCallback).toHaveBeenCalledWith({id: 1}, MEMBER_1); expect(collectionCallback).toHaveBeenCalledWith(collectionData, COLLECTION); @@ -146,10 +146,10 @@ describe('OnyxStore', () => { const memberCallback = jest.fn(); const collectionCallback = jest.fn(); - onyxStore.subscribe(MEMBER_1, memberCallback); - onyxStore.subscribe(COLLECTION, collectionCallback); + onyxSubscriptionManager.subscribe(MEMBER_1, memberCallback); + onyxSubscriptionManager.subscribe(COLLECTION, collectionCallback); - onyxStore.notifyKey(MEMBER_1, {id: 1}, {suppressCollectionNotify: true}); + onyxSubscriptionManager.notifyKey(MEMBER_1, {id: 1}, {suppressCollectionNotify: true}); expect(memberCallback).toHaveBeenCalledTimes(1); expect(collectionCallback).not.toHaveBeenCalled(); @@ -160,9 +160,9 @@ describe('OnyxStore', () => { it('should not perform collection routing for a non-member single key', () => { const getCollectionData = jest.spyOn(cache, 'getCollectionData'); const callback = jest.fn(); - onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback); + onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, callback); - onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'x'); + onyxSubscriptionManager.notifyKey(ONYXKEYS.TEST_KEY, 'x'); expect(callback).toHaveBeenCalledTimes(1); expect(getCollectionData).not.toHaveBeenCalled(); @@ -175,9 +175,9 @@ describe('OnyxStore', () => { jest.spyOn(cache, 'getCollectionData').mockReturnValue(collectionData); const callback = jest.fn(); - onyxStore.subscribe(COLLECTION, callback); + onyxSubscriptionManager.subscribe(COLLECTION, callback); - onyxStore.notifyCollection(COLLECTION, {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}); + onyxSubscriptionManager.notifyCollection(COLLECTION, {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}); expect(callback).toHaveBeenCalledTimes(1); expect(callback).toHaveBeenCalledWith(collectionData, COLLECTION); @@ -190,10 +190,10 @@ describe('OnyxStore', () => { const member1Callback = jest.fn(); const member2Callback = jest.fn(); - onyxStore.subscribe(MEMBER_1, member1Callback); - onyxStore.subscribe(MEMBER_2, member2Callback); + onyxSubscriptionManager.subscribe(MEMBER_1, member1Callback); + onyxSubscriptionManager.subscribe(MEMBER_2, member2Callback); - onyxStore.notifyCollection( + onyxSubscriptionManager.notifyCollection( COLLECTION, {[MEMBER_1]: {id: 1}, [MEMBER_2]: shared}, {[MEMBER_2]: shared}, // previous: member 2 unchanged by reference @@ -205,9 +205,9 @@ describe('OnyxStore', () => { it('should be a no-op when the partial collection is empty', () => { const callback = jest.fn(); - onyxStore.subscribe(COLLECTION, callback); + onyxSubscriptionManager.subscribe(COLLECTION, callback); - onyxStore.notifyCollection(COLLECTION, {}); + onyxSubscriptionManager.notifyCollection(COLLECTION, {}); expect(callback).not.toHaveBeenCalled(); }); @@ -215,36 +215,36 @@ describe('OnyxStore', () => { describe('hasListenersForKey', () => { it('should return true for an exact-key subscriber', () => { - onyxStore.subscribe(ONYXKEYS.TEST_KEY, jest.fn()); - expect(onyxStore.hasListenersForKey(ONYXKEYS.TEST_KEY)).toBeTruthy(); + onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, jest.fn()); + expect(onyxSubscriptionManager.hasListenersForKey(ONYXKEYS.TEST_KEY)).toBeTruthy(); }); it('should return true for a member key when its parent collection has a subscriber', () => { - onyxStore.subscribe(COLLECTION, jest.fn()); - expect(onyxStore.hasListenersForKey(MEMBER_1)).toBeTruthy(); + onyxSubscriptionManager.subscribe(COLLECTION, jest.fn()); + expect(onyxSubscriptionManager.hasListenersForKey(MEMBER_1)).toBeTruthy(); }); it('should return false when there are no relevant subscribers', () => { - expect(onyxStore.hasListenersForKey('someUnwatchedKey')).toBeFalsy(); + expect(onyxSubscriptionManager.hasListenersForKey('someUnwatchedKey')).toBeFalsy(); }); it('should return false after the last listener unsubscribes', () => { - const unsubscribe = onyxStore.subscribe(ONYXKEYS.TEST_KEY, jest.fn()); + const unsubscribe = onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, jest.fn()); unsubscribe(); - expect(onyxStore.hasListenersForKey(ONYXKEYS.TEST_KEY)).toBeFalsy(); + expect(onyxSubscriptionManager.hasListenersForKey(ONYXKEYS.TEST_KEY)).toBeFalsy(); }); }); describe('clearAll', () => { it('should wipe key and collection subscriptions', () => { const keyCallback = jest.fn(); - onyxStore.subscribe(ONYXKEYS.TEST_KEY, keyCallback); + onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, keyCallback); - onyxStore.clearAll(); - onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'x'); + onyxSubscriptionManager.clearAll(); + onyxSubscriptionManager.notifyKey(ONYXKEYS.TEST_KEY, 'x'); expect(keyCallback).not.toHaveBeenCalled(); - expect(onyxStore.hasListenersForKey(ONYXKEYS.TEST_KEY)).toBeFalsy(); + expect(onyxSubscriptionManager.hasListenersForKey(ONYXKEYS.TEST_KEY)).toBeFalsy(); }); }); @@ -253,11 +253,11 @@ describe('OnyxStore', () => { let unsubscribe: () => void = jest.fn(); const callback = jest.fn(() => { unsubscribe(); - unsubscribe = onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback); + unsubscribe = onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, callback); }); - unsubscribe = onyxStore.subscribe(ONYXKEYS.TEST_KEY, callback); + unsubscribe = onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, callback); - onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'x'); + onyxSubscriptionManager.notifyKey(ONYXKEYS.TEST_KEY, 'x'); expect(callback).toHaveBeenCalledTimes(1); }); @@ -265,15 +265,15 @@ describe('OnyxStore', () => { it('should not deliver the in-flight notification to a listener added during dispatch', () => { const lateCallback = jest.fn(); const firstCallback = jest.fn(() => { - onyxStore.subscribe(ONYXKEYS.TEST_KEY, lateCallback); + onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, lateCallback); }); - onyxStore.subscribe(ONYXKEYS.TEST_KEY, firstCallback); + onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, firstCallback); - onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'first'); + onyxSubscriptionManager.notifyKey(ONYXKEYS.TEST_KEY, 'first'); expect(lateCallback).not.toHaveBeenCalled(); // It receives later notifications normally. - onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'second'); + onyxSubscriptionManager.notifyKey(ONYXKEYS.TEST_KEY, 'second'); expect(lateCallback).toHaveBeenCalledTimes(1); expect(lateCallback).toHaveBeenCalledWith('second', ONYXKEYS.TEST_KEY); }); @@ -284,15 +284,15 @@ describe('OnyxStore', () => { const firstCallback = jest.fn(() => { unsubscribeSibling(); }); - onyxStore.subscribe(ONYXKEYS.TEST_KEY, firstCallback); - unsubscribeSibling = onyxStore.subscribe(ONYXKEYS.TEST_KEY, siblingCallback); + onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, firstCallback); + unsubscribeSibling = onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, siblingCallback); // The sibling was registered when dispatch began, so the snapshot still fires it. - onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'first'); + onyxSubscriptionManager.notifyKey(ONYXKEYS.TEST_KEY, 'first'); expect(siblingCallback).toHaveBeenCalledTimes(1); // Now unsubscribed, it does not fire again. - onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'second'); + onyxSubscriptionManager.notifyKey(ONYXKEYS.TEST_KEY, 'second'); expect(siblingCallback).toHaveBeenCalledTimes(1); }); }); @@ -306,10 +306,10 @@ describe('OnyxStore', () => { throw new Error('boom'); }); const healthyCallback = jest.fn(); - onyxStore.subscribe(ONYXKEYS.TEST_KEY, throwingCallback); - onyxStore.subscribe(ONYXKEYS.TEST_KEY, healthyCallback); + onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, throwingCallback); + onyxSubscriptionManager.subscribe(ONYXKEYS.TEST_KEY, healthyCallback); - expect(() => onyxStore.notifyKey(ONYXKEYS.TEST_KEY, 'x')).not.toThrow(); + expect(() => onyxSubscriptionManager.notifyKey(ONYXKEYS.TEST_KEY, 'x')).not.toThrow(); expect(throwingCallback).toHaveBeenCalledTimes(1); expect(healthyCallback).toHaveBeenCalledTimes(1); From 6033e5885bb8b0971923c121ab3a59a9399d9d7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 7 Sep 2026 14:37:27 +0100 Subject: [PATCH 6/8] Rename listener types per review: Listener, GenericListener Co-Authored-By: Claude Code --- lib/OnyxSubscriptionManager.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/OnyxSubscriptionManager.ts b/lib/OnyxSubscriptionManager.ts index 09072b4da..13f9350e2 100644 --- a/lib/OnyxSubscriptionManager.ts +++ b/lib/OnyxSubscriptionManager.ts @@ -7,12 +7,13 @@ import OnyxKeys from './OnyxKeys'; /** * Listener fired when an exact key's value changes. */ -type KeyListener = (value: OnyxValue, key: TKey) => void; +type Listener = (value: OnyxValue, key: TKey) => void; /** - * Storage form of a listener, value erased so one Map can hold listeners for every key type. + * Generic listener, without specific types for the keys or values. + * This way a single Map can hold listeners for every key type. */ -type StoredListener = (value: unknown, key: OnyxKey) => void; +type GenericListener = (value: unknown, key: OnyxKey) => void; type NotifyKeyOptions = { /** @@ -30,7 +31,7 @@ type NotifyKeyOptions = { * - `notifyCollection` for batch updates to collections */ class OnyxSubscriptionManager { - private keyListeners: Map>; + private keyListeners: Map>; constructor() { this.keyListeners = new Map(); @@ -54,14 +55,14 @@ class OnyxSubscriptionManager { * * Returns an unsubscribe function. */ - subscribe(key: TKey, listener: KeyListener): () => void { + subscribe(key: TKey, listener: Listener): () => void { let listeners = this.keyListeners.get(key); if (!listeners) { listeners = new Set(); this.keyListeners.set(key, listeners); } - listeners.add(listener as StoredListener); + listeners.add(listener as GenericListener); return () => { const set = this.keyListeners.get(key); @@ -69,7 +70,7 @@ class OnyxSubscriptionManager { return; } - set.delete(listener as StoredListener); + set.delete(listener as GenericListener); if (set.size === 0) { this.keyListeners.delete(key); @@ -199,4 +200,4 @@ class OnyxSubscriptionManager { const onyxSubscriptionManager = new OnyxSubscriptionManager(); export default onyxSubscriptionManager; -export type {KeyListener}; +export type {Listener}; From 457d3bf5f178a36dccde7e812d9a3843de748b24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 7 Sep 2026 15:27:16 +0100 Subject: [PATCH 7/8] Add getState tests and guard notifyCollection skip against omitted previous collection A removed member reads undefined on both sides when partialPreviousCollection is omitted, so the ref-equality skip must only apply when previous actually carries the member. Adds regression test plus getState coverage for both the single-key and collection-key paths. Co-Authored-By: Claude Code --- lib/OnyxSubscriptionManager.ts | 9 ++++--- tests/unit/OnyxSubscriptionManagerTest.ts | 33 +++++++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/lib/OnyxSubscriptionManager.ts b/lib/OnyxSubscriptionManager.ts index 13f9350e2..7d9b8b374 100644 --- a/lib/OnyxSubscriptionManager.ts +++ b/lib/OnyxSubscriptionManager.ts @@ -128,8 +128,6 @@ class OnyxSubscriptionManager { if (changedKeys.length === 0) { return; } - const previous = partialPreviousCollection ?? {}; - // Read the merged collection once. `cache.getCollectionData()` returns the post-merge // frozen object, which is what listeners should see (not the raw `partialCollection` // input, which is just the delta and lacks fields preserved during merge). @@ -144,10 +142,13 @@ class OnyxSubscriptionManager { } // 2. Exact-member subscribers fire per changed key (skip if ref unchanged vs previous). + // Only treat a member as unchanged when `previous` actually carries it: when the + // previous collection is omitted, a removed member reads `undefined` on both sides + // and would otherwise be skipped even though it changed. for (const memberKey of changedKeys) { const value = collectionData?.[memberKey]; - const prev = previous[memberKey]; - if (value === prev) { + const prev = partialPreviousCollection?.[memberKey]; + if (partialPreviousCollection && Object.prototype.hasOwnProperty.call(partialPreviousCollection, memberKey) && value === prev) { continue; } diff --git a/tests/unit/OnyxSubscriptionManagerTest.ts b/tests/unit/OnyxSubscriptionManagerTest.ts index cba0e4a2e..a48837575 100644 --- a/tests/unit/OnyxSubscriptionManagerTest.ts +++ b/tests/unit/OnyxSubscriptionManagerTest.ts @@ -111,6 +111,25 @@ describe('OnyxSubscriptionManager', () => { }); }); + describe('getState', () => { + it('should return the cached value for a single key', () => { + cache.set(ONYXKEYS.TEST_KEY, 'hello'); + + expect(onyxSubscriptionManager.getState(ONYXKEYS.TEST_KEY)).toBe('hello'); + }); + + it('should return undefined for a single key that is not in the cache', () => { + expect(onyxSubscriptionManager.getState(ONYXKEYS.OTHER_TEST)).toBeUndefined(); + }); + + it('should return the collection object for a collection key', () => { + const collectionData = {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}; + jest.spyOn(cache, 'getCollectionData').mockReturnValue(collectionData); + + expect(onyxSubscriptionManager.getState(COLLECTION)).toBe(collectionData); + }); + }); + describe('collection routing on notifyKey', () => { it('should fire the collection-root listener with the cache collection object when a member is written', () => { const collectionData = {[MEMBER_1]: {id: 1}, [MEMBER_2]: {id: 2}}; @@ -203,6 +222,20 @@ describe('OnyxSubscriptionManager', () => { expect(member2Callback).not.toHaveBeenCalled(); }); + it('should notify the exact-member listener when the member is deleted without a previous collection', () => { + // member1 has already been removed from the cache by the time notifyCollection runs. + jest.spyOn(cache, 'getCollectionData').mockReturnValue({[MEMBER_2]: {id: 2}}); + + const member1Callback = jest.fn(); + onyxSubscriptionManager.subscribe(MEMBER_1, member1Callback); + + // No partialPreviousCollection passed — a removed member reads undefined on both + // sides and must not be treated as unchanged. + onyxSubscriptionManager.notifyCollection(COLLECTION, {[MEMBER_1]: null}); + + expect(member1Callback).toHaveBeenCalledTimes(1); + }); + it('should be a no-op when the partial collection is empty', () => { const callback = jest.fn(); onyxSubscriptionManager.subscribe(COLLECTION, callback); From 435487c380b97cd912f590690e3c2b3d11ab1d39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 8 Sep 2026 10:24:38 +0100 Subject: [PATCH 8/8] Remove the suppressCollectionNotify option from notifyKey No production call site sets it: collection-batch write paths route members through a single notifyCollection(), so per-member notifyKey calls always want the collection-root routing. Drops the option, the NotifyKeyOptions type, and the now-dead suppression test. Co-Authored-By: Claude Code --- lib/OnyxSubscriptionManager.ts | 14 +++----------- tests/unit/OnyxSubscriptionManagerTest.ts | 16 ---------------- 2 files changed, 3 insertions(+), 27 deletions(-) diff --git a/lib/OnyxSubscriptionManager.ts b/lib/OnyxSubscriptionManager.ts index 7d9b8b374..7e61ec7e1 100644 --- a/lib/OnyxSubscriptionManager.ts +++ b/lib/OnyxSubscriptionManager.ts @@ -15,14 +15,6 @@ type Listener = (value: OnyxValue, key: TK */ type GenericListener = (value: unknown, key: OnyxKey) => void; -type NotifyKeyOptions = { - /** - * Skips collection-level routing. Collection-batch write paths set it so each member write - * doesn't re-trigger the collection-level listeners; the outer `notifyCollection()` fires those once. - */ - suppressCollectionNotify?: boolean; -}; - /** * OnyxSubscriptionManager is a registry for Onyx subscriptions. * Subscriptions are stored in `keyListeners`, a flat map keyed by OnyxKey. @@ -84,9 +76,9 @@ class OnyxSubscriptionManager { * Dispatch: * 1. keyListeners.get(key): exact-key subscribers (always fires). * 2. If key is a collection member, keyListeners.get(collectionKey): collection - * listeners for the parent collection (unless `options.suppressCollectionNotify`). + * listeners for the parent collection. */ - notifyKey(key: TKey, value: OnyxValue, options?: NotifyKeyOptions): void { + notifyKey(key: TKey, value: OnyxValue): void { // 1. Exact-key listeners const exact = this.keyListeners.get(key); if (exact && exact.size > 0) { @@ -100,7 +92,7 @@ class OnyxSubscriptionManager { // unsupported anti-pattern; treat them as opaque single-key writes. const collectionKey = OnyxKeys.getCollectionKey(key); const isCollectionMemberWrite = collectionKey !== undefined && collectionKey !== key; - if (isCollectionMemberWrite && !options?.suppressCollectionNotify) { + if (isCollectionMemberWrite) { const collectionListeners = this.keyListeners.get(collectionKey); if (collectionListeners && collectionListeners.size > 0) { const collectionData = cache.getCollectionData(collectionKey); diff --git a/tests/unit/OnyxSubscriptionManagerTest.ts b/tests/unit/OnyxSubscriptionManagerTest.ts index a48837575..1b8ba7a09 100644 --- a/tests/unit/OnyxSubscriptionManagerTest.ts +++ b/tests/unit/OnyxSubscriptionManagerTest.ts @@ -160,22 +160,6 @@ describe('OnyxSubscriptionManager', () => { expect(collectionCallback).toHaveBeenCalledWith(collectionData, COLLECTION); }); - it('should skip the collection-root listener but still fire the exact-member listener when suppressCollectionNotify is set', () => { - const getCollectionData = jest.spyOn(cache, 'getCollectionData').mockReturnValue({}); - - const memberCallback = jest.fn(); - const collectionCallback = jest.fn(); - onyxSubscriptionManager.subscribe(MEMBER_1, memberCallback); - onyxSubscriptionManager.subscribe(COLLECTION, collectionCallback); - - onyxSubscriptionManager.notifyKey(MEMBER_1, {id: 1}, {suppressCollectionNotify: true}); - - expect(memberCallback).toHaveBeenCalledTimes(1); - expect(collectionCallback).not.toHaveBeenCalled(); - // The collection object is never read when suppressed. - expect(getCollectionData).not.toHaveBeenCalled(); - }); - it('should not perform collection routing for a non-member single key', () => { const getCollectionData = jest.spyOn(cache, 'getCollectionData'); const callback = jest.fn();