fix: re-run selector if dependencies change

lib/useOnyx.ts

@@ -10,6 +10,7 @@ import type {CollectionKeyBase, OnyxKey, OnyxValue} from './types';
 import usePrevious from './usePrevious';
 import decorateWithMetrics from './metrics';
 import * as Logger from './Logger';
+import useLiveRef from './useLiveRef';
 
 type UseOnyxSelector<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>> = (data: OnyxValue<TKey> | undefined) => TReturnValue;
 
@@ -75,30 +76,42 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
     const connectionRef = useRef<Connection | null>(null);
     const previousKey = usePrevious(key);
 
+    const currentDependenciesRef = useLiveRef(dependencies);
+    const currentSelectorRef = useLiveRef(options?.selector);
+
     // Create memoized version of selector for performance
     const memoizedSelector = useMemo(() => {
         if (!options?.selector) return null;
 
         let lastInput: OnyxValue<TKey> | undefined;
         let lastOutput: TReturnValue;
+        let lastDependencies: DependencyList = [];
         let hasComputed = false;
 
         return (input: OnyxValue<TKey> | undefined): TReturnValue => {
-            // Always recompute when input changes
-            if (!hasComputed || lastInput !== input) {
-                const newOutput = options.selector!(input);
-
-                // Deep equality mode: only update if output actually changed
-                if (!hasComputed || !deepEqual(lastOutput, newOutput)) {
-                    lastInput = input;
-                    lastOutput = newOutput;
-                    hasComputed = true;
+            const currentDependencies = currentDependenciesRef.current;
+            const currentSelector = currentSelectorRef.current;
+
+            // Recompute if input changed, dependencies changed, or first time
+            const dependenciesChanged = !shallowEqual(lastDependencies, currentDependencies);
+            if (!hasComputed || lastInput !== input || dependenciesChanged) {
+                // Only proceed if we have a valid selector
+                if (currentSelector) {
+                    const newOutput = currentSelector(input);
+
+                    // Deep equality mode: only update if output actually changed
+                    if (!hasComputed || !deepEqual(lastOutput, newOutput) || dependenciesChanged) {
+                        lastInput = input;
+                        lastOutput = newOutput;
+                        lastDependencies = [...currentDependencies];
+                        hasComputed = true;
+                    }
                 }
             }
 
             return lastOutput;
         };
-    }, [options?.selector]);
+    }, [currentDependenciesRef, currentSelectorRef, options?.selector]);
 
     // Stores the previous cached value as it's necessary to compare with the new value in `getSnapshot()`.
     // We initialize it to `null` to simulate that we don't have any value from cache yet.
@@ -157,10 +170,22 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
         );
     }, [previousKey, key, options?.allowDynamicKey]);
 
+    // Track previous dependencies to prevent infinite loops
+    const previousDependenciesRef = useRef<DependencyList>([]);
+
     useEffect(() => {
         // This effect will only run if the `dependencies` array changes. If it changes it will force the hook
         // to trigger a `getSnapshot()` update by calling the stored `onStoreChange()` function reference, thus
         // re-running the hook and returning the latest value to the consumer.
+
+        // Deep equality check to prevent infinite loops when dependencies array reference changes
+        // but content remains the same
+        if (shallowEqual(previousDependenciesRef.current, dependencies)) {
+            return;
+        }
+
+        previousDependenciesRef.current = dependencies;
+
         if (connectionRef.current === null || isConnectingRef.current || !onStoreChangeFnRef.current) {
             return;
         }

tests/unit/useOnyxTest.ts

@@ -465,48 +465,6 @@ describe('useOnyx', () => {
             expect(result.current[0]).toBe(firstResult);
         });
 
-        it('should use reference equality for memoized selectors instead of deep equality', async () => {
-            // This test verifies the optimization where memoized selectors use reference equality
-            const complexObject = {
-                nested: {
-                    array: [1, 2, 3],
-                    object: {prop: 'value'},
-                },
-            };
-
-            Onyx.set(ONYXKEYS.TEST_KEY, complexObject);
-
-            const {result} = renderHook(() =>
-                useOnyx(ONYXKEYS.TEST_KEY, {
-                    // @ts-expect-error bypass
-                    selector: (entry: OnyxEntry<typeof complexObject>) => entry?.nested,
-                }),
-            );
-
-            await act(async () => waitForPromisesToResolve());
-
-            const firstResult = result.current[0];
-
-            // Set the exact same object reference
-            await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, complexObject));
-
-            // Should return same reference due to memoization
-            expect(result.current[0]).toBe(firstResult);
-
-            // Set different object with same content
-            const differentObjectSameContent = {
-                nested: {
-                    array: [1, 2, 3],
-                    object: {prop: 'value'},
-                },
-            };
-
-            await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, differentObjectSameContent));
-
-            // Should return same reference due to deep equality in memoized selector
-            expect(result.current[0]).toBe(firstResult);
-        });
-
         it('should memoize primitive selector results correctly', async () => {
             Onyx.set(ONYXKEYS.TEST_KEY, {count: 5, name: 'test'});
 
@@ -536,6 +494,175 @@ describe('useOnyx', () => {
             expect(result.current[0]).not.toBe(firstResult);
             expect(result.current[0]).toBe(10);
         });
+
+        it('should recompute selector when dependencies change even if input data stays the same', async () => {
+            const testCollection = {
+                [`${ONYXKEYS.COLLECTION.TEST_KEY}1`]: {id: '1', value: 'item1'},
+                [`${ONYXKEYS.COLLECTION.TEST_KEY}2`]: {id: '2', value: 'item2'},
+                [`${ONYXKEYS.COLLECTION.TEST_KEY}3`]: {id: '3', value: 'item3'},
+            };
+
+            await act(async () => Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, testCollection as GenericCollection));
+
+            let filterIds = ['1'];
+            let selectorCallCount = 0;
+
+            const {result, rerender} = renderHook(() =>
+                useOnyx(
+                    ONYXKEYS.COLLECTION.TEST_KEY,
+                    {
+                        selector: (collection) => {
+                            selectorCallCount++;
+                            return filterIds.map((id) => (collection as OnyxCollection<GenericCollection>)?.[`${ONYXKEYS.COLLECTION.TEST_KEY}${id}`]).filter(Boolean);
+                        },
+                    },
+                    [filterIds],
+                ),
+            );
+
+            await act(async () => waitForPromisesToResolve());
+
+            // Record count after initial stabilization
+            const initialCallCount = selectorCallCount;
+            const initialResult = result.current[0];
+
+            // Should return item with id '1'
+            expect(initialResult).toEqual([{id: '1', value: 'item1'}]);
+
+            // Change dependencies without changing underlying data
+            await act(async () => {
+                filterIds = ['1', '2'];
+                rerender(ONYXKEYS.COLLECTION.TEST_KEY);
+            });
+
+            // Selector should recompute and return items with id '1' and '2'
+            expect(result.current[0]).toEqual([
+                {id: '1', value: 'item1'},
+                {id: '2', value: 'item2'},
+            ]);
+            expect(selectorCallCount).toBeGreaterThan(initialCallCount);
+
+            // Record count after first dependency change
+            const firstChangeCallCount = selectorCallCount;
+
+            // Change dependencies again
+            await act(async () => {
+                filterIds = ['2', '3'];
+                rerender(ONYXKEYS.COLLECTION.TEST_KEY);
+            });
+
+            // Selector should recompute and return items with id '2' and '3'
+            expect(result.current[0]).toEqual([
+                {id: '2', value: 'item2'},
+                {id: '3', value: 'item3'},
+            ]);
+            expect(selectorCallCount).toBeGreaterThan(firstChangeCallCount);
+        });
+
+        it('should handle complex dependency scenarios with multiple values', async () => {
+            type TestItem = {id: string; category: string; priority: number};
+            const testData = {
+                [`${ONYXKEYS.COLLECTION.TEST_KEY}item1`]: {id: 'item1', category: 'A', priority: 1},
+                [`${ONYXKEYS.COLLECTION.TEST_KEY}item2`]: {id: 'item2', category: 'B', priority: 2},
+                [`${ONYXKEYS.COLLECTION.TEST_KEY}item3`]: {id: 'item3', category: 'A', priority: 3},
+                [`${ONYXKEYS.COLLECTION.TEST_KEY}item4`]: {id: 'item4', category: 'B', priority: 4},
+            };
+
+            await act(async () => Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, testData as GenericCollection));
+
+            let categoryFilter = 'A';
+            let sortAscending = true;
+
+            const {result, rerender} = renderHook(() =>
+                useOnyx(
+                    ONYXKEYS.COLLECTION.TEST_KEY,
+                    {
+                        selector: (collection) => {
+                            const typedCollection = collection as OnyxCollection<TestItem>;
+                            if (!typedCollection) return [];
+
+                            const filtered = Object.values(typedCollection).filter((item) => item?.category === categoryFilter);
+
+                            return filtered.sort((a, b) => (sortAscending ? (a?.priority ?? 0) - (b?.priority ?? 0) : (b?.priority ?? 0) - (a?.priority ?? 0)));
+                        },
+                    },
+                    [categoryFilter, sortAscending],
+                ),
+            );
+
+            await act(async () => waitForPromisesToResolve());
+
+            // Should return category A items sorted ascending
+            expect(result.current[0]).toEqual([
+                {id: 'item1', category: 'A', priority: 1},
+                {id: 'item3', category: 'A', priority: 3},
+            ]);
+
+            // Change sort order only
+            await act(async () => {
+                sortAscending = false;
+                rerender(ONYXKEYS.COLLECTION.TEST_KEY);
+            });
+
+            // Should return category A items sorted descending
+            expect(result.current[0]).toEqual([
+                {id: 'item3', category: 'A', priority: 3},
+                {id: 'item1', category: 'A', priority: 1},
+            ]);
+
+            // Change category filter
+            await act(async () => {
+                categoryFilter = 'B';
+                rerender(ONYXKEYS.COLLECTION.TEST_KEY);
+            });
+
+            // Should return category B items sorted descending
+            expect(result.current[0]).toEqual([
+                {id: 'item4', category: 'B', priority: 4},
+                {id: 'item2', category: 'B', priority: 2},
+            ]);
+        });
+
+        it('should not trigger unnecessary recomputations when dependencies remain the same', async () => {
+            await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, {value: 'test'}));
+
+            const dependencies = ['constant'];
+            let selectorCallCount = 0;
+
+            const {result, rerender} = renderHook(() =>
+                useOnyx(
+                    ONYXKEYS.TEST_KEY,
+                    {
+                        selector: (data) => {
+                            selectorCallCount++;
+                            return `${dependencies.join(',')}:${(data as {value?: string})?.value}`;
+                        },
+                    },
+                    dependencies,
+                ),
+            );
+
+            await act(async () => waitForPromisesToResolve());
+
+            expect(result.current[0]).toBe('constant:test');
+            expect(selectorCallCount).toBe(1);
+
+            // Force rerender without changing dependencies
+            await act(async () => {
+                rerender(ONYXKEYS.COLLECTION.TEST_KEY);
+            });
+
+            // Selector should not recompute since dependencies haven't changed
+            expect(result.current[0]).toBe('constant:test');
+            expect(selectorCallCount).toBe(1);
+
+            // Update underlying data
+            await act(async () => Onyx.merge(ONYXKEYS.TEST_KEY, {value: 'updated'}));
+
+            // Selector should recompute due to data change
+            expect(result.current[0]).toBe('constant:updated');
+            expect(selectorCallCount).toBe(2);
+        });
     });
 
     describe('allowStaleData', () => {