diff --git a/API-INTERNAL.md b/API-INTERNAL.md index 0dc8b5d84..d8665de66 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 diff --git a/README.md b/README.md index f82919919..67c7b5590 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 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}) => { + // 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`). diff --git a/lib/Logger.ts b/lib/Logger.ts index a1fff8332..922b1dd0b 100644 --- a/lib/Logger.ts +++ b/lib/Logger.ts @@ -1,6 +1,9 @@ +type Parameters = string | Record | Array> | Error; + type LogData = { message: string; level: 'alert' | 'info' | 'hmmm'; + parameters?: Parameters; }; type LoggerCallback = (data: LogData) => void; @@ -17,22 +20,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, parameters?: Parameters) { + logger({message: `[Onyx] ${message}`, level: 'alert', parameters}); } /** * Send an info message to the logger */ -function logInfo(message: string) { - logger({message: `[Onyx] ${message}`, level: 'info'}); +function logInfo(message: string, parameters?: Parameters) { + logger({message: `[Onyx] ${message}`, level: 'info', parameters}); } /** * Send an hmmm message to the logger */ -function logHmmm(message: string) { - logger({message: `[Onyx] ${message}`, level: 'hmmm'}); +function logHmmm(message: string, parameters?: Parameters) { + logger({message: `[Onyx] ${message}`, level: 'hmmm', parameters}); } export {registerLogger, logInfo, logAlert, logHmmm}; 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}>; diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index e95b5faad..7b8e4f99a 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,14 @@ type BaseUseOnyxOptions = { * If set to `true`, the key can be changed dynamically during the component lifecycle. */ allowDynamicKey?: boolean; + + /** + * 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; }; type UseOnyxInitialValueOption = { @@ -98,17 +107,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>, @@ -219,6 +217,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; @@ -228,11 +228,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. @@ -255,7 +258,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'; } @@ -269,7 +272,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 @@ -280,11 +283,18 @@ 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 newStatus = newFetchStatus ?? 'loaded'; + resultRef.current = [previousValueRef.current ?? undefined, {status: newStatus}]; + + // 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 with canBeMissing set to false.`, {key, showAlert: true}); + } } 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..284ae24cc 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,135 @@ 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(); + 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(alertMessage); + + 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(alertMessage); + }); + + 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(alertMessage); + }); + + 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'); + 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, alertMessage, {key: ONYXKEYS.TEST_KEY, showAlert: true}); + + 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).toHaveBeenCalledTimes(2); + 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 () => { + 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'); + 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, alertMessage, {key: ONYXKEYS.TEST_KEY, showAlert: true}); + + 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).toHaveBeenCalledTimes(2); + 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 () => { + 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).toHaveBeenCalledTimes(1); + expect(logAlertFn).toHaveBeenNthCalledWith(1, alertMessage, {key: ONYXKEYS.TEST_KEY, showAlert: true}); + + 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).toHaveBeenCalledTimes(2); + expect(logAlertFn).toHaveBeenNthCalledWith(2, alertMessage, {key: ONYXKEYS.TEST_KEY, showAlert: true}); + }); + }); + // 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: []}).`;