Merge pull request Expensify#534 from fabioh8010/feature/useOnyx-type…

…-improvements useOnyx type improvements
callstack-internal · May 7, 2024 · 6a3eb7d · 6a3eb7d
2 parents 2704488 + e532b79
commit 6a3eb7d
Show file tree

Hide file tree

Showing 7 changed files with 109 additions and 46 deletions.
diff --git a/lib/Onyx.ts b/lib/Onyx.ts
@@ -14,6 +14,7 @@ import type {
     InitOptions,
     KeyValueMapping,
     Mapping,
+    NonUndefined,
     NullableKeyValueMapping,
     NullishDeep,
     OnyxCollection,
@@ -208,7 +209,7 @@ function disconnect(connectionID: number, keyToRemoveFromEvictionBlocklist?: Ony
  * @param key ONYXKEY to set
  * @param value value to store
  */
-function set<TKey extends OnyxKey>(key: TKey, value: OnyxEntry<KeyValueMapping[TKey]>): Promise<void> {
+function set<TKey extends OnyxKey>(key: TKey, value: NonUndefined<OnyxEntry<KeyValueMapping[TKey]>>): Promise<void> {
     // check if the value is compatible with the existing value in the storage
     const existingValue = cache.getValue(key, false);
     const {isCompatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(value, existingValue);
@@ -289,7 +290,7 @@ function multiSet(data: Partial<NullableKeyValueMapping>): Promise<void> {
  * Onyx.merge(ONYXKEYS.POLICY, {id: 1}); // -> {id: 1}
  * Onyx.merge(ONYXKEYS.POLICY, {name: 'My Workspace'}); // -> {id: 1, name: 'My Workspace'}
  */
-function merge<TKey extends OnyxKey>(key: TKey, changes: OnyxEntry<NullishDeep<KeyValueMapping[TKey]>>): Promise<void> {
+function merge<TKey extends OnyxKey>(key: TKey, changes: NonUndefined<OnyxEntry<NullishDeep<KeyValueMapping[TKey]>>>): Promise<void> {
     const mergeQueue = OnyxUtils.getMergeQueue();
     const mergeQueuePromise = OnyxUtils.getMergeQueuePromise();
 

diff --git a/lib/types.ts b/lib/types.ts
@@ -3,6 +3,16 @@ import type {Merge} from 'type-fest';
 import type {BuiltIns} from 'type-fest/source/internal';
 import type OnyxUtils from './OnyxUtils';
 
+/**
+ * Utility type that excludes `null` from the type `TValue`.
+ */
+type NonNull<TValue> = TValue extends null ? never : TValue;
+
+/**
+ * Utility type that excludes `undefined` from the type `TValue`.
+ */
+type NonUndefined<TValue> = TValue extends undefined ? never : TValue;
+
 /**
  * Represents a deeply nested record. It maps keys to values,
  * and those values can either be of type `TValue` or further nested `DeepRecord` instances.
@@ -141,7 +151,7 @@ type NullableKeyValueMapping = {
 type Selector<TKey extends OnyxKey, TOnyxProps, TReturnType> = (value: OnyxEntry<KeyValueMapping[TKey]>, state: WithOnyxInstanceState<TOnyxProps>) => TReturnType;
 
 /**
- * Represents a single Onyx entry, that can be either `TOnyxValue` or `null` if it doesn't exist.
+ * Represents a single Onyx entry, that can be either `TOnyxValue` or `null` / `undefined` if it doesn't exist.
  *
  * It can be used to specify data retrieved from Onyx e.g. `withOnyx` HOC mappings.
  *
@@ -168,10 +178,10 @@ type Selector<TKey extends OnyxKey, TOnyxProps, TReturnType> = (value: OnyxEntry
  * })(Component);
  * ```
  */
-type OnyxEntry<TOnyxValue> = TOnyxValue | null;
+type OnyxEntry<TOnyxValue> = TOnyxValue | null | undefined;
 
 /**
- * Represents an Onyx collection of entries, that can be either a record of `TOnyxValue`s or `null` if it is empty or doesn't exist.
+ * Represents an Onyx collection of entries, that can be either a record of `TOnyxValue`s or `null` / `undefined` if it is empty or doesn't exist.
  *
  * It can be used to specify collection data retrieved from Onyx e.g. `withOnyx` HOC mappings.
  *
@@ -198,7 +208,7 @@ type OnyxEntry<TOnyxValue> = TOnyxValue | null;
  * })(Component);
  * ```
  */
-type OnyxCollection<TOnyxValue> = OnyxEntry<Record<string, TOnyxValue | null>>;
+type OnyxCollection<TOnyxValue> = OnyxEntry<Record<string, TOnyxValue | null | undefined>>;
 
 /** Utility type to extract `TOnyxValue` from `OnyxCollection<TOnyxValue>` */
 type ExtractOnyxCollectionValue<TOnyxCollection> = TOnyxCollection extends NonNullable<OnyxCollection<infer U>> ? U : never;
@@ -284,9 +294,9 @@ type WithOnyxConnectOptions<TKey extends OnyxKey> = {
     canEvict?: boolean;
 };
 
-type DefaultConnectCallback<TKey extends OnyxKey> = (value: OnyxEntry<KeyValueMapping[TKey]>, key: TKey) => void;
+type DefaultConnectCallback<TKey extends OnyxKey> = (value: NonUndefined<OnyxEntry<KeyValueMapping[TKey]>>, key: TKey) => void;
 
-type CollectionConnectCallback<TKey extends OnyxKey> = (value: OnyxCollection<KeyValueMapping[TKey]>) => void;
+type CollectionConnectCallback<TKey extends OnyxKey> = (value: NonUndefined<OnyxCollection<KeyValueMapping[TKey]>>) => void;
 
 /** Represents the callback function used in `Onyx.connect()` method with a regular key. */
 type DefaultConnectOptions<TKey extends OnyxKey> = {
@@ -331,12 +341,12 @@ type OnyxUpdate =
               | {
                     onyxMethod: typeof OnyxUtils.METHOD.SET;
                     key: TKey;
-                    value: OnyxEntry<KeyValueMapping[TKey]>;
+                    value: NonUndefined<OnyxEntry<KeyValueMapping[TKey]>>;
                 }
               | {
                     onyxMethod: typeof OnyxUtils.METHOD.MERGE;
                     key: TKey;
-                    value: OnyxEntry<NullishDeep<KeyValueMapping[TKey]>>;
+                    value: NonUndefined<OnyxEntry<NullishDeep<KeyValueMapping[TKey]>>>;
                 }
               | {
                     onyxMethod: typeof OnyxUtils.METHOD.MULTI_SET;
@@ -391,31 +401,33 @@ type InitOptions = {
 };
 
 export type {
+    BaseConnectOptions,
+    Collection,
+    CollectionConnectCallback,
+    CollectionConnectOptions,
     CollectionKey,
     CollectionKeyBase,
+    ConnectOptions,
     CustomTypeOptions,
     DeepRecord,
+    DefaultConnectCallback,
+    DefaultConnectOptions,
+    ExtractOnyxCollectionValue,
+    InitOptions,
     Key,
     KeyValueMapping,
+    Mapping,
+    NonNull,
+    NonUndefined,
     NullableKeyValueMapping,
+    NullishDeep,
     OnyxCollection,
     OnyxEntry,
     OnyxKey,
+    OnyxUpdate,
     OnyxValue,
     Selector,
-    NullishDeep,
-    WithOnyxInstanceState,
-    ExtractOnyxCollectionValue,
-    Collection,
-    WithOnyxInstance,
-    BaseConnectOptions,
     WithOnyxConnectOptions,
-    DefaultConnectCallback,
-    CollectionConnectCallback,
-    DefaultConnectOptions,
-    CollectionConnectOptions,
-    ConnectOptions,
-    Mapping,
-    OnyxUpdate,
-    InitOptions,
+    WithOnyxInstance,
+    WithOnyxInstanceState,
 };
diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts
@@ -2,12 +2,22 @@ import {deepEqual} from 'fast-equals';
 import {useCallback, useEffect, useRef, useSyncExternalStore} from 'react';
 import type {IsEqual} from 'type-fest';
 import OnyxUtils from './OnyxUtils';
-import type {CollectionKeyBase, OnyxCollection, OnyxKey, OnyxValue, Selector} from './types';
+import type {CollectionKeyBase, KeyValueMapping, NonNull, OnyxCollection, OnyxEntry, OnyxKey, Selector} from './types';
 import useLiveRef from './useLiveRef';
 import usePrevious from './usePrevious';
 import Onyx from './Onyx';
 
-type UseOnyxOptions<TKey extends OnyxKey, TReturnValue> = {
+/**
+ * Represents a Onyx value that can be either a single entry or a collection of entries, depending on the `TKey` provided.
+ * It's a variation of `OnyxValue` type that is read-only and excludes the `null` type.
+ */
+type UseOnyxValue<TKey extends OnyxKey> = string extends TKey
+    ? unknown
+    : TKey extends CollectionKeyBase
+    ? Readonly<NonNull<OnyxCollection<KeyValueMapping[TKey]>>>
+    : Readonly<NonNull<OnyxEntry<KeyValueMapping[TKey]>>>;
+
+type BaseUseOnyxOptions = {
     /**
      * Determines if this key in this subscription is safe to be evicted.
      */
@@ -22,12 +32,16 @@ type UseOnyxOptions<TKey extends OnyxKey, TReturnValue> = {
      * If set to true, data will be retrieved from cache during the first render even if there is a pending merge for the key.
      */
     allowStaleData?: boolean;
+};
 
+type UseOnyxInitialValueOption<TInitialValue> = {
     /**
      * This value will be returned by the hook on the first render while the data is being read from Onyx.
      */
-    initialValue?: TReturnValue;
+    initialValue?: TInitialValue;
+};
 
+type UseOnyxSelectorOption<TKey extends OnyxKey, TReturnValue> = {
     /**
      * This will be used to subscribe to a subset of an Onyx key's data.
      * Using this setting on `useOnyx` can have very positive performance benefits because the component will only re-render
@@ -37,9 +51,15 @@ type UseOnyxOptions<TKey extends OnyxKey, TReturnValue> = {
     selector?: Selector<TKey, unknown, TReturnValue>;
 };
 
+type UseOnyxOptions<TKey extends OnyxKey, TReturnValue> = BaseUseOnyxOptions & UseOnyxInitialValueOption<TReturnValue> & UseOnyxSelectorOption<TKey, TReturnValue>;
+
 type FetchStatus = 'loading' | 'loaded';
 
-type CachedValue<TKey extends OnyxKey, TValue> = IsEqual<TValue, OnyxValue<TKey>> extends true ? TValue : TKey extends CollectionKeyBase ? NonNullable<OnyxCollection<TValue>> : TValue;
+type CachedValue<TKey extends OnyxKey, TValue> = IsEqual<TValue, UseOnyxValue<TKey>> extends true
+    ? TValue
+    : TKey extends CollectionKeyBase
+    ? Readonly<NonNullable<OnyxCollection<TValue>>>
+    : Readonly<TValue>;
 
 type ResultMetadata = {
     status: FetchStatus;
@@ -51,7 +71,15 @@ function getCachedValue<TKey extends OnyxKey, TValue>(key: TKey, selector?: Sele
     return OnyxUtils.tryGetCachedValue(key, {selector}) as CachedValue<TKey, TValue> | undefined;
 }
 
-function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(key: TKey, options?: UseOnyxOptions<TKey, TReturnValue>): UseOnyxResult<TKey, TReturnValue> {
+function useOnyx<TKey extends OnyxKey, TReturnValue = UseOnyxValue<TKey>>(
+    key: TKey,
+    options?: BaseUseOnyxOptions & UseOnyxInitialValueOption<TReturnValue> & Required<UseOnyxSelectorOption<TKey, TReturnValue>>,
+): UseOnyxResult<TKey, TReturnValue>;
+function useOnyx<TKey extends OnyxKey, TReturnValue = UseOnyxValue<TKey>>(
+    key: TKey,
+    options?: BaseUseOnyxOptions & UseOnyxInitialValueOption<NoInfer<TReturnValue>>,
+): UseOnyxResult<TKey, TReturnValue>;
+function useOnyx<TKey extends OnyxKey, TReturnValue = UseOnyxValue<TKey>>(key: TKey, options?: UseOnyxOptions<TKey, TReturnValue>): UseOnyxResult<TKey, TReturnValue> {
     const connectionIDRef = useRef<number | null>(null);
     const previousKey = usePrevious(key);
 
@@ -63,10 +91,10 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(key: TKey
     const cachedValueRef = useRef<CachedValue<TKey, TReturnValue> | undefined>(undefined);
 
     // Stores the previously result returned by the hook, containing the data from cache and the fetch status.
-    // We initialize it to `null` and `loading` fetch status to simulate the initial result when the hook is loading from the cache.
+    // 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<UseOnyxResult<TKey, TReturnValue>>([
-        null as CachedValue<TKey, TReturnValue>,
+        undefined as CachedValue<TKey, TReturnValue>,
         {
             status: options?.initWithStoredValues === false ? 'loaded' : 'loading',
         },
@@ -131,8 +159,8 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(key: TKey
         if (!deepEqual(cachedValueRef.current, newValue)) {
             cachedValueRef.current = newValue;
 
-            // If the new value is `undefined` we default it to `null` to ensure the consumer get a consistent result from the hook.
-            resultRef.current = [(cachedValueRef.current ?? null) as CachedValue<TKey, TReturnValue>, {status: newFetchStatus ?? 'loaded'}];
+            // If the new value is `null` we default it to `undefined` to ensure the consumer get a consistent result from the hook.
+            resultRef.current = [(cachedValueRef.current ?? undefined) as CachedValue<TKey, TReturnValue>, {status: newFetchStatus ?? 'loaded'}];
         }
 
         return resultRef.current;

diff --git a/lib/withOnyx.d.ts b/lib/withOnyx.d.ts
@@ -125,6 +125,8 @@ type OnyxPropCollectionMapping<TComponentProps, TOnyxProps, TOnyxProp extends ke
 }[CollectionKeyBase];
 
 /**
+ * @deprecated Use `useOnyx` instead of `withOnyx` whenever possible.
+ *
  * This is a higher order component that provides the ability to map a state property directly to
  * something in Onyx (a key/value store). That way, as soon as data in Onyx changes, the state will be set and the view
  * will automatically change to reflect the new data.

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -85,7 +85,7 @@
     "reassure": "^0.11.0",
     "ts-node": "^10.9.2",
     "type-fest": "^3.12.0",
-    "typescript": "^5.3.3"
+    "typescript": "^5.4.5"
   },
   "peerDependencies": {
     "idb-keyval": "^6.2.1",

diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts
@@ -102,7 +102,7 @@ describe('useOnyx', () => {
 
             const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY));
 
-            expect(result.current[0]).toEqual(null);
+            expect(result.current[0]).toBeUndefined();
             expect(result.current[1].status).toEqual('loading');
 
             await act(async () => waitForPromisesToResolve());
@@ -118,7 +118,7 @@ describe('useOnyx', () => {
 
             const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY));
 
-            expect(result.current[0]).toEqual(null);
+            expect(result.current[0]).toBeUndefined();
             expect(result.current[1].status).toEqual('loading');
 
             await act(async () => waitForPromisesToResolve());
@@ -242,6 +242,26 @@ describe('useOnyx', () => {
             expect(result.current[0]).toEqual('id - changed_id, name - changed_name - selector changed');
             expect(result.current[1].status).toEqual('loaded');
         });
+
+        it('should return initial value if selected data is undefined', async () => {
+            Onyx.set(ONYXKEYS.TEST_KEY, 'test_id_1');
+
+            const {result} = renderHook(() =>
+                useOnyx(ONYXKEYS.TEST_KEY, {
+                    // @ts-expect-error bypass
+                    selector: (_entry: OnyxEntry<string>) => undefined,
+                    initialValue: 'initial value',
+                }),
+            );
+
+            expect(result.current[0]).toEqual('initial value');
+            expect(result.current[1].status).toEqual('loaded');
+
+            await act(async () => waitForPromisesToResolve());
+
+            expect(result.current[0]).toBeUndefined();
+            expect(result.current[1].status).toEqual('loaded');
+        });
     });
 
     describe('initialValue', () => {
@@ -257,7 +277,7 @@ describe('useOnyx', () => {
 
             await act(async () => waitForPromisesToResolve());
 
-            expect(result.current[0]).toEqual(null);
+            expect(result.current[0]).toBeUndefined();
             expect(result.current[1].status).toEqual('loaded');
         });
 
@@ -290,7 +310,7 @@ describe('useOnyx', () => {
 
             const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY));
 
-            expect(result.current[0]).toEqual(null);
+            expect(result.current[0]).toBeUndefined();
             expect(result.current[1].status).toEqual('loading');
 
             await act(async () => waitForPromisesToResolve());
@@ -348,7 +368,7 @@ describe('useOnyx', () => {
 
             await act(async () => waitForPromisesToResolve());
 
-            expect(result.current[0]).toEqual(null);
+            expect(result.current[0]).toBeUndefined();
             expect(result.current[1].status).toEqual('loaded');
 
             await act(async () => Onyx.merge(ONYXKEYS.TEST_KEY, 'test2'));