From 174684ae4d26e4f9f7036b2b22adb66f6b6eb7c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 27 Sep 2024 11:56:13 +0200 Subject: [PATCH 01/15] update selector collection test to match new functionality --- tests/unit/useOnyxTest.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 25b56631..e846cb45 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -1,5 +1,5 @@ import {act, renderHook} from '@testing-library/react-native'; -import type {OnyxEntry} from '../../lib'; +import type {OnyxCollection, OnyxEntry} from '../../lib'; import Onyx, {useOnyx} from '../../lib'; import OnyxUtils from '../../lib/OnyxUtils'; import StorageMock from '../../lib/storage'; @@ -176,7 +176,11 @@ describe('useOnyx', () => { const {result} = renderHook(() => useOnyx(ONYXKEYS.COLLECTION.TEST_KEY, { // @ts-expect-error bypass - selector: (entry: OnyxEntry<{id: string; name: string}>) => entry?.id, + selector: (entries: OnyxCollection<{id: string; name: string}>) => + Object.entries(entries ?? {}).reduce>>((acc, [key, value]) => { + acc[key] = value?.id; + return acc; + }, {}), }), ); From 5a53c321fcd173f0448bae82bc429abbe163e7ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 27 Sep 2024 22:49:58 +0200 Subject: [PATCH 02/15] useOnyx collection selector changes --- lib/useOnyx.ts | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 3abea163..6c75cf35 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -5,7 +5,7 @@ import OnyxCache from './OnyxCache'; import type {Connection} from './OnyxConnectionManager'; import connectionManager from './OnyxConnectionManager'; import OnyxUtils from './OnyxUtils'; -import type {CollectionKeyBase, OnyxCollection, OnyxEntry, OnyxKey, OnyxValue, Selector} from './types'; +import type {CollectionKeyBase, KeyValueMapping, OnyxCollection, OnyxEntry, OnyxKey, OnyxValue} from './types'; import useLiveRef from './useLiveRef'; import usePrevious from './usePrevious'; @@ -40,7 +40,7 @@ type UseOnyxSelectorOption = { * when the subset of data changes. Otherwise, any change of data on any property would normally * cause the component to re-render (and that can be expensive from a performance standpoint). */ - selector?: Selector; + selector?: UseOnyxSelector; }; type UseOnyxOptions = BaseUseOnyxOptions & UseOnyxInitialValueOption & UseOnyxSelectorOption; @@ -57,8 +57,35 @@ type ResultMetadata = { type UseOnyxResult = [CachedValue, ResultMetadata]; -function getCachedValue(key: TKey, selector?: Selector): CachedValue | undefined { - return (OnyxUtils.tryGetCachedValue(key, {selector}) ?? undefined) as CachedValue | undefined; +type UseOnyxSelector = (data: OnyxData) => SelectorValue; + +function getCollectionCachedValue(key: TKey) { + const allCacheKeys = OnyxCache.getAllKeys(); + + // It is possible we haven't loaded all keys yet so we do not know if the + // collection actually exists. + if (allCacheKeys.size === 0) { + return; + } + + const values: OnyxCollection = {}; + allCacheKeys.forEach((cacheKey) => { + if (!cacheKey.startsWith(key)) { + return; + } + + values[cacheKey] = OnyxCache.get(cacheKey); + }); + + return values; +} + +function getCachedValue(key: TKey, selector?: UseOnyxSelector) { + const value = OnyxUtils.isCollectionKey(key) ? getCollectionCachedValue(key) : OnyxCache.get(key); + + const selectedValue = selector ? selector(value) : value; + + return selectedValue as CachedValue | undefined; } function useOnyx>( From fbb4a8c19afa3ebc14034c187125c08747e38c2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Fri, 27 Sep 2024 23:59:22 +0200 Subject: [PATCH 03/15] useOnyx selector changes part2 --- lib/useOnyx.ts | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 6c75cf35..92ede14c 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -40,7 +40,7 @@ type UseOnyxSelectorOption = { * when the subset of data changes. Otherwise, any change of data on any property would normally * cause the component to re-render (and that can be expensive from a performance standpoint). */ - selector?: UseOnyxSelector; + selector?: UseOnyxSelector; }; type UseOnyxOptions = BaseUseOnyxOptions & UseOnyxInitialValueOption & UseOnyxSelectorOption; @@ -55,11 +55,15 @@ type ResultMetadata = { status: FetchStatus; }; -type UseOnyxResult = [CachedValue, ResultMetadata]; +type UseOnyxResult = [TValue, ResultMetadata]; -type UseOnyxSelector = (data: OnyxData) => SelectorValue; +type UseOnyxSelector = (data?: OnyxValue) => SelectorValue; + +function getCachedValue(key: TKey): OnyxValue | undefined { + if (!OnyxUtils.isCollectionKey(key)) { + return OnyxCache.get(key) as OnyxValue; + } -function getCollectionCachedValue(key: TKey) { const allCacheKeys = OnyxCache.getAllKeys(); // It is possible we haven't loaded all keys yet so we do not know if the @@ -77,26 +81,26 @@ function getCollectionCachedValue(key: TKey) { values[cacheKey] = OnyxCache.get(cacheKey); }); - return values; + return values as OnyxValue; } -function getCachedValue(key: TKey, selector?: UseOnyxSelector) { - const value = OnyxUtils.isCollectionKey(key) ? getCollectionCachedValue(key) : OnyxCache.get(key); +function getValue | undefined>(key: TKey, selector?: UseOnyxSelector) { + const value = getCachedValue(key); const selectedValue = selector ? selector(value) : value; - return selectedValue as CachedValue | undefined; + return selectedValue; } function useOnyx>( key: TKey, options?: BaseUseOnyxOptions & UseOnyxInitialValueOption & Required>, -): UseOnyxResult; +): UseOnyxResult; function useOnyx>( key: TKey, options?: BaseUseOnyxOptions & UseOnyxInitialValueOption>, -): UseOnyxResult; -function useOnyx>(key: TKey, options?: UseOnyxOptions): UseOnyxResult { +): UseOnyxResult; +function useOnyx>(key: TKey, options?: UseOnyxOptions): UseOnyxResult { const connectionRef = useRef(null); const previousKey = usePrevious(key); @@ -105,16 +109,16 @@ function useOnyx>(key: TKey // Stores the previous cached value as it's necessary to compare with the new value in `getSnapshot()`. // We initialize it to `null` to simulate that we don't have any value from cache yet. - const previousValueRef = useRef | undefined | null>(null); + const previousValueRef = useRef | undefined | null>(null); // Stores the newest cached value in order to compare with the previous one and optimize `getSnapshot()` execution. - const newValueRef = useRef | undefined | null>(null); + const newValueRef = useRef | undefined | null>(); // Stores the previously result returned by the hook, containing the data from cache and the fetch status. // We initialize it to `undefined` and `loading` fetch status to simulate the initial result when the hook is loading from the cache. // However, if `initWithStoredValues` is `true` we set the fetch status to `loaded` since we want to signal that data is ready. - const resultRef = useRef>([ - undefined as CachedValue, + const resultRef = useRef>([ + undefined as TReturnValue, { status: options?.initWithStoredValues === false ? 'loaded' : 'loading', }, @@ -181,7 +185,7 @@ function useOnyx>(key: TKey // 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); + newValueRef.current = getValue(key, selectorRef.current); // 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. @@ -226,7 +230,7 @@ function useOnyx>(key: TKey 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 as CachedValue, {status: newFetchStatus ?? 'loaded'}]; + resultRef.current = [previousValueRef.current as TReturnValue, {status: newFetchStatus ?? 'loaded'}]; } return resultRef.current; @@ -269,7 +273,7 @@ function useOnyx>(key: TKey checkEvictableKey(); }, [checkEvictableKey]); - const result = useSyncExternalStore>(subscribe, getSnapshot); + const result = useSyncExternalStore(subscribe, getSnapshot); return result; } From 397b4dd9fea446eeed951017945678afd5eb707c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Mon, 30 Sep 2024 13:26:47 +0200 Subject: [PATCH 04/15] cleanup --- lib/useOnyx.ts | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 00454624..e70b40cf 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -55,7 +55,7 @@ type ResultMetadata = { status: FetchStatus; }; -type UseOnyxResult = [TValue, ResultMetadata]; +type UseOnyxResult = [CachedValue, ResultMetadata]; type UseOnyxSelector = (data?: OnyxValue) => SelectorValue; @@ -84,7 +84,7 @@ function getCachedValue(key: TKey): OnyxValue | unde return values as OnyxValue; } -function getValue | undefined>(key: TKey, selector?: UseOnyxSelector) { +function getSelectedValue | undefined>(key: TKey, selector?: UseOnyxSelector) { const value = getCachedValue(key); const selectedValue = selector ? selector(value) : value; @@ -95,12 +95,12 @@ function getValue | undefined>(ke function useOnyx>( key: TKey, options?: BaseUseOnyxOptions & UseOnyxInitialValueOption & Required>, -): UseOnyxResult; +): UseOnyxResult; function useOnyx>( key: TKey, options?: BaseUseOnyxOptions & UseOnyxInitialValueOption>, -): UseOnyxResult; -function useOnyx>(key: TKey, options?: UseOnyxOptions): UseOnyxResult { +): UseOnyxResult; +function useOnyx>(key: TKey, options?: UseOnyxOptions): UseOnyxResult { const connectionRef = useRef(null); const previousKey = usePrevious(key); @@ -112,13 +112,13 @@ function useOnyx>(key: TKey const previousValueRef = useRef | undefined | null>(null); // Stores the newest cached value in order to compare with the previous one and optimize `getSnapshot()` execution. - const newValueRef = useRef | undefined | null>(); + const newValueRef = useRef | undefined | null>(null); // Stores the previously result returned by the hook, containing the data from cache and the fetch status. // We initialize it to `undefined` and `loading` fetch status to simulate the initial result when the hook is loading from the cache. - // However, if `initWithStoredValues` is `true` we set the fetch status to `loaded` since we want to signal that data is ready. - const resultRef = useRef>([ - undefined as TReturnValue, + // However, if `initWithStoredValues` is `false` we set the fetch status to `loaded` since we want to signal that data is ready. + const resultRef = useRef>([ + undefined as CachedValue, { status: options?.initWithStoredValues === false ? 'loaded' : 'loading', }, @@ -190,7 +190,7 @@ function useOnyx>(key: TKey // 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 = getValue(key, selectorRef.current); + newValueRef.current = getSelectedValue(key, selectorRef.current); // 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. @@ -235,7 +235,7 @@ function useOnyx>(key: TKey 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 as TReturnValue, {status: newFetchStatus ?? 'loaded'}]; + resultRef.current = [previousValueRef.current as CachedValue, {status: newFetchStatus ?? 'loaded'}]; } return resultRef.current; @@ -278,7 +278,7 @@ function useOnyx>(key: TKey checkEvictableKey(); }, [checkEvictableKey]); - const result = useSyncExternalStore(subscribe, getSnapshot); + const result = useSyncExternalStore>(subscribe, getSnapshot); return result; } From a41bd1bdbeef84d06bb49a1b62f2d1ec8c49b4c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Mon, 30 Sep 2024 13:28:11 +0200 Subject: [PATCH 05/15] cleanup part 2 --- lib/useOnyx.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index e70b40cf..cff4958b 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -109,7 +109,7 @@ function useOnyx>(key: TKey // Stores the previous cached value as it's necessary to compare with the new value in `getSnapshot()`. // We initialize it to `null` to simulate that we don't have any value from cache yet. - const previousValueRef = useRef | undefined | null>(null); + const previousValueRef = useRef | undefined | null>(null); // Stores the newest cached value in order to compare with the previous one and optimize `getSnapshot()` execution. const newValueRef = useRef | undefined | null>(null); From 68c27023e19541a3d4678eef94046525222cf04e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Mon, 30 Sep 2024 15:43:43 +0200 Subject: [PATCH 06/15] cleanup part 3 --- lib/useOnyx.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index cff4958b..4549f202 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -5,7 +5,7 @@ import OnyxCache from './OnyxCache'; import type {Connection} from './OnyxConnectionManager'; import connectionManager from './OnyxConnectionManager'; import OnyxUtils from './OnyxUtils'; -import type {CollectionKeyBase, KeyValueMapping, OnyxCollection, OnyxEntry, OnyxKey, OnyxValue} from './types'; +import type {CollectionKeyBase, OnyxCollection, OnyxEntry, OnyxKey, OnyxValue} from './types'; import useLiveRef from './useLiveRef'; import usePrevious from './usePrevious'; @@ -61,7 +61,7 @@ type UseOnyxSelector = (data?: OnyxValue(key: TKey): OnyxValue | undefined { if (!OnyxUtils.isCollectionKey(key)) { - return OnyxCache.get(key) as OnyxValue; + return OnyxCache.get(key); } const allCacheKeys = OnyxCache.getAllKeys(); @@ -72,9 +72,9 @@ function getCachedValue(key: TKey): OnyxValue | unde return; } - const values: OnyxCollection = {}; + const values: Record = {}; allCacheKeys.forEach((cacheKey) => { - if (!cacheKey.startsWith(key)) { + if (cacheKey.startsWith(key)) { return; } From b74380bc37ac635b74a22cdddd5095d93e242c2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Mon, 30 Sep 2024 15:59:00 +0200 Subject: [PATCH 07/15] cleanup part 4 --- lib/useOnyx.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 4549f202..9c584a65 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -61,7 +61,7 @@ type UseOnyxSelector = (data?: OnyxValue(key: TKey): OnyxValue | undefined { if (!OnyxUtils.isCollectionKey(key)) { - return OnyxCache.get(key); + return OnyxCache.get(key) as OnyxValue | undefined; } const allCacheKeys = OnyxCache.getAllKeys(); @@ -190,7 +190,7 @@ function useOnyx>(key: TKey // 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 = getSelectedValue(key, selectorRef.current); + newValueRef.current = getSelectedValue(key, selectorRef.current) as CachedValue; // 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. From ee1d28d85a61822e5e350a778e2cdd83cb8a2424 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Mon, 30 Sep 2024 16:51:48 +0200 Subject: [PATCH 08/15] scary exclamation mark --- lib/useOnyx.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 9c584a65..73f16215 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -74,7 +74,7 @@ function getCachedValue(key: TKey): OnyxValue | unde const values: Record = {}; allCacheKeys.forEach((cacheKey) => { - if (cacheKey.startsWith(key)) { + if (!cacheKey.startsWith(key)) { return; } From 24a9e8b490e43a8adc3422e7f7d9e3ded77ec46a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Tue, 1 Oct 2024 10:25:40 +0200 Subject: [PATCH 09/15] add comments for helpers --- lib/useOnyx.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 73f16215..78fb050e 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -59,6 +59,10 @@ type UseOnyxResult = [CachedValue, R type UseOnyxSelector = (data?: OnyxValue) => SelectorValue; +/** + * Gets the cached value from the Onyx cache. If the key is a collection key, it will return all the values in the collection. + * It is a fork of `tryGetCachedValue` from `OnyxUtils` caused by different selector logic in `useOnyx`. It should be unified in the future, when `withOnyx` is removed. + */ function getCachedValue(key: TKey): OnyxValue | undefined { if (!OnyxUtils.isCollectionKey(key)) { return OnyxCache.get(key) as OnyxValue | undefined; @@ -84,6 +88,9 @@ function getCachedValue(key: TKey): OnyxValue | unde return values as OnyxValue; } +/** + * Gets the value from cache and maps it with selector. + */ function getSelectedValue | undefined>(key: TKey, selector?: UseOnyxSelector) { const value = getCachedValue(key); From a16d45f2ec1157563f0ac53e48ee2aa7f56d03c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Tue, 1 Oct 2024 11:43:02 +0200 Subject: [PATCH 10/15] fix typings --- lib/useOnyx.ts | 43 +++++++++++++++---------------------------- 1 file changed, 15 insertions(+), 28 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 78fb050e..13561a7c 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -1,11 +1,10 @@ import {deepEqual, shallowEqual} from 'fast-equals'; import {useCallback, useEffect, useRef, useSyncExternalStore} from 'react'; -import type {IsEqual} from 'type-fest'; import OnyxCache from './OnyxCache'; import type {Connection} from './OnyxConnectionManager'; import connectionManager from './OnyxConnectionManager'; import OnyxUtils from './OnyxUtils'; -import type {CollectionKeyBase, OnyxCollection, OnyxEntry, OnyxKey, OnyxValue} from './types'; +import type {CollectionKeyBase, OnyxKey, OnyxValue} from './types'; import useLiveRef from './useLiveRef'; import usePrevious from './usePrevious'; @@ -33,6 +32,8 @@ type UseOnyxInitialValueOption = { initialValue?: TInitialValue; }; +type UseOnyxSelector> = (data: OnyxValue | undefined) => TReturnValue; + type UseOnyxSelectorOption = { /** * This will be used to subscribe to a subset of an Onyx key's data. @@ -47,17 +48,11 @@ type UseOnyxOptions = BaseUseOnyxOptions & U type FetchStatus = 'loading' | 'loaded'; -type SelectedValue = TKey extends CollectionKeyBase ? OnyxCollection : OnyxEntry; - -type CachedValue = IsEqual> extends true ? TValue : SelectedValue; - type ResultMetadata = { status: FetchStatus; }; -type UseOnyxResult = [CachedValue, ResultMetadata]; - -type UseOnyxSelector = (data?: OnyxValue) => SelectorValue; +type UseOnyxResult = [TValue, ResultMetadata]; /** * Gets the cached value from the Onyx cache. If the key is a collection key, it will return all the values in the collection. @@ -91,23 +86,15 @@ function getCachedValue(key: TKey): OnyxValue | unde /** * Gets the value from cache and maps it with selector. */ -function getSelectedValue | undefined>(key: TKey, selector?: UseOnyxSelector) { +function getSelectedValue(key: TKey, selector?: UseOnyxSelector) { const value = getCachedValue(key); - const selectedValue = selector ? selector(value) : value; + const selectedValue = (selector ? selector(value) : value) as TValue; return selectedValue; } -function useOnyx>( - key: TKey, - options?: BaseUseOnyxOptions & UseOnyxInitialValueOption & Required>, -): UseOnyxResult; -function useOnyx>( - key: TKey, - options?: BaseUseOnyxOptions & UseOnyxInitialValueOption>, -): UseOnyxResult; -function useOnyx>(key: TKey, options?: UseOnyxOptions): UseOnyxResult { +function useOnyx>(key: TKey, options?: UseOnyxOptions) { const connectionRef = useRef(null); const previousKey = usePrevious(key); @@ -116,16 +103,16 @@ function useOnyx>(key: TKey // Stores the previous cached value as it's necessary to compare with the new value in `getSnapshot()`. // We initialize it to `null` to simulate that we don't have any value from cache yet. - const previousValueRef = useRef | undefined | null>(null); + const previousValueRef = useRef(null); // Stores the newest cached value in order to compare with the previous one and optimize `getSnapshot()` execution. - const newValueRef = useRef | undefined | null>(null); + const newValueRef = useRef(null); // Stores the previously result returned by the hook, containing the data from cache and the fetch status. // We initialize it to `undefined` and `loading` fetch status to simulate the initial result when the hook is loading from the cache. // However, if `initWithStoredValues` is `false` we set the fetch status to `loaded` since we want to signal that data is ready. - const resultRef = useRef>([ - undefined as CachedValue, + const resultRef = useRef>([ + undefined, { status: options?.initWithStoredValues === false ? 'loaded' : 'loading', }, @@ -197,7 +184,7 @@ function useOnyx>(key: TKey // 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 = getSelectedValue(key, selectorRef.current) as CachedValue; + newValueRef.current = getSelectedValue(key, selectorRef.current); // 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. @@ -220,7 +207,7 @@ function useOnyx>(key: TKey // 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 CachedValue; + newValueRef.current = options?.initialValue ?? undefined; newFetchStatus = 'loaded'; } @@ -242,7 +229,7 @@ function useOnyx>(key: TKey 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 as CachedValue, {status: newFetchStatus ?? 'loaded'}]; + resultRef.current = [previousValueRef.current, {status: newFetchStatus ?? 'loaded'}]; } return resultRef.current; @@ -285,7 +272,7 @@ function useOnyx>(key: TKey checkEvictableKey(); }, [checkEvictableKey]); - const result = useSyncExternalStore>(subscribe, getSnapshot); + const result = useSyncExternalStore(subscribe, getSnapshot); return result; } From 7c65dd0ec361258392c3a038de7171b1ff7c0465 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Tue, 1 Oct 2024 13:09:54 +0200 Subject: [PATCH 11/15] fix null typo --- lib/useOnyx.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 13561a7c..5de00bfc 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -111,7 +111,7 @@ function useOnyx>(key: TKey // Stores the previously result returned by the hook, containing the data from cache and the fetch status. // We initialize it to `undefined` and `loading` fetch status to simulate the initial result when the hook is loading from the cache. // However, if `initWithStoredValues` is `false` we set the fetch status to `loaded` since we want to signal that data is ready. - const resultRef = useRef>([ + const resultRef = useRef>([ undefined, { status: options?.initWithStoredValues === false ? 'loaded' : 'loading', From 18a84bac73891063dc85eae6a2c4b130504fbfd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Tue, 1 Oct 2024 13:25:47 +0200 Subject: [PATCH 12/15] revive overloads --- lib/useOnyx.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 5de00bfc..cc299b14 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -94,6 +94,14 @@ function getSelectedValue(key: TKey, selector?: Us return selectedValue; } +function useOnyx>( + key: TKey, + options?: BaseUseOnyxOptions & UseOnyxInitialValueOption & Required>, +): UseOnyxResult; +function useOnyx>( + key: TKey, + options?: BaseUseOnyxOptions & UseOnyxInitialValueOption>, +): UseOnyxResult; function useOnyx>(key: TKey, options?: UseOnyxOptions) { const connectionRef = useRef(null); const previousKey = usePrevious(key); From 4629b7826630d098cfcdc7167f93191ee36ddb3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Wed, 2 Oct 2024 12:58:50 +0200 Subject: [PATCH 13/15] batched types fixes --- lib/useOnyx.ts | 44 ++++++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index cc299b14..993130df 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -4,7 +4,7 @@ import OnyxCache from './OnyxCache'; import type {Connection} from './OnyxConnectionManager'; import connectionManager from './OnyxConnectionManager'; import OnyxUtils from './OnyxUtils'; -import type {CollectionKeyBase, OnyxKey, OnyxValue} from './types'; +import type {CollectionKeyBase, KeyValueMapping, OnyxCollection, OnyxKey, OnyxValue} from './types'; import useLiveRef from './useLiveRef'; import usePrevious from './usePrevious'; @@ -40,6 +40,7 @@ type UseOnyxSelectorOption = { * Using this setting on `useOnyx` can have very positive performance benefits because the component will only re-render * when the subset of data changes. Otherwise, any change of data on any property would normally * cause the component to re-render (and that can be expensive from a performance standpoint). + * @see `useOnyx` cannot return `null` and so selector will replace `null` with `undefined` to maintain compatibility. */ selector?: UseOnyxSelector; }; @@ -52,15 +53,15 @@ type ResultMetadata = { status: FetchStatus; }; -type UseOnyxResult = [TValue, ResultMetadata]; +type UseOnyxResult = [NonNullable | undefined, ResultMetadata]; /** * Gets the cached value from the Onyx cache. If the key is a collection key, it will return all the values in the collection. * It is a fork of `tryGetCachedValue` from `OnyxUtils` caused by different selector logic in `useOnyx`. It should be unified in the future, when `withOnyx` is removed. */ -function getCachedValue(key: TKey): OnyxValue | undefined { +function tryGetCachedValue(key: TKey): OnyxValue { if (!OnyxUtils.isCollectionKey(key)) { - return OnyxCache.get(key) as OnyxValue | undefined; + return OnyxCache.get(key); } const allCacheKeys = OnyxCache.getAllKeys(); @@ -71,7 +72,7 @@ function getCachedValue(key: TKey): OnyxValue | unde return; } - const values: Record = {}; + const values: OnyxCollection = {}; allCacheKeys.forEach((cacheKey) => { if (!cacheKey.startsWith(key)) { return; @@ -80,18 +81,18 @@ function getCachedValue(key: TKey): OnyxValue | unde values[cacheKey] = OnyxCache.get(cacheKey); }); - return values as OnyxValue; + return values; } /** - * Gets the value from cache and maps it with selector. + * Gets the value from cache and maps it with selector. It changes `null` to `undefined` for `useOnyx` compatibility. */ -function getSelectedValue(key: TKey, selector?: UseOnyxSelector) { - const value = getCachedValue(key); +function getCachedValue(key: TKey, selector?: UseOnyxSelector) { + const value = tryGetCachedValue(key) as OnyxValue; - const selectedValue = (selector ? selector(value) : value) as TValue; + const selectedValue = selector ? selector(value) : (value as TValue); - return selectedValue; + return selectedValue ?? undefined; } function useOnyx>( @@ -102,7 +103,7 @@ function useOnyx>( key: TKey, options?: BaseUseOnyxOptions & UseOnyxInitialValueOption>, ): UseOnyxResult; -function useOnyx>(key: TKey, options?: UseOnyxOptions) { +function useOnyx>(key: TKey, options?: UseOnyxOptions): UseOnyxResult { const connectionRef = useRef(null); const previousKey = usePrevious(key); @@ -118,8 +119,8 @@ function useOnyx>(key: TKey // Stores the previously result returned by the hook, containing the data from cache and the fetch status. // We initialize it to `undefined` and `loading` fetch status to simulate the initial result when the hook is loading from the cache. - // However, if `initWithStoredValues` is `false` we set the fetch status to `loaded` since we want to signal that data is ready. - const resultRef = useRef>([ + // However, if `initWithStoredValues` is `true` we set the fetch status to `loaded` since we want to signal that data is ready. + const resultRef = useRef>([ undefined, { status: options?.initWithStoredValues === false ? 'loaded' : 'loading', @@ -179,11 +180,6 @@ function useOnyx>(key: TKey }, [key, options?.canEvict]); const getSnapshot = useCallback(() => { - // 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; - } - // We get the value from cache while the first connection to Onyx is being made, // so we can return any cached value right away. After the connection is made, we only // update `newValueRef` when `Onyx.connect()` callback is fired. @@ -192,7 +188,7 @@ function useOnyx>(key: TKey // 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 = getSelectedValue(key, selectorRef.current); + newValueRef.current = getCachedValue(key, selectorRef.current); // 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. @@ -215,7 +211,7 @@ function useOnyx>(key: TKey // 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; + newValueRef.current = (options?.initialValue ?? undefined) as TReturnValue; newFetchStatus = 'loaded'; } @@ -237,11 +233,11 @@ function useOnyx>(key: TKey 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, {status: newFetchStatus ?? 'loaded'}]; + resultRef.current = [previousValueRef.current ?? undefined, {status: newFetchStatus ?? 'loaded'}]; } return resultRef.current; - }, [key, selectorRef, options?.initWithStoredValues, options?.allowStaleData, options?.initialValue]); + }, [key, selectorRef, options?.allowStaleData, options?.initialValue]); const subscribe = useCallback( (onStoreChange: () => void) => { @@ -280,7 +276,7 @@ function useOnyx>(key: TKey checkEvictableKey(); }, [checkEvictableKey]); - const result = useSyncExternalStore(subscribe, getSnapshot); + const result = useSyncExternalStore>(subscribe, getSnapshot); return result; } From 81127bf03dd53f40b6bdddd781d1957ad823dbd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Wed, 2 Oct 2024 13:09:51 +0200 Subject: [PATCH 14/15] fix copy&paste accidents --- lib/useOnyx.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 993130df..e294fe93 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -119,7 +119,7 @@ function useOnyx>(key: TKey // Stores the previously result returned by the hook, containing the data from cache and the fetch status. // We initialize it to `undefined` and `loading` fetch status to simulate the initial result when the hook is loading from the cache. - // However, if `initWithStoredValues` is `true` we set the fetch status to `loaded` since we want to signal that data is ready. + // However, if `initWithStoredValues` is `false` we set the fetch status to `loaded` since we want to signal that data is ready. const resultRef = useRef>([ undefined, { @@ -180,6 +180,11 @@ function useOnyx>(key: TKey }, [key, options?.canEvict]); const getSnapshot = useCallback(() => { + // 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; + } + // We get the value from cache while the first connection to Onyx is being made, // so we can return any cached value right away. After the connection is made, we only // update `newValueRef` when `Onyx.connect()` callback is fired. @@ -237,7 +242,7 @@ function useOnyx>(key: TKey } return resultRef.current; - }, [key, selectorRef, options?.allowStaleData, options?.initialValue]); + }, [options?.initWithStoredValues, options?.allowStaleData, options?.initialValue, key, selectorRef]); const subscribe = useCallback( (onStoreChange: () => void) => { From 5a82d802dddf1b58d980e077139789b66966aaeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Miko=C5=82ajczak?= Date: Thu, 3 Oct 2024 11:13:02 +0200 Subject: [PATCH 15/15] update comment --- lib/useOnyx.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index e294fe93..2d89f508 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -220,7 +220,7 @@ function useOnyx>(key: TKey newFetchStatus = 'loaded'; } - // We do a deep equality check if `selector` is defined, since each `OnyxUtils.tryGetCachedValue()` call will + // We do a deep equality check if `selector` is defined, since each `tryGetCachedValue()` call will // generate a plain new primitive/object/array that was created using the `selector` function. // For the other cases we will only deal with object reference checks, so just a shallow equality check is enough. let areValuesEqual: boolean;