Skip to content

Commit

Permalink
Merge pull request #561 from margelo/@chrispader/more-onyx-bump-fixes
Browse files Browse the repository at this point in the history 
More minor fixes for the Onyx bump in E/App
  • Loading branch information
@mountiny
mountiny authored Jun 11, 2024
2 parents a4f75c1 + cdbd255 commit 6762004
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 24 deletions.
6 changes: 3 additions & 3 deletions lib/Onyx.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -575,17 +575,17 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise<void> {
// since collection key subscribers need to be updated differently
if (!isKeyToPreserve) {
const oldValue = cache.get(key);
const newValue = defaultKeyStates[key] ?? undefined;
const newValue = defaultKeyStates[key] ?? null;
if (newValue !== oldValue) {
cache.set(key, newValue);
const collectionKey = key.substring(0, key.indexOf('_') + 1);
if (collectionKey) {
if (!keyValuesToResetAsCollection[collectionKey]) {
keyValuesToResetAsCollection[collectionKey] = {};
}
keyValuesToResetAsCollection[collectionKey]![key] = newValue;
keyValuesToResetAsCollection[collectionKey]![key] = newValue ?? undefined;
} else {
keyValuesToResetIndividually[key] = newValue;
keyValuesToResetIndividually[key] = newValue ?? undefined;
}
}
}
Expand Down
24 changes: 13 additions & 11 deletions lib/useOnyx.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {CollectionKeyBase, OnyxCollection, OnyxKey, OnyxValue, Selector} fr
import useLiveRef from './useLiveRef';
import usePrevious from './usePrevious';
import Onyx from './Onyx';
import cache from './OnyxCache';

type BaseUseOnyxOptions = {
/**
Expand Down Expand Up @@ -54,7 +55,7 @@ type ResultMetadata = {
type UseOnyxResult<TKey extends OnyxKey, TValue> = [CachedValue<TKey, TValue>, ResultMetadata];

function getCachedValue<TKey extends OnyxKey, TValue>(key: TKey, selector?: Selector<TKey, unknown, unknown>): CachedValue<TKey, TValue> | undefined {
return OnyxUtils.tryGetCachedValue(key, {selector}) as CachedValue<TKey, TValue> | undefined;
return (OnyxUtils.tryGetCachedValue(key, {selector}) ?? undefined) as CachedValue<TKey, TValue> | undefined;
}

function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
Expand All @@ -73,8 +74,8 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(key: TKey
const selectorRef = useLiveRef(options?.selector);

// Stores the previous cached value as it's necessary to compare with the new value in `getSnapshot()`.
// We initialize it to `undefined` to simulate that we don't have any value from cache yet.
const cachedValueRef = useRef<CachedValue<TKey, TReturnValue> | undefined>(undefined);
// We initialize it to `null` to simulate that we don't have any value from cache yet.
const cachedValueRef = useRef<CachedValue<TKey, TReturnValue> | 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.
Expand Down Expand Up @@ -117,10 +118,11 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(key: TKey
const getSnapshot = useCallback(() => {
// We get the value from the cache, supplying a selector too in case it's defined.
// If `newValue` is `undefined` it means that the cache doesn't have a value for that key yet.
// If `newValue` is `null` or any other value if means that the cache does have a value for that key.
// If `newValue` 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.
let newValue = getCachedValue<TKey, TReturnValue>(key, selectorRef.current);
const hasCacheForKey = cache.hasCacheForKey(key);

// Since the fetch status can be different given the use cases below, we define the variable right away.
let newFetchStatus: FetchStatus | undefined;
Expand All @@ -133,20 +135,20 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(key: TKey
newFetchStatus = 'loading';
}

// If data is not present in cache (if it's `undefined`) and `initialValue` is set during the first connection,
// 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 && newValue === undefined && options?.initialValue !== undefined) {
newValue = options?.initialValue as CachedValue<TKey, TReturnValue>;
if (isFirstConnectionRef.current && !hasCacheForKey && options?.initialValue !== undefined) {
newValue = (options?.initialValue ?? undefined) as CachedValue<TKey, TReturnValue>;
newFetchStatus = 'loaded';
}

// If the previously cached value is different from the new value, we update both cached value
// and the result to be returned by the hook.
if (!deepEqual(cachedValueRef.current, newValue)) {
// If the cache was set for the first time, we also update the cached value and the result.
const isCacheSetFirstTime = cachedValueRef.current === null && hasCacheForKey;
if (isCacheSetFirstTime || !deepEqual(cachedValueRef.current ?? undefined, newValue)) {
cachedValueRef.current = newValue;

// 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'}];
resultRef.current = [cachedValueRef.current as CachedValue<TKey, TReturnValue>, {status: newFetchStatus ?? 'loaded'}];
}

return resultRef.current;
Expand Down
20 changes: 10 additions & 10 deletions tests/unit/useOnyxTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ describe('useOnyx', () => {
expect(result.current[1].status).toEqual('loaded');
});

it('should initially return null while loading non-cached key, and then return value and loaded state', async () => {
it('should initially return `undefined` while loading non-cached key, and then return value and loaded state', async () => {
await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test');

const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY));
Expand Down Expand Up @@ -244,8 +244,6 @@ describe('useOnyx', () => {
});

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
Expand All @@ -257,6 +255,8 @@ describe('useOnyx', () => {
expect(result.current[0]).toEqual('initial value');
expect(result.current[1].status).toEqual('loaded');

Onyx.set(ONYXKEYS.TEST_KEY, 'test_id_1');

await act(async () => waitForPromisesToResolve());

expect(result.current[0]).toBeUndefined();
Expand Down Expand Up @@ -320,12 +320,6 @@ describe('useOnyx', () => {
});

it('should return initial value and loaded state while we have pending merges for the key, and then return updated value and loaded state', async () => {
Onyx.set(ONYXKEYS.TEST_KEY, 'test1');

Onyx.merge(ONYXKEYS.TEST_KEY, 'test2');
Onyx.merge(ONYXKEYS.TEST_KEY, 'test3');
Onyx.merge(ONYXKEYS.TEST_KEY, 'test4');

const {result} = renderHook(() =>
useOnyx(ONYXKEYS.TEST_KEY, {
initialValue: 'initial value',
Expand All @@ -335,6 +329,12 @@ describe('useOnyx', () => {
expect(result.current[0]).toEqual('initial value');
expect(result.current[1].status).toEqual('loaded');

Onyx.set(ONYXKEYS.TEST_KEY, 'test1');

Onyx.merge(ONYXKEYS.TEST_KEY, 'test2');
Onyx.merge(ONYXKEYS.TEST_KEY, 'test3');
Onyx.merge(ONYXKEYS.TEST_KEY, 'test4');

await act(async () => waitForPromisesToResolve());

expect(result.current[0]).toEqual('test4');
Expand All @@ -361,7 +361,7 @@ describe('useOnyx', () => {
});

describe('initWithStoredValues', () => {
it('should return null and loaded state, and after merge return updated value and loaded state', async () => {
it('should return `undefined` and loaded state, and after merge return updated value and loaded state', async () => {
await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test1');

const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY, {initWithStoredValues: false}));
Expand Down

0 comments on commit 6762004

Please sign in to comment.