From 397b670e76327ca9705be5af43659f8aad2b44c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Wed, 19 Mar 2025 22:18:27 +0000 Subject: [PATCH 01/15] Initial implementation of canBeMissing option --- lib/useOnyx.ts | 16 ++++- tests/unit/useOnyxTest.ts | 122 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 2 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index e95b5faad..a841da132 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -10,6 +10,7 @@ import type {CollectionKeyBase, KeyValueMapping, OnyxCollection, OnyxKey, OnyxVa import useLiveRef from './useLiveRef'; import usePrevious from './usePrevious'; import decorateWithMetrics from './metrics'; +import * as Logger from './Logger'; type BaseUseOnyxOptions = { /** @@ -37,6 +38,11 @@ type BaseUseOnyxOptions = { * If set to `true`, the key can be changed dynamically during the component lifecycle. */ allowDynamicKey?: boolean; + + /** + * If set to `false`, the hook will log an alert when Onyx doesn't return data for that key. + */ + canBeMissing?: boolean; }; type UseOnyxInitialValueOption = { @@ -280,11 +286,17 @@ function useOnyx>( previousValueRef.current = newValueRef.current; // If the new value is `null` we default it to `undefined` to ensure the consumer gets a consistent result from the hook. - resultRef.current = [previousValueRef.current ?? undefined, {status: newFetchStatus ?? 'loaded'}]; + const newValue = previousValueRef.current ?? undefined; + const newStatus = newFetchStatus ?? 'loaded'; + resultRef.current = [previousValueRef.current ?? undefined, {status: newStatus}]; + + if (options?.canBeMissing === false && newStatus === 'loaded' && newValue === undefined) { + Logger.logAlert(`useOnyx returned no data for key '${key}' while canBeMissing was set to false.`); + } } return resultRef.current; - }, [options?.initWithStoredValues, options?.allowStaleData, options?.initialValue, key, selectorRef]); + }, [options?.initWithStoredValues, options?.allowStaleData, options?.initialValue, options?.canBeMissing, key, selectorRef]); const subscribe = useCallback( (onStoreChange: () => void) => { diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 7a674cf77..bb6db87d5 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -5,6 +5,7 @@ import OnyxUtils from '../../lib/OnyxUtils'; import StorageMock from '../../lib/storage'; import type GenericCollection from '../utils/GenericCollection'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; +import * as Logger from '../../lib/Logger'; const ONYXKEYS = { TEST_KEY: 'test', @@ -749,6 +750,127 @@ describe('useOnyx', () => { }); }); + describe('canBeMissing', () => { + let logAlertFn = jest.fn(); + const alert = (key: string) => `useOnyx returned no data for key '${key}' while canBeMissing was set to false.`; + + beforeEach(() => { + logAlertFn = jest.fn(); + jest.spyOn(Logger, 'logAlert').mockImplementation(logAlertFn); + }); + + afterEach(() => { + (Logger.logAlert as unknown as jest.SpyInstance>).mockRestore(); + }); + + it('should not log an alert if Onyx doesn\'t return data in loaded state and "canBeMissing" property is not provided', async () => { + const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + + expect(result1.current[0]).toBeUndefined(); + expect(result1.current[1].status).toEqual('loading'); + + await act(async () => waitForPromisesToResolve()); + + expect(result1.current[0]).toBeUndefined(); + expect(result1.current[1].status).toEqual('loaded'); + expect(logAlertFn).not.toBeCalledWith(alert(ONYXKEYS.TEST_KEY)); + + await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test')); + + expect(result1.current[0]).toBe('test'); + + await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, null)); + + expect(result1.current[0]).toBeUndefined(); + expect(logAlertFn).not.toBeCalledWith(alert(ONYXKEYS.TEST_KEY)); + }); + + it('should not log an alert if Onyx doesn\'t return data, "canBeMissing" property is false but "initWithStoredValues" is also false', async () => { + const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY, {canBeMissing: false, initWithStoredValues: false})); + + expect(result1.current[0]).toBeUndefined(); + expect(result1.current[1].status).toEqual('loaded'); + + expect(logAlertFn).not.toBeCalledWith(alert(ONYXKEYS.TEST_KEY)); + }); + + it('should log an alert if Onyx doesn\'t return data in loaded state and "canBeMissing" property is false', async () => { + const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY, {canBeMissing: false})); + + expect(result1.current[0]).toBeUndefined(); + expect(result1.current[1].status).toEqual('loading'); + + await act(async () => waitForPromisesToResolve()); + + expect(result1.current[0]).toBeUndefined(); + expect(result1.current[1].status).toEqual('loaded'); + expect(logAlertFn).toHaveBeenNthCalledWith(1, alert(ONYXKEYS.TEST_KEY)); + + await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test')); + + expect(result1.current[0]).toBe('test'); + + await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, null)); + + expect(result1.current[0]).toBeUndefined(); + expect(logAlertFn).toHaveBeenNthCalledWith(2, alert(ONYXKEYS.TEST_KEY)); + }); + + it('should log an alert if Onyx doesn\'t return selected data in loaded state and "canBeMissing" property is false', async () => { + const {result: result1} = renderHook(() => + useOnyx(ONYXKEYS.TEST_KEY, { + // @ts-expect-error bypass + selector: (entry: OnyxEntry) => (entry ? `${entry}_changed` : undefined), + canBeMissing: false, + }), + ); + + expect(result1.current[0]).toBeUndefined(); + expect(result1.current[1].status).toEqual('loading'); + + await act(async () => waitForPromisesToResolve()); + + expect(result1.current[0]).toBeUndefined(); + expect(result1.current[1].status).toEqual('loaded'); + expect(logAlertFn).toHaveBeenNthCalledWith(1, alert(ONYXKEYS.TEST_KEY)); + + await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test')); + + expect(result1.current[0]).toBe('test_changed'); + + await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, null)); + + expect(result1.current[0]).toBeUndefined(); + expect(logAlertFn).toHaveBeenNthCalledWith(2, alert(ONYXKEYS.TEST_KEY)); + }); + + it('should not log an alert if Onyx doesn\'t return data but there is a selector that always return something and "canBeMissing" property is false', async () => { + const {result: result1} = renderHook(() => + useOnyx(ONYXKEYS.TEST_KEY, { + // @ts-expect-error bypass + // This selector will always return a value, even if the Onyx data is missing. + selector: (entry: OnyxEntry) => `${entry}_changed`, + canBeMissing: false, + }), + ); + + await act(async () => waitForPromisesToResolve()); + + expect(result1.current[0]).toBe('undefined_changed'); + expect(result1.current[1].status).toEqual('loaded'); + expect(logAlertFn).not.toBeCalledWith(alert(ONYXKEYS.TEST_KEY)); + + await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test')); + + expect(result1.current[0]).toBe('test_changed'); + + await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, null)); + + expect(result1.current[0]).toBe('undefined_changed'); + expect(logAlertFn).not.toBeCalledWith(alert(ONYXKEYS.TEST_KEY)); + }); + }); + // This test suite must be the last one to avoid problems when running the other tests here. describe('canEvict', () => { const error = (key: string) => `canEvict can't be used on key '${key}'. This key must explicitly be flagged as safe for removal by adding it to Onyx.init({safeEvictionKeys: []}).`; From aed81f4c5bdb3676fa25b5d024cedfac37dc22f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Wed, 19 Mar 2025 22:23:13 +0000 Subject: [PATCH 02/15] Improve tests --- tests/unit/useOnyxTest.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index bb6db87d5..659ef5c8c 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -799,11 +799,13 @@ describe('useOnyx', () => { expect(result1.current[0]).toBeUndefined(); expect(result1.current[1].status).toEqual('loading'); + expect(logAlertFn).not.toBeCalledWith(alert(ONYXKEYS.TEST_KEY)); await act(async () => waitForPromisesToResolve()); expect(result1.current[0]).toBeUndefined(); expect(result1.current[1].status).toEqual('loaded'); + expect(logAlertFn).toHaveBeenCalledTimes(1); expect(logAlertFn).toHaveBeenNthCalledWith(1, alert(ONYXKEYS.TEST_KEY)); await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test')); @@ -813,6 +815,7 @@ describe('useOnyx', () => { await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, null)); expect(result1.current[0]).toBeUndefined(); + expect(logAlertFn).toHaveBeenCalledTimes(2); expect(logAlertFn).toHaveBeenNthCalledWith(2, alert(ONYXKEYS.TEST_KEY)); }); @@ -827,11 +830,13 @@ describe('useOnyx', () => { expect(result1.current[0]).toBeUndefined(); expect(result1.current[1].status).toEqual('loading'); + expect(logAlertFn).not.toBeCalledWith(alert(ONYXKEYS.TEST_KEY)); await act(async () => waitForPromisesToResolve()); expect(result1.current[0]).toBeUndefined(); expect(result1.current[1].status).toEqual('loaded'); + expect(logAlertFn).toHaveBeenCalledTimes(1); expect(logAlertFn).toHaveBeenNthCalledWith(1, alert(ONYXKEYS.TEST_KEY)); await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test')); @@ -841,6 +846,7 @@ describe('useOnyx', () => { await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, null)); expect(result1.current[0]).toBeUndefined(); + expect(logAlertFn).toHaveBeenCalledTimes(2); expect(logAlertFn).toHaveBeenNthCalledWith(2, alert(ONYXKEYS.TEST_KEY)); }); From 0f777a3dcf2574ff38f159041fe3487eb5e7ba12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Sun, 23 Mar 2025 12:26:28 +0000 Subject: [PATCH 03/15] Change logic to alert even if selector always return value --- lib/useOnyx.ts | 35 +++++++++++++++-------------------- tests/unit/useOnyxTest.ts | 8 +++++--- 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index a841da132..05cac1f81 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -104,17 +104,6 @@ function tryGetCachedValue(key: TKey): OnyxValue return values; } -/** - * Gets the value from cache and maps it with selector. It changes `null` to `undefined` for `useOnyx` compatibility. - */ -function getCachedValue(key: TKey, selector?: UseOnyxSelector) { - const value = tryGetCachedValue(key) as OnyxValue; - - const selectedValue = selector ? selector(value) : (value as TValue); - - return selectedValue ?? undefined; -} - function useOnyx>( key: TKey, options?: BaseUseOnyxOptions & UseOnyxInitialValueOption & Required>, @@ -225,6 +214,8 @@ function useOnyx>( }, [key, options?.canEvict]); const getSnapshot = useCallback(() => { + let isOnyxValueDefined = true; + // We return the initial result right away during the first connection if `initWithStoredValues` is set to `false`. if (isFirstConnectionRef.current && options?.initWithStoredValues === false) { return resultRef.current; @@ -234,11 +225,14 @@ function useOnyx>( // so we can return any cached value right away. After the connection is made, we only // update `newValueRef` when `Onyx.connect()` callback is fired. if (isFirstConnectionRef.current || shouldGetCachedValueRef.current) { - // If `newValueRef.current` is `undefined` it means that the cache doesn't have a value for that key yet. - // If `newValueRef.current` is `null` or any other value it means that the cache does have a value for that key. - // This difference between `undefined` and other values is crucial and it's used to address the following - // conditions and use cases. - newValueRef.current = getCachedValue(key, selectorRef.current); + // Gets the value from cache and maps it with selector. It changes `null` to `undefined` for `useOnyx` compatibility. + const value = tryGetCachedValue(key) as OnyxValue; + const selectedValue = selectorRef.current ? selectorRef.current(value) : value; + newValueRef.current = (selectedValue ?? undefined) as TReturnValue | undefined; + + // This flag is `false` when the original Onyx value (without selector) is not defined yet. + // It will be used later to check if we need to log an alert that the value is missing. + isOnyxValueDefined = value !== null && value !== undefined; // We set this flag to `false` again since we don't want to get the newest cached value every time `getSnapshot()` is executed, // and only when `Onyx.connect()` callback is fired. @@ -261,7 +255,7 @@ function useOnyx>( // If data is not present in cache and `initialValue` is set during the first connection, // we set the new value to `initialValue` and fetch status to `loaded` since we already have some data to return to the consumer. if (isFirstConnectionRef.current && !hasCacheForKey && options?.initialValue !== undefined) { - newValueRef.current = (options?.initialValue ?? undefined) as TReturnValue; + newValueRef.current = options.initialValue; newFetchStatus = 'loaded'; } @@ -275,7 +269,7 @@ function useOnyx>( areValuesEqual = shallowEqual(previousValueRef.current ?? undefined, newValueRef.current); } - // We updated the cached value and the result in the following conditions: + // We update the cached value and the result in the following conditions: // We will update the cached value and the result in any of the following situations: // - The previously cached value is different from the new value. // - The previously cached value is `null` (not set from cache yet) and we have cache for this key @@ -286,11 +280,12 @@ function useOnyx>( previousValueRef.current = newValueRef.current; // If the new value is `null` we default it to `undefined` to ensure the consumer gets a consistent result from the hook. - const newValue = previousValueRef.current ?? undefined; const newStatus = newFetchStatus ?? 'loaded'; resultRef.current = [previousValueRef.current ?? undefined, {status: newStatus}]; - if (options?.canBeMissing === false && newStatus === 'loaded' && newValue === undefined) { + // If `canBeMissing` is set to `false` and the Onyx value of that key is not defined, + // we log an alert so it can be acknowledged by the consumer. + if (options?.canBeMissing === false && newStatus === 'loaded' && !isOnyxValueDefined) { Logger.logAlert(`useOnyx returned no data for key '${key}' while canBeMissing was set to false.`); } } diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 659ef5c8c..a3f24d296 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -850,7 +850,7 @@ describe('useOnyx', () => { expect(logAlertFn).toHaveBeenNthCalledWith(2, alert(ONYXKEYS.TEST_KEY)); }); - it('should not log an alert if Onyx doesn\'t return data but there is a selector that always return something and "canBeMissing" property is false', async () => { + it('should log an alert if Onyx doesn\'t return data but there is a selector that always return something and "canBeMissing" property is false', async () => { const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY, { // @ts-expect-error bypass @@ -864,7 +864,8 @@ describe('useOnyx', () => { expect(result1.current[0]).toBe('undefined_changed'); expect(result1.current[1].status).toEqual('loaded'); - expect(logAlertFn).not.toBeCalledWith(alert(ONYXKEYS.TEST_KEY)); + expect(logAlertFn).toHaveBeenCalledTimes(1); + expect(logAlertFn).toHaveBeenNthCalledWith(1, alert(ONYXKEYS.TEST_KEY)); await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test')); @@ -873,7 +874,8 @@ describe('useOnyx', () => { await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, null)); expect(result1.current[0]).toBe('undefined_changed'); - expect(logAlertFn).not.toBeCalledWith(alert(ONYXKEYS.TEST_KEY)); + expect(logAlertFn).toHaveBeenCalledTimes(2); + expect(logAlertFn).toHaveBeenNthCalledWith(2, alert(ONYXKEYS.TEST_KEY)); }); }); From c5eb697d6b5c1a47e13cc14d67e49c1baf1ed98f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Sun, 23 Mar 2025 12:26:57 +0000 Subject: [PATCH 04/15] Add support to send extra data while logging --- lib/Logger.ts | 13 +++++++------ lib/useOnyx.ts | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/Logger.ts b/lib/Logger.ts index a1fff8332..a3c54bc0c 100644 --- a/lib/Logger.ts +++ b/lib/Logger.ts @@ -1,6 +1,7 @@ type LogData = { message: string; level: 'alert' | 'info' | 'hmmm'; + extraData?: Record; }; type LoggerCallback = (data: LogData) => void; @@ -17,22 +18,22 @@ function registerLogger(callback: LoggerCallback) { /** * Send an alert message to the logger */ -function logAlert(message: string) { - logger({message: `[Onyx] ${message}`, level: 'alert'}); +function logAlert(message: string, extraData?: Record) { + logger({message: `[Onyx] ${message}`, level: 'alert', extraData}); } /** * Send an info message to the logger */ -function logInfo(message: string) { - logger({message: `[Onyx] ${message}`, level: 'info'}); +function logInfo(message: string, extraData?: Record) { + logger({message: `[Onyx] ${message}`, level: 'info', extraData}); } /** * Send an hmmm message to the logger */ -function logHmmm(message: string) { - logger({message: `[Onyx] ${message}`, level: 'hmmm'}); +function logHmmm(message: string, extraData?: Record) { + logger({message: `[Onyx] ${message}`, level: 'hmmm', extraData}); } export {registerLogger, logInfo, logAlert, logHmmm}; diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 05cac1f81..84996c480 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -286,7 +286,7 @@ function useOnyx>( // If `canBeMissing` is set to `false` and the Onyx value of that key is not defined, // we log an alert so it can be acknowledged by the consumer. if (options?.canBeMissing === false && newStatus === 'loaded' && !isOnyxValueDefined) { - Logger.logAlert(`useOnyx returned no data for key '${key}' while canBeMissing was set to false.`); + Logger.logAlert(`useOnyx returned no data for key '${key}' while canBeMissing was set to false.`, {showAlert: true}); } } From 93cf87a249473591c9b4f57663ed19018014fb4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 24 Mar 2025 18:02:11 +0000 Subject: [PATCH 05/15] Fix tests --- tests/unit/useOnyxTest.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index a3f24d296..4eb908fe3 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -806,7 +806,7 @@ describe('useOnyx', () => { expect(result1.current[0]).toBeUndefined(); expect(result1.current[1].status).toEqual('loaded'); expect(logAlertFn).toHaveBeenCalledTimes(1); - expect(logAlertFn).toHaveBeenNthCalledWith(1, alert(ONYXKEYS.TEST_KEY)); + expect(logAlertFn).toHaveBeenNthCalledWith(1, alert(ONYXKEYS.TEST_KEY), {showAlert: true}); await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test')); @@ -816,7 +816,7 @@ describe('useOnyx', () => { expect(result1.current[0]).toBeUndefined(); expect(logAlertFn).toHaveBeenCalledTimes(2); - expect(logAlertFn).toHaveBeenNthCalledWith(2, alert(ONYXKEYS.TEST_KEY)); + expect(logAlertFn).toHaveBeenNthCalledWith(2, alert(ONYXKEYS.TEST_KEY), {showAlert: true}); }); it('should log an alert if Onyx doesn\'t return selected data in loaded state and "canBeMissing" property is false', async () => { @@ -837,7 +837,7 @@ describe('useOnyx', () => { expect(result1.current[0]).toBeUndefined(); expect(result1.current[1].status).toEqual('loaded'); expect(logAlertFn).toHaveBeenCalledTimes(1); - expect(logAlertFn).toHaveBeenNthCalledWith(1, alert(ONYXKEYS.TEST_KEY)); + expect(logAlertFn).toHaveBeenNthCalledWith(1, alert(ONYXKEYS.TEST_KEY), {showAlert: true}); await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test')); @@ -847,7 +847,7 @@ describe('useOnyx', () => { expect(result1.current[0]).toBeUndefined(); expect(logAlertFn).toHaveBeenCalledTimes(2); - expect(logAlertFn).toHaveBeenNthCalledWith(2, alert(ONYXKEYS.TEST_KEY)); + expect(logAlertFn).toHaveBeenNthCalledWith(2, alert(ONYXKEYS.TEST_KEY), {showAlert: true}); }); it('should log an alert if Onyx doesn\'t return data but there is a selector that always return something and "canBeMissing" property is false', async () => { @@ -865,7 +865,7 @@ describe('useOnyx', () => { expect(result1.current[0]).toBe('undefined_changed'); expect(result1.current[1].status).toEqual('loaded'); expect(logAlertFn).toHaveBeenCalledTimes(1); - expect(logAlertFn).toHaveBeenNthCalledWith(1, alert(ONYXKEYS.TEST_KEY)); + expect(logAlertFn).toHaveBeenNthCalledWith(1, alert(ONYXKEYS.TEST_KEY), {showAlert: true}); await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test')); @@ -875,7 +875,7 @@ describe('useOnyx', () => { expect(result1.current[0]).toBe('undefined_changed'); expect(logAlertFn).toHaveBeenCalledTimes(2); - expect(logAlertFn).toHaveBeenNthCalledWith(2, alert(ONYXKEYS.TEST_KEY)); + expect(logAlertFn).toHaveBeenNthCalledWith(2, alert(ONYXKEYS.TEST_KEY), {showAlert: true}); }); }); From 2ab712c0522ba5fa81f66060440c2d4ce3176d16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 25 Mar 2025 10:45:38 +0000 Subject: [PATCH 06/15] Update comment --- lib/useOnyx.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 84996c480..0375945dd 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -40,7 +40,10 @@ type BaseUseOnyxOptions = { allowDynamicKey?: boolean; /** - * If set to `false`, the hook will log an alert when Onyx doesn't return data for that key. + * If the component calling this is the one loading the data by calling an action, then you should set this to `true`. + * + * If the component calling this does not load the data then you should set it to `false`, which means that if the data + * is not there, it will log an alert, as it means we are using data that no one loaded and that's most probably a bug. */ canBeMissing?: boolean; }; From 0b58ab42eb7a84b094836e11f708072c15a05c4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 25 Mar 2025 10:46:21 +0000 Subject: [PATCH 07/15] Build docs --- API-INTERNAL.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/API-INTERNAL.md b/API-INTERNAL.md index 0dc8b5d84..ed157fc23 100644 --- a/API-INTERNAL.md +++ b/API-INTERNAL.md @@ -43,6 +43,11 @@ The resulting collection will only contain items that are returned by the select
get()

Get some data from the store

+
tupleGet()
+

This helper exists to map an array of Onyx keys such as ['report_', 'conciergeReportID'] +to the values for those keys (correctly typed) such as [OnyxCollection<Report>, OnyxEntry<string>]

+

Note: just using .map, you'd end up with Array<OnyxCollection<Report>|OnyxEntry<string>>, which is not what we want. This preserves the order of the keys provided.

+
storeKeyBySubscriptions(subscriptionID, key)

Stores a subscription ID associated with a given key.

@@ -241,6 +246,15 @@ The resulting collection will only contain items that are returned by the select ## get() Get some data from the store +**Kind**: global function + + +## tupleGet() +This helper exists to map an array of Onyx keys such as `['report_', 'conciergeReportID']` +to the values for those keys (correctly typed) such as `[OnyxCollection, OnyxEntry]` + +Note: just using .map, you'd end up with `Array|OnyxEntry>`, which is not what we want. This preserves the order of the keys provided. + **Kind**: global function From d33e478910cb426e6b46c15f41d8c52448e97cd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 25 Mar 2025 11:42:03 +0000 Subject: [PATCH 08/15] Add section about canBeMissing --- README.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/README.md b/README.md index f82919919..4c149bc68 100644 --- a/README.md +++ b/README.md @@ -256,6 +256,21 @@ DO NOT use more than one `withOnyx` component at a time. It adds overhead and pr It's also beneficial to use a [selector](https://github.com/Expensify/react-native-onyx/blob/main/API.md#connectmapping--number) with the mapping in case you need to grab a single item in a collection (like a single report action). +### useOnyx()'s `canBeMissing` option + +You can pass the `canBeMissing` configuration flag to `useOnyx` if you want the hook to log an alert when data is missing from Onyx store. Regarding usage in `Expensify/App` repo, if the component calling this is the one loading the data by calling an action, then you should set this to `true`. If the component calling this does not load the data then you should set it to `false`, which means that if the data is not there, it will log an alert, as it means we are using data that no one loaded and that's most probably a bug. + +```javascript +const Component = ({reportID}) => { + // This hook will log an alert (via `Logger.logAlert()`) if `report` is `undefined`. + const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {canBeMissing: false}); + + // rest of the component's code. +}; + +export default Component; +``` + ## Collections Collections allow keys with similar value types to be subscribed together by subscribing to the collection key. To define one, it must be included in the `ONYXKEYS.COLLECTION` object and it must be suffixed with an underscore. Member keys should use a unique identifier or index after the collection key prefix (e.g. `report_42`). From e85307e8cfc188e60540cbaf9d259141ccba5cfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 25 Mar 2025 18:00:24 +0000 Subject: [PATCH 09/15] Use same parameters signarue from expensify-common --- lib/Logger.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/Logger.ts b/lib/Logger.ts index a3c54bc0c..922b1dd0b 100644 --- a/lib/Logger.ts +++ b/lib/Logger.ts @@ -1,7 +1,9 @@ +type Parameters = string | Record | Array> | Error; + type LogData = { message: string; level: 'alert' | 'info' | 'hmmm'; - extraData?: Record; + parameters?: Parameters; }; type LoggerCallback = (data: LogData) => void; @@ -18,22 +20,22 @@ function registerLogger(callback: LoggerCallback) { /** * Send an alert message to the logger */ -function logAlert(message: string, extraData?: Record) { - logger({message: `[Onyx] ${message}`, level: 'alert', extraData}); +function logAlert(message: string, parameters?: Parameters) { + logger({message: `[Onyx] ${message}`, level: 'alert', parameters}); } /** * Send an info message to the logger */ -function logInfo(message: string, extraData?: Record) { - logger({message: `[Onyx] ${message}`, level: 'info', extraData}); +function logInfo(message: string, parameters?: Parameters) { + logger({message: `[Onyx] ${message}`, level: 'info', parameters}); } /** * Send an hmmm message to the logger */ -function logHmmm(message: string, extraData?: Record) { - logger({message: `[Onyx] ${message}`, level: 'hmmm', extraData}); +function logHmmm(message: string, parameters?: Parameters) { + logger({message: `[Onyx] ${message}`, level: 'hmmm', parameters}); } export {registerLogger, logInfo, logAlert, logHmmm}; From 82e58820543fab9be1ed8130adfd8972cc2533a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 25 Mar 2025 18:03:06 +0000 Subject: [PATCH 10/15] Change logAlert message and parameters --- lib/useOnyx.ts | 2 +- tests/unit/useOnyxTest.ts | 25 +++++++++++++------------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 0375945dd..7b8e4f99a 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -289,7 +289,7 @@ function useOnyx>( // If `canBeMissing` is set to `false` and the Onyx value of that key is not defined, // we log an alert so it can be acknowledged by the consumer. if (options?.canBeMissing === false && newStatus === 'loaded' && !isOnyxValueDefined) { - Logger.logAlert(`useOnyx returned no data for key '${key}' while canBeMissing was set to false.`, {showAlert: true}); + Logger.logAlert(`useOnyx returned no data for key with canBeMissing set to false.`, {key, showAlert: true}); } } diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 4eb908fe3..775ea61cb 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -752,7 +752,8 @@ describe('useOnyx', () => { describe('canBeMissing', () => { let logAlertFn = jest.fn(); - const alert = (key: string) => `useOnyx returned no data for key '${key}' while canBeMissing was set to false.`; + // const alert = (key: string) => `useOnyx returned no data for key '${key}' while canBeMissing was set to false.`; + const alertMessage = 'useOnyx returned no data for key with canBeMissing set to false.'; beforeEach(() => { logAlertFn = jest.fn(); @@ -773,7 +774,7 @@ describe('useOnyx', () => { expect(result1.current[0]).toBeUndefined(); expect(result1.current[1].status).toEqual('loaded'); - expect(logAlertFn).not.toBeCalledWith(alert(ONYXKEYS.TEST_KEY)); + expect(logAlertFn).not.toBeCalledWith(alertMessage); await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test')); @@ -782,7 +783,7 @@ describe('useOnyx', () => { await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, null)); expect(result1.current[0]).toBeUndefined(); - expect(logAlertFn).not.toBeCalledWith(alert(ONYXKEYS.TEST_KEY)); + expect(logAlertFn).not.toBeCalledWith(alertMessage); }); it('should not log an alert if Onyx doesn\'t return data, "canBeMissing" property is false but "initWithStoredValues" is also false', async () => { @@ -791,7 +792,7 @@ describe('useOnyx', () => { expect(result1.current[0]).toBeUndefined(); expect(result1.current[1].status).toEqual('loaded'); - expect(logAlertFn).not.toBeCalledWith(alert(ONYXKEYS.TEST_KEY)); + expect(logAlertFn).not.toBeCalledWith(alertMessage); }); it('should log an alert if Onyx doesn\'t return data in loaded state and "canBeMissing" property is false', async () => { @@ -799,14 +800,14 @@ describe('useOnyx', () => { expect(result1.current[0]).toBeUndefined(); expect(result1.current[1].status).toEqual('loading'); - expect(logAlertFn).not.toBeCalledWith(alert(ONYXKEYS.TEST_KEY)); + expect(logAlertFn).not.toBeCalledWith(alertMessage); await act(async () => waitForPromisesToResolve()); expect(result1.current[0]).toBeUndefined(); expect(result1.current[1].status).toEqual('loaded'); expect(logAlertFn).toHaveBeenCalledTimes(1); - expect(logAlertFn).toHaveBeenNthCalledWith(1, alert(ONYXKEYS.TEST_KEY), {showAlert: true}); + expect(logAlertFn).toHaveBeenNthCalledWith(1, alertMessage, {key: ONYXKEYS.TEST_KEY, showAlert: true}); await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test')); @@ -816,7 +817,7 @@ describe('useOnyx', () => { expect(result1.current[0]).toBeUndefined(); expect(logAlertFn).toHaveBeenCalledTimes(2); - expect(logAlertFn).toHaveBeenNthCalledWith(2, alert(ONYXKEYS.TEST_KEY), {showAlert: true}); + expect(logAlertFn).toHaveBeenNthCalledWith(2, alertMessage, {key: ONYXKEYS.TEST_KEY, showAlert: true}); }); it('should log an alert if Onyx doesn\'t return selected data in loaded state and "canBeMissing" property is false', async () => { @@ -830,14 +831,14 @@ describe('useOnyx', () => { expect(result1.current[0]).toBeUndefined(); expect(result1.current[1].status).toEqual('loading'); - expect(logAlertFn).not.toBeCalledWith(alert(ONYXKEYS.TEST_KEY)); + expect(logAlertFn).not.toBeCalledWith(alertMessage); await act(async () => waitForPromisesToResolve()); expect(result1.current[0]).toBeUndefined(); expect(result1.current[1].status).toEqual('loaded'); expect(logAlertFn).toHaveBeenCalledTimes(1); - expect(logAlertFn).toHaveBeenNthCalledWith(1, alert(ONYXKEYS.TEST_KEY), {showAlert: true}); + expect(logAlertFn).toHaveBeenNthCalledWith(1, alertMessage, {key: ONYXKEYS.TEST_KEY, showAlert: true}); await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test')); @@ -847,7 +848,7 @@ describe('useOnyx', () => { expect(result1.current[0]).toBeUndefined(); expect(logAlertFn).toHaveBeenCalledTimes(2); - expect(logAlertFn).toHaveBeenNthCalledWith(2, alert(ONYXKEYS.TEST_KEY), {showAlert: true}); + expect(logAlertFn).toHaveBeenNthCalledWith(2, alertMessage, {key: ONYXKEYS.TEST_KEY, showAlert: true}); }); it('should log an alert if Onyx doesn\'t return data but there is a selector that always return something and "canBeMissing" property is false', async () => { @@ -865,7 +866,7 @@ describe('useOnyx', () => { expect(result1.current[0]).toBe('undefined_changed'); expect(result1.current[1].status).toEqual('loaded'); expect(logAlertFn).toHaveBeenCalledTimes(1); - expect(logAlertFn).toHaveBeenNthCalledWith(1, alert(ONYXKEYS.TEST_KEY), {showAlert: true}); + expect(logAlertFn).toHaveBeenNthCalledWith(1, alertMessage, {key: ONYXKEYS.TEST_KEY, showAlert: true}); await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test')); @@ -875,7 +876,7 @@ describe('useOnyx', () => { expect(result1.current[0]).toBe('undefined_changed'); expect(logAlertFn).toHaveBeenCalledTimes(2); - expect(logAlertFn).toHaveBeenNthCalledWith(2, alert(ONYXKEYS.TEST_KEY), {showAlert: true}); + expect(logAlertFn).toHaveBeenNthCalledWith(2, alertMessage, {key: ONYXKEYS.TEST_KEY, showAlert: true}); }); }); From 8aed23718b7afb2936b839cf5c46a31e0f1abaec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 25 Mar 2025 18:04:36 +0000 Subject: [PATCH 11/15] Minor adjustment in docs --- API-INTERNAL.md | 4 ++-- lib/OnyxUtils.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/API-INTERNAL.md b/API-INTERNAL.md index ed157fc23..d8665de66 100644 --- a/API-INTERNAL.md +++ b/API-INTERNAL.md @@ -46,7 +46,7 @@ The resulting collection will only contain items that are returned by the select
tupleGet()

This helper exists to map an array of Onyx keys such as ['report_', 'conciergeReportID'] to the values for those keys (correctly typed) such as [OnyxCollection<Report>, OnyxEntry<string>]

-

Note: just using .map, you'd end up with Array<OnyxCollection<Report>|OnyxEntry<string>>, which is not what we want. This preserves the order of the keys provided.

+

Note: just using .map, you'd end up with Array<OnyxCollection<Report>|OnyxEntry<string>>, which is not what we want. This preserves the order of the keys provided.

storeKeyBySubscriptions(subscriptionID, key)

Stores a subscription ID associated with a given key.

@@ -253,7 +253,7 @@ Get some data from the store This helper exists to map an array of Onyx keys such as `['report_', 'conciergeReportID']` to the values for those keys (correctly typed) such as `[OnyxCollection, OnyxEntry]` -Note: just using .map, you'd end up with `Array|OnyxEntry>`, which is not what we want. This preserves the order of the keys provided. +Note: just using `.map`, you'd end up with `Array|OnyxEntry>`, which is not what we want. This preserves the order of the keys provided. **Kind**: global function diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index de1c4eada..30cf1a703 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -390,7 +390,7 @@ function multiGet(keys: CollectionKeyBase[]): Promise, OnyxEntry]` * - * Note: just using .map, you'd end up with `Array|OnyxEntry>`, which is not what we want. This preserves the order of the keys provided. + * Note: just using `.map`, you'd end up with `Array|OnyxEntry>`, which is not what we want. This preserves the order of the keys provided. */ function tupleGet(keys: Keys): Promise<{[Index in keyof Keys]: OnyxValue}> { return Promise.all(keys.map((key) => OnyxUtils.get(key))) as Promise<{[Index in keyof Keys]: OnyxValue}>; From 7f814b2c50f4a96be6ab89f2cbbb766bff68e27b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 25 Mar 2025 18:56:36 +0000 Subject: [PATCH 12/15] Update tests/unit/useOnyxTest.ts Co-authored-by: Amy Evans --- tests/unit/useOnyxTest.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 775ea61cb..284ae24cc 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -752,7 +752,6 @@ describe('useOnyx', () => { describe('canBeMissing', () => { let logAlertFn = jest.fn(); - // const alert = (key: string) => `useOnyx returned no data for key '${key}' while canBeMissing was set to false.`; const alertMessage = 'useOnyx returned no data for key with canBeMissing set to false.'; beforeEach(() => { From 88cc5473f7ebdfe4735319d9c5738909cd43129f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 25 Mar 2025 18:57:41 +0000 Subject: [PATCH 13/15] Update tests/unit/useOnyxTest.ts Co-authored-by: Amy Evans --- tests/unit/useOnyxTest.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 284ae24cc..cf7337df6 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -752,7 +752,6 @@ describe('useOnyx', () => { describe('canBeMissing', () => { let logAlertFn = jest.fn(); - const alertMessage = 'useOnyx returned no data for key with canBeMissing set to false.'; beforeEach(() => { logAlertFn = jest.fn(); From 7e0f88cb29a0d2cc387a546f0c55b7952a30e2f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 25 Mar 2025 18:59:03 +0000 Subject: [PATCH 14/15] Rollback GH mess --- tests/unit/useOnyxTest.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index cf7337df6..284ae24cc 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -752,6 +752,7 @@ describe('useOnyx', () => { describe('canBeMissing', () => { let logAlertFn = jest.fn(); + const alertMessage = 'useOnyx returned no data for key with canBeMissing set to false.'; beforeEach(() => { logAlertFn = jest.fn(); From 3740788c0e6a8e0da4f826a379bcf727a943cc11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 25 Mar 2025 19:06:36 +0000 Subject: [PATCH 15/15] Update README.md Co-authored-by: Ionatan Wiznia --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 4c149bc68..67c7b5590 100644 --- a/README.md +++ b/README.md @@ -258,7 +258,7 @@ It's also beneficial to use a [selector](https://github.com/Expensify/react-nati ### useOnyx()'s `canBeMissing` option -You can pass the `canBeMissing` configuration flag to `useOnyx` if you want the hook to log an alert when data is missing from Onyx store. Regarding usage in `Expensify/App` repo, if the component calling this is the one loading the data by calling an action, then you should set this to `true`. If the component calling this does not load the data then you should set it to `false`, which means that if the data is not there, it will log an alert, as it means we are using data that no one loaded and that's most probably a bug. +You must pass the `canBeMissing` configuration flag to `useOnyx` if you want the hook to log an alert when data is missing from Onyx store. Regarding usage in `Expensify/App` repo, if the component calling this is the one loading the data by calling an action, then you should set this to `true`. If the component calling this does not load the data then you should set it to `false`, which means that if the data is not there, it will log an alert, as it means we are using data that no one loaded and that's most probably a bug. ```javascript const Component = ({reportID}) => {