Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

useOnyx type improvements #534

Merged
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions lib/Onyx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type {
InitOptions,
KeyValueMapping,
Mapping,
NonUndefined,
NullableKeyValueMapping,
NullishDeep,
OnyxCollection,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();

Expand Down
56 changes: 34 additions & 22 deletions lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

fabioh8010 marked this conversation as resolved.
Show resolved Hide resolved
/**
* Represents a deeply nested record. It maps keys to values,
* and those values can either be of type `TValue` or further nested `DeepRecord` instances.
Expand Down Expand Up @@ -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.
*
Expand All @@ -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;
fabioh8010 marked this conversation as resolved.
Show resolved Hide resolved

/**
* 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.
*
Expand All @@ -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;
Expand Down Expand Up @@ -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> = {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
};
46 changes: 37 additions & 9 deletions lib/useOnyx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -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
Expand All @@ -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;
Expand All @@ -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);

Expand All @@ -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',
},
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions lib/withOnyx.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ type OnyxPropCollectionMapping<TComponentProps, TOnyxProps, TOnyxProp extends ke
}[CollectionKeyBase];

/**
* @deprecated Use `useOnyx` instead of `withOnyx` whenever possible.
fabioh8010 marked this conversation as resolved.
Show resolved Hide resolved
*
* 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.
Expand Down
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
30 changes: 25 additions & 5 deletions tests/unit/useOnyxTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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());
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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');
});

Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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'));
Expand Down
Loading