From fcda31294335b9bbe51988e7757a930cd75013a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 14 Oct 2024 22:29:15 +0100 Subject: [PATCH] Refactor Onyx cache tasks --- lib/Onyx.ts | 4 ++-- lib/OnyxCache.ts | 18 +++++++++++++++--- lib/OnyxUtils.ts | 14 ++++++-------- lib/useOnyx.ts | 4 ++-- tests/unit/onyxCacheTest.tsx | 17 ++++++++++------- 5 files changed, 35 insertions(+), 22 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 6275e78b..3f5d75ff 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -2,7 +2,7 @@ import _ from 'underscore'; import lodashPick from 'lodash/pick'; import * as Logger from './Logger'; -import cache from './OnyxCache'; +import cache, {TASK} from './OnyxCache'; import * as PerformanceUtils from './PerformanceUtils'; import Storage from './storage'; import utils from './utils'; @@ -538,7 +538,7 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { }) .then(() => undefined); - return cache.captureTask('clear', promise) as Promise; + return cache.captureTask(TASK.CLEAR, promise) as Promise; } function updateSnapshots(data: OnyxUpdate[]) { diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index 530f8e66..a6df22f7 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -1,8 +1,18 @@ import {deepEqual} from 'fast-equals'; import bindAll from 'lodash/bindAll'; +import type {ValueOf} from 'type-fest'; import utils from './utils'; import type {OnyxKey, OnyxValue} from './types'; +// Task constants +const TASK = { + GET: 'get', + GET_ALL_KEYS: 'getAllKeys', + CLEAR: 'clear', +} as const; + +type CacheTask = ValueOf | `${ValueOf}:${string}`; + /** * In memory cache providing data by reference * Encapsulates Onyx cache related functionality @@ -172,7 +182,7 @@ class OnyxCache { * Check whether the given task is already running * @param taskName - unique name given for the task */ - hasPendingTask(taskName: string): boolean { + hasPendingTask(taskName: CacheTask): boolean { return this.pendingPromises.get(taskName) !== undefined; } @@ -182,7 +192,7 @@ class OnyxCache { * provided from this function * @param taskName - unique name given for the task */ - getTaskPromise(taskName: string): Promise | OnyxKey[]> | undefined { + getTaskPromise(taskName: CacheTask): Promise | OnyxKey[]> | undefined { return this.pendingPromises.get(taskName); } @@ -191,7 +201,7 @@ class OnyxCache { * hook up to the promise if it's still pending * @param taskName - unique name for the task */ - captureTask(taskName: string, promise: Promise>): Promise> { + captureTask(taskName: CacheTask, promise: Promise>): Promise> { const returnPromise = promise.finally(() => { this.pendingPromises.delete(taskName); }); @@ -242,3 +252,5 @@ class OnyxCache { const instance = new OnyxCache(); export default instance; +export {TASK}; +export type {CacheTask}; diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index ec6eadf6..de89a230 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -6,7 +6,7 @@ import type {ValueOf} from 'type-fest'; import DevTools from './DevTools'; import * as Logger from './Logger'; import type Onyx from './Onyx'; -import cache from './OnyxCache'; +import cache, {TASK} from './OnyxCache'; import * as PerformanceUtils from './PerformanceUtils'; import * as Str from './Str'; import unstable_batchedUpdates from './batch'; @@ -244,7 +244,7 @@ function get>(key: TKey): P return Promise.resolve(cache.get(key) as TValue); } - const taskName = `get:${key}`; + const taskName = `${TASK.GET}:${key}` as const; // When a value retrieving task for this key is still running hook to it if (cache.hasPendingTask(taskName)) { @@ -296,7 +296,7 @@ function multiGet(keys: CollectionKeyBase[]): Promise>); pendingKeys.push(key); @@ -378,11 +378,9 @@ function getAllKeys(): Promise> { return Promise.resolve(cachedKeys); } - const taskName = 'getAllKeys'; - // When a value retrieving task for all keys is still running hook to it - if (cache.hasPendingTask(taskName)) { - return cache.getTaskPromise(taskName) as Promise>; + if (cache.hasPendingTask(TASK.GET_ALL_KEYS)) { + return cache.getTaskPromise(TASK.GET_ALL_KEYS) as Promise>; } // Otherwise retrieve the keys from storage and capture a promise to aid concurrent usages @@ -393,7 +391,7 @@ function getAllKeys(): Promise> { return cache.getAllKeys(); }); - return cache.captureTask(taskName, promise) as Promise>; + return cache.captureTask(TASK.GET_ALL_KEYS, promise) as Promise>; } /** diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index f2857eac..f560802e 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -1,6 +1,6 @@ import {deepEqual, shallowEqual} from 'fast-equals'; import {useCallback, useEffect, useRef, useSyncExternalStore} from 'react'; -import OnyxCache from './OnyxCache'; +import OnyxCache, {TASK} from './OnyxCache'; import type {Connection} from './OnyxConnectionManager'; import connectionManager from './OnyxConnectionManager'; import OnyxUtils from './OnyxUtils'; @@ -239,7 +239,7 @@ function useOnyx>(key: TKey // 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 the cache was set for the first time or we have a pending Onyx.clear() task, we also update the cached value and the result. - const isCacheSetFirstTime = previousValueRef.current === null && (hasCacheForKey || OnyxCache.hasPendingTask('clear')); + const isCacheSetFirstTime = previousValueRef.current === null && (hasCacheForKey || OnyxCache.hasPendingTask(TASK.CLEAR)); if (isCacheSetFirstTime || !areValuesEqual) { previousValueRef.current = newValueRef.current; diff --git a/tests/unit/onyxCacheTest.tsx b/tests/unit/onyxCacheTest.tsx index 0759044c..383ced8e 100644 --- a/tests/unit/onyxCacheTest.tsx +++ b/tests/unit/onyxCacheTest.tsx @@ -14,6 +14,9 @@ import type withOnyxType from '../../lib/withOnyx'; import type {InitOptions} from '../../lib/types'; import generateRange from '../utils/generateRange'; import type {Connection} from '../../lib/OnyxConnectionManager'; +import type {CacheTask} from '../../lib/OnyxCache'; + +const MOCK_TASK = 'mockTask' as CacheTask; describe('Onyx', () => { describe('Cache Service', () => { @@ -374,22 +377,22 @@ describe('Onyx', () => { // Given empty cache with no started tasks // When a task has not been started // Then it should return false - expect(cache.hasPendingTask('mockTask')).toBe(false); + expect(cache.hasPendingTask(MOCK_TASK)).toBe(false); }); it('Should return true when a task is running', () => { // Given empty cache with no started tasks // When a unique task is started const promise = Promise.resolve(); - cache.captureTask('mockTask', promise); + cache.captureTask(MOCK_TASK, promise); // Then `hasPendingTask` should return true - expect(cache.hasPendingTask('mockTask')).toBe(true); + expect(cache.hasPendingTask(MOCK_TASK)).toBe(true); // When the promise is completed return waitForPromisesToResolve().then(() => { // Then `hasPendingTask` should return false - expect(cache.hasPendingTask('mockTask')).toBe(false); + expect(cache.hasPendingTask(MOCK_TASK)).toBe(false); }); }); }); @@ -399,7 +402,7 @@ describe('Onyx', () => { // Given empty cache with no started tasks // When a task is retrieved - const task = cache.getTaskPromise('mockTask'); + const task = cache.getTaskPromise(MOCK_TASK); // Then it should be undefined expect(task).not.toBeDefined(); @@ -409,10 +412,10 @@ describe('Onyx', () => { // Given empty cache with no started tasks // When a unique task is started const promise = Promise.resolve({mockResult: true}); - cache.captureTask('mockTask', promise); + cache.captureTask(MOCK_TASK, promise); // When a task is retrieved - const taskPromise = cache.getTaskPromise('mockTask'); + const taskPromise = cache.getTaskPromise(MOCK_TASK); // Then it should resolve with the same result as the captured task return taskPromise!.then((result) => {