From 48778d4f1b704ae814d2f0669f243729a1ab5d78 Mon Sep 17 00:00:00 2001 From: Artem Makushov Date: Wed, 29 Oct 2025 00:45:03 +0100 Subject: [PATCH 1/7] add guard that prevents invalid keys --- lib/OnyxUtils.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 39e8afd06..3596c5276 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1064,6 +1064,23 @@ function subscribeToKey(connectOptions: ConnectOptions; callbackToStateMapping[subscriptionID].subscriptionID = subscriptionID; + // If the subscriber is attempting to connect to a collection member whose ID is skippable (e.g. "undefined", "null", etc.) + // we suppress wiring the subscription fully to avoid unnecessary callback emissions such as for "report_undefined". + // We still return a valid subscriptionID so callers can disconnect safely. + try { + const skippableIDs = getSkippableCollectionMemberIDs(); + if (skippableIDs.size) { + const [, collectionMemberID] = splitCollectionMemberKey(mapping.key); + if (skippableIDs.has(collectionMemberID)) { + // Clean up the provisional mapping to avoid retaining unused subscribers + delete callbackToStateMapping[subscriptionID]; + return subscriptionID; + } + } + } catch (e) { + // Not a collection member key, proceed as usual. + } + // When keyChanged is called, a key is passed and the method looks through all the Subscribers in callbackToStateMapping for the matching key to get the subscriptionID // to avoid having to loop through all the Subscribers all the time (even when just one connection belongs to one key), // We create a mapping from key to lists of subscriptionIDs to access the specific list of subscriptionIDs. From afb861eef264032f2e52b1a4f867b3f1db643612 Mon Sep 17 00:00:00 2001 From: Artem Makushov Date: Wed, 29 Oct 2025 18:49:51 +0100 Subject: [PATCH 2/7] add init tests --- tests/unit/onyxUtilsTest.ts | 58 +++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/unit/onyxUtilsTest.ts b/tests/unit/onyxUtilsTest.ts index 9b120a580..60d0347e2 100644 --- a/tests/unit/onyxUtilsTest.ts +++ b/tests/unit/onyxUtilsTest.ts @@ -1,9 +1,11 @@ +import {act} from '@testing-library/react-native'; import Onyx from '../../lib'; import OnyxUtils from '../../lib/OnyxUtils'; import type {GenericDeepRecord} from '../types'; import utils from '../../lib/utils'; import type {Collection, OnyxCollection} from '../../lib/types'; import type GenericCollection from '../utils/GenericCollection'; +import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; const testObject: GenericDeepRecord = { a: 'a', @@ -83,6 +85,62 @@ Onyx.init({ beforeEach(() => Onyx.clear()); describe('OnyxUtils', () => { + describe('skippable member subscriptions', () => { + const BASE = ONYXKEYS.COLLECTION.TEST_KEY; + + beforeEach(() => { + // Enable skipping of undefined member IDs for these tests + OnyxUtils.setSkippableCollectionMemberIDs(new Set(['undefined'])); + }); + + afterEach(() => { + // Restore to no skippable IDs to avoid affecting other tests + OnyxUtils.setSkippableCollectionMemberIDs(new Set()); + }); + + it('does not emit initial callback for report_undefined member', async () => { + const key = `${BASE}undefined`; + const callback = jest.fn(); + Onyx.connect({key, callback}); + + // Flush async subscription flow + await act(async () => waitForPromisesToResolve()); + + // No initial data should be sent for a skippable member + expect(callback).not.toHaveBeenCalled(); + }); + + it('still emits for valid member keys', async () => { + const key = `${BASE}123`; + await Onyx.set(key, {id: 123}); + + const callback = jest.fn(); + Onyx.connect({key, callback}); + await act(async () => waitForPromisesToResolve()); + expect(callback).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenCalledWith({id: 123}, key); + }); + + it('omits skippable members from base collection', async () => { + const undefinedKey = `${BASE}undefined`; + const validKey = `${BASE}1`; + + await Onyx.set(undefinedKey, {bad: true}); + await Onyx.set(validKey, {ok: true}); + + let received: Record | undefined; + Onyx.connect({ + key: BASE, + waitForCollectionCallback: true, + callback: (value) => { + received = value as Record; + }, + }); + await act(async () => waitForPromisesToResolve()); + expect(received).toEqual({[validKey]: {ok: true}}); + expect(Object.keys(received ?? {})).not.toContain(undefinedKey); + }); + }); describe('splitCollectionMemberKey', () => { describe('should return correct values', () => { const dataResult: Record = { From e418fb1e82cd6c4565b4d5a939981f7f1cb9c719 Mon Sep 17 00:00:00 2001 From: Artem Makushov Date: Wed, 29 Oct 2025 19:03:24 +0100 Subject: [PATCH 3/7] updates --- lib/OnyxUtils.ts | 3 +++ lib/useOnyx.ts | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 3596c5276..f5f261114 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1073,6 +1073,9 @@ function subscribeToKey(connectOptions: ConnectOptions); delete callbackToStateMapping[subscriptionID]; return subscriptionID; } diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 016eb0f33..196df6516 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -229,6 +229,24 @@ function useOnyx>( }, [key, options?.canEvict]); const getSnapshot = useCallback(() => { + // Fast path: if subscribing to a skippable collection member id, return undefined as loaded immediately + if (isFirstConnectionRef.current) { + try { + const [, memberId] = OnyxUtils.splitCollectionMemberKey(key); + if (OnyxUtils.getSkippableCollectionMemberIDs().has(memberId)) { + // Finalize initial state as loaded undefined and stop further first-connection flows + if (resultRef.current[1].status !== 'loaded' || resultRef.current[0] !== undefined) { + resultRef.current = [undefined, {status: 'loaded'}]; + onyxSnapshotCache.setCachedResult>(key, cacheKey, resultRef.current); + } + isFirstConnectionRef.current = false; + shouldGetCachedValueRef.current = false; + return resultRef.current; + } + } catch (e) { + // Not a collection member, continue as usual + } + } // Check if we have any cache for this Onyx key // Don't use cache for first connection with initWithStoredValues: false // Also don't use cache during active data updates (when shouldGetCachedValueRef is true) From 06f63eade5f64cc4b95f9b29fd0b784b7f8966ec Mon Sep 17 00:00:00 2001 From: Artem Makushov Date: Sun, 15 Feb 2026 22:07:52 +0700 Subject: [PATCH 4/7] add isSkippableKey to avoid recalculations --- lib/useOnyx.ts | 42 +++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 29bc57734..533a5d829 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -163,6 +163,21 @@ function useOnyx>( useEffect(() => () => onyxSnapshotCache.deregisterConsumer(key, cacheKey), [key, cacheKey]); + // Precompute whether this key is a skippable collection member so that getSnapshot() + // can use a cheap boolean check instead of calling splitCollectionMemberKey on every render. + const isSkippableKey = useMemo(() => { + const skippableIDs = OnyxUtils.getSkippableCollectionMemberIDs(); + if (!skippableIDs.size) { + return false; + } + try { + const [, memberId] = OnyxUtils.splitCollectionMemberKey(key); + return skippableIDs.has(memberId); + } catch { + return false; + } + }, [key]); + useEffect(() => { // These conditions will ensure we can only handle dynamic collection member keys from the same collection. if (options?.allowDynamicKey || previousKey === key) { @@ -231,23 +246,16 @@ function useOnyx>( }, [key, options?.canEvict]); const getSnapshot = useCallback(() => { - // Fast path: if subscribing to a skippable collection member id, return undefined as loaded immediately - if (isFirstConnectionRef.current) { - try { - const [, memberId] = OnyxUtils.splitCollectionMemberKey(key); - if (OnyxUtils.getSkippableCollectionMemberIDs().has(memberId)) { - // Finalize initial state as loaded undefined and stop further first-connection flows - if (resultRef.current[1].status !== 'loaded' || resultRef.current[0] !== undefined) { - resultRef.current = [undefined, {status: 'loaded'}]; - onyxSnapshotCache.setCachedResult>(key, cacheKey, resultRef.current); - } - isFirstConnectionRef.current = false; - shouldGetCachedValueRef.current = false; - return resultRef.current; - } - } catch (e) { - // Not a collection member, continue as usual + // Fast path: if subscribing to a skippable collection member id, return undefined as loaded immediately. + // The `isSkippableKey` flag is precomputed so we avoid calling splitCollectionMemberKey here. + if (isFirstConnectionRef.current && isSkippableKey) { + if (resultRef.current[1].status !== 'loaded' || resultRef.current[0] !== undefined) { + resultRef.current = [undefined, {status: 'loaded'}]; + onyxSnapshotCache.setCachedResult>(key, cacheKey, resultRef.current); } + isFirstConnectionRef.current = false; + shouldGetCachedValueRef.current = false; + return resultRef.current; } // Check if we have any cache for this Onyx key // Don't use cache for first connection with initWithStoredValues: false @@ -349,7 +357,7 @@ function useOnyx>( } return resultRef.current; - }, [options?.initWithStoredValues, options?.allowStaleData, options?.canBeMissing, key, memoizedSelector, cacheKey, previousKey]); + }, [options?.initWithStoredValues, options?.allowStaleData, options?.canBeMissing, key, memoizedSelector, cacheKey, previousKey, isSkippableKey]); const subscribe = useCallback( (onStoreChange: () => void) => { From cb47923301f5f0ba3aeea2ab9d3fe988ef99d1f9 Mon Sep 17 00:00:00 2001 From: Artem Makushov Date: Sun, 15 Feb 2026 22:10:15 +0700 Subject: [PATCH 5/7] update comments --- lib/useOnyx.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 533a5d829..dfa2ccf19 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -163,8 +163,7 @@ function useOnyx>( useEffect(() => () => onyxSnapshotCache.deregisterConsumer(key, cacheKey), [key, cacheKey]); - // Precompute whether this key is a skippable collection member so that getSnapshot() - // can use a cheap boolean check instead of calling splitCollectionMemberKey on every render. + // Precompute whether this key is a skippable collection member key. const isSkippableKey = useMemo(() => { const skippableIDs = OnyxUtils.getSkippableCollectionMemberIDs(); if (!skippableIDs.size) { @@ -247,7 +246,6 @@ function useOnyx>( const getSnapshot = useCallback(() => { // Fast path: if subscribing to a skippable collection member id, return undefined as loaded immediately. - // The `isSkippableKey` flag is precomputed so we avoid calling splitCollectionMemberKey here. if (isFirstConnectionRef.current && isSkippableKey) { if (resultRef.current[1].status !== 'loaded' || resultRef.current[0] !== undefined) { resultRef.current = [undefined, {status: 'loaded'}]; From 7b5d5cd81501b386210aa4aad76f2871ee145401 Mon Sep 17 00:00:00 2001 From: Yauheni Pasiukevich Date: Thu, 9 Apr 2026 13:36:16 +0200 Subject: [PATCH 6/7] Refactor OnyxUtils and useOnyx: clean the code --- lib/OnyxUtils.ts | 2 +- lib/useOnyx.ts | 50 +------------------------------------ tests/unit/onyxUtilsTest.ts | 42 ------------------------------- 3 files changed, 2 insertions(+), 92 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 3aa2d0123..efcc12c6d 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1079,7 +1079,7 @@ function subscribeToKey(connectOptions: ConnectOptions>( useEffect(() => () => onyxSnapshotCache.deregisterConsumer(key, cacheKey), [key, cacheKey]); - // Precompute whether this key is a skippable collection member key. - const isSkippableKey = useMemo(() => { - const skippableIDs = OnyxUtils.getSkippableCollectionMemberIDs(); - if (!skippableIDs.size) { - return false; - } - try { - const [, memberId] = OnyxUtils.splitCollectionMemberKey(key); - return skippableIDs.has(memberId); - } catch { - return false; - } - }, [key]); - - useEffect(() => { - // These conditions will ensure we can only handle dynamic collection member keys from the same collection. - if (options?.allowDynamicKey || previousKey === key) { - return; - } - - try { - const previousCollectionKey = OnyxUtils.splitCollectionMemberKey(previousKey)[0]; - const collectionKey = OnyxUtils.splitCollectionMemberKey(key)[0]; - - if (OnyxUtils.isCollectionMemberKey(previousCollectionKey, previousKey) && OnyxUtils.isCollectionMemberKey(collectionKey, key) && previousCollectionKey === collectionKey) { - return; - } - } catch (e) { - throw new Error( - `'${previousKey}' key can't be changed to '${key}'. useOnyx() only supports dynamic keys if they are both collection member keys from the same collection e.g. from 'collection_id1' to 'collection_id2'.`, - ); - } - - throw new Error( - `'${previousKey}' key can't be changed to '${key}'. useOnyx() only supports dynamic keys if they are both collection member keys from the same collection e.g. from 'collection_id1' to 'collection_id2'.`, - ); - }, [previousKey, key, options?.allowDynamicKey]); - // Track previous dependencies to prevent infinite loops const previousDependenciesRef = useRef([]); @@ -228,16 +190,6 @@ function useOnyx>( const lastComputedSelectorRef = useRef(memoizedSelector); const getSnapshot = useCallback(() => { - // Fast path: if subscribing to a skippable collection member id, return undefined as loaded immediately. - if (isFirstConnectionRef.current && isSkippableKey) { - if (resultRef.current[1].status !== 'loaded' || resultRef.current[0] !== undefined) { - resultRef.current = [undefined, {status: 'loaded'}]; - onyxSnapshotCache.setCachedResult>(key, cacheKey, resultRef.current); - } - isFirstConnectionRef.current = false; - shouldGetCachedValueRef.current = false; - return resultRef.current; - } // Check if we have any cache for this Onyx key // Don't use cache for first connection with initWithStoredValues: false // Also don't use cache during active data updates (when shouldGetCachedValueRef is true) @@ -327,7 +279,7 @@ function useOnyx>( } return resultRef.current; - }, [options?.initWithStoredValues, options?.allowStaleData, options?.canBeMissing, key, memoizedSelector, cacheKey, previousKey, isSkippableKey]); + }, [options?.initWithStoredValues, key, memoizedSelector, cacheKey]); const subscribe = useCallback( (onStoreChange: () => void) => { diff --git a/tests/unit/onyxUtilsTest.ts b/tests/unit/onyxUtilsTest.ts index 73d41bf1b..231df9b16 100644 --- a/tests/unit/onyxUtilsTest.ts +++ b/tests/unit/onyxUtilsTest.ts @@ -151,48 +151,6 @@ describe('OnyxUtils', () => { }); }); - describe('splitCollectionMemberKey', () => { - describe('should return correct values', () => { - const dataResult: Record = { - test_: ['test_', ''], - test_level_: ['test_level_', ''], - test_level_1: ['test_level_', '1'], - test_level_2: ['test_level_', '2'], - test_level_last_3: ['test_level_last_', '3'], - test___FAKE__: ['test_', '__FAKE__'], - 'test_-1_something': ['test_', '-1_something'], - 'test_level_-1_something': ['test_level_', '-1_something'], - }; - - it.each(Object.keys(dataResult))('%s', (key) => { - const [collectionKey, id] = OnyxUtils.splitCollectionMemberKey(key); - expect(collectionKey).toEqual(dataResult[key][0]); - expect(id).toEqual(dataResult[key][1]); - }); - }); - - it('should throw error if key does not contain underscore', () => { - expect(() => { - OnyxUtils.splitCollectionMemberKey(ONYXKEYS.TEST_KEY); - }).toThrowError("Invalid 'test' key provided, only collection keys are allowed."); - expect(() => { - OnyxUtils.splitCollectionMemberKey(''); - }).toThrowError("Invalid '' key provided, only collection keys are allowed."); - }); - - it('should allow passing the collection key beforehand for performance gains', () => { - const [collectionKey, id] = OnyxUtils.splitCollectionMemberKey(`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, ONYXKEYS.COLLECTION.TEST_KEY); - expect(collectionKey).toEqual(ONYXKEYS.COLLECTION.TEST_KEY); - expect(id).toEqual('id1'); - }); - - it("should throw error if the passed collection key isn't compatible with the key", () => { - expect(() => { - OnyxUtils.splitCollectionMemberKey(`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, ONYXKEYS.COLLECTION.TEST_LEVEL_KEY); - }).toThrowError("Invalid 'test_level_' collection key provided, it isn't compatible with 'test_id1' key."); - }); - }); - describe('partialSetCollection', () => { beforeEach(() => { Onyx.clear(); From 0c80c8434fbfb8bed90820f9f1e416b707bd2d27 Mon Sep 17 00:00:00 2001 From: Yauheni Pasiukevich Date: Thu, 9 Apr 2026 18:25:00 +0200 Subject: [PATCH 7/7] Refactor subscribeToKey function in OnyxUtils to improve code clarity by removing unnecessary comments and unused code related to provisional mapping cleanup. --- lib/OnyxUtils.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index efcc12c6d..db39ab735 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1081,10 +1081,8 @@ function subscribeToKey(connectOptions: ConnectOptions); delete callbackToStateMapping[subscriptionID]; return subscriptionID; }