Merge pull request Expensify#746 from callstack-internal/feature/remove-allowStaleData-flag

feat: remove allowStaleData flag

Author: francoisl

lib/OnyxSnapshotCache.ts

@@ -59,18 +59,16 @@ class OnyxSnapshotCache {
      *
      * - `selector`: Different selectors produce different results, so each selector needs its own cache entry
      * - `initWithStoredValues`: This flag changes the initial loading behavior and affects the returned fetch status
-     * - `allowStaleData`: Controls whether stale data can be returned during pending merges, affecting result timing
      *
      * Other options like `canEvict`, `reuseConnection`, and `allowDynamicKey` don't affect the data transformation
      * or timing behavior of getSnapshot, so they're excluded from the cache key for better cache hit rates.
      */
-    registerConsumer<TKey extends OnyxKey, TReturnValue>(options: Pick<UseOnyxOptions<TKey, TReturnValue>, 'selector' | 'initWithStoredValues' | 'allowStaleData'>): string {
+    registerConsumer<TKey extends OnyxKey, TReturnValue>(options: Pick<UseOnyxOptions<TKey, TReturnValue>, 'selector' | 'initWithStoredValues'>): string {
         const selectorID = options?.selector ? this.getSelectorID(options.selector) : 'no_selector';
 
         // Create options hash without expensive JSON.stringify
         const initWithStoredValues = options?.initWithStoredValues ?? true;
-        const allowStaleData = options?.allowStaleData ?? false;
-        const cacheKey = `${selectorID}_${initWithStoredValues}_${allowStaleData}`;
+        const cacheKey = `${selectorID}_${initWithStoredValues}`;
 
         // Increment reference count for this cache key
         const currentCount = this.cacheKeyRefCounts.get(cacheKey) || 0;

lib/useOnyx.ts

@@ -26,11 +26,6 @@ type UseOnyxOptions<TKey extends OnyxKey, TReturnValue> = {
      */
     initWithStoredValues?: boolean;
 
-    /**
-     * 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;
-
     /**
      * If set to `false`, the connection won't be reused between other subscribers that are listening to the same Onyx key
      * with the same connect configurations.
@@ -146,9 +141,8 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
             onyxSnapshotCache.registerConsumer({
                 selector: options?.selector,
                 initWithStoredValues: options?.initWithStoredValues,
-                allowStaleData: options?.allowStaleData,
             }),
-        [options?.selector, options?.initWithStoredValues, options?.allowStaleData],
+        [options?.selector, options?.initWithStoredValues],
     );
 
     useEffect(() => () => onyxSnapshotCache.deregisterConsumer(key, cacheKey), [key, cacheKey]);
@@ -269,8 +263,7 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
 
         // If we have pending merge operations for the key during the first connection, we set the new value to `undefined`
         // and fetch status to `loading` to simulate that it is still being loaded until we have the most updated data.
-        // If `allowStaleData` is `true` this logic will be ignored and cached value will be used, even if it's stale data.
-        if (isFirstConnectionRef.current && OnyxUtils.hasPendingMergeForKey(key) && !options?.allowStaleData) {
+        if (isFirstConnectionRef.current && OnyxUtils.hasPendingMergeForKey(key)) {
             newValueRef.current = undefined;
             newFetchStatus = 'loading';
         }
@@ -315,7 +308,7 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
         }
 
         return resultRef.current;
-    }, [options?.initWithStoredValues, options?.allowStaleData, key, memoizedSelector, cacheKey, previousKey]);
+    }, [options?.initWithStoredValues, key, memoizedSelector, cacheKey, previousKey]);
 
     const subscribe = useCallback(
         (onStoreChange: () => void) => {

tests/perf-test/OnyxSnapshotCache.perf-test.ts

@@ -38,13 +38,11 @@ const complexSelector: UseOnyxSelector<OnyxKey, ComplexSelectorResult> = (data)
 const selectorOptions: UseOnyxOptions<string, number | undefined> = {
     selector: simpleSelector,
     initWithStoredValues: true,
-    allowStaleData: false,
 };
 
 const complexSelectorOptions: UseOnyxOptions<string, ComplexSelectorResult> = {
     selector: complexSelector,
     initWithStoredValues: true,
-    allowStaleData: false,
 };
 
 // Mock results

tests/unit/OnyxSnapshotCacheTest.ts

@@ -39,11 +39,9 @@ describe('OnyxSnapshotCache', () => {
             const optionsWithSelector: UseOnyxOptions<OnyxKey, string> = {
                 selector,
                 initWithStoredValues: true,
-                allowStaleData: false,
             };
             const optionsWithoutSelector: UseOnyxOptions<OnyxKey, string> = {
                 initWithStoredValues: false,
-                allowStaleData: true,
             };
             const keyWithSelector = cache.registerConsumer(optionsWithSelector);
             const keyWithoutSelector = cache.registerConsumer(optionsWithoutSelector);

tests/unit/useOnyxTest.ts

@@ -737,7 +737,7 @@ describe('useOnyx', () => {
         });
     });
 
-    describe('allowStaleData', () => {
+    describe('pending merges', () => {
         it('should return undefined and loading state while we have pending merges for the key, and then return updated value and loaded state', async () => {
             Onyx.set(ONYXKEYS.TEST_KEY, 'test1');
 
@@ -777,24 +777,6 @@ describe('useOnyx', () => {
             expect(result.current[0]).toEqual('test4_changed');
             expect(result.current[1].status).toEqual('loaded');
         });
-
-        it('should return stale value and loaded state if allowStaleData is true, 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, {allowStaleData: true}));
-
-            expect(result.current[0]).toEqual('test1');
-            expect(result.current[1].status).toEqual('loaded');
-
-            await act(async () => waitForPromisesToResolve());
-
-            expect(result.current[0]).toEqual('test4');
-            expect(result.current[1].status).toEqual('loaded');
-        });
     });
 
     describe('initWithStoredValues', () => {
@@ -877,39 +859,6 @@ describe('useOnyx', () => {
             expect(result2.current[1].status).toEqual('loaded');
         });
 
-        it('"allowStaleData" should work correctly for the same key if more than one hook is using it', 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: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY, {allowStaleData: true}));
-
-            expect(result1.current[0]).toEqual('test1');
-            expect(result1.current[1].status).toEqual('loaded');
-
-            await act(async () => waitForPromisesToResolve());
-
-            expect(result1.current[0]).toEqual('test4');
-            expect(result1.current[1].status).toEqual('loaded');
-
-            // Second hook
-            Onyx.merge(ONYXKEYS.TEST_KEY, 'test5');
-            Onyx.merge(ONYXKEYS.TEST_KEY, 'test6');
-            Onyx.merge(ONYXKEYS.TEST_KEY, 'test7');
-
-            const {result: result2} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY, {allowStaleData: true}));
-
-            expect(result2.current[0]).toEqual('test4');
-            expect(result2.current[1].status).toEqual('loaded');
-
-            await act(async () => waitForPromisesToResolve());
-
-            expect(result2.current[0]).toEqual('test7');
-            expect(result2.current[1].status).toEqual('loaded');
-        });
-
         it('"initWithStoredValues" should work correctly for the same key if more than one hook is using it', async () => {
             await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test1');