From dd67718abe37c6d2f3b082d5fb409029547a33e0 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Tue, 14 Jul 2026 10:37:14 +0200 Subject: [PATCH 01/17] Add subscribed option --- lib/useOnyx.ts | 25 +++++- tests/unit/useOnyxTest.ts | 159 +++++++++++++++++++++++++++++++++++++- 2 files changed, 179 insertions(+), 5 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 6d4f8cd25..2298b4d06 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -26,6 +26,13 @@ type UseOnyxOptions = { * @see `useOnyx` cannot return `null` and so selector will replace `null` with `undefined` to maintain compatibility. */ selector?: UseOnyxSelector; + + /** + * Defaults to `true`. When `false`, keeps the connection open (value stays cache-warm) but stops + * re-rendering on background writes. It defers the render trigger, not the value: any other render still reads the latest value. + * Flipping back to `true` re-renders. + */ + subscribed?: boolean; }; type FetchStatus = 'loading' | 'loaded'; @@ -45,6 +52,13 @@ function useOnyx>( const currentDependenciesRef = useLiveRef(dependencies); const selector = options?.selector; + // Read via a ref inside the Onyx callback so toggling `subscribed` never re-subscribes. + const subscribed = options?.subscribed !== false; + const subscribedRef = useRef(subscribed); + useEffect(() => { + subscribedRef.current = subscribed; + }, [subscribed]); + // Create memoized version of selector for performance const memoizedSelector = useMemo((): UseOnyxSelector | null => { if (!selector) { @@ -150,7 +164,10 @@ function useOnyx>( // Invalidate cache when dependencies change so selector runs with new closure values onyxSnapshotCache.invalidateForKey(key); shouldGetCachedValueRef.current = true; - onStoreChangeFnRef.current(); + // Skip the re-render while paused; the next render picks up the new dependencies via `getSnapshot()`. + if (subscribedRef.current) { + onStoreChangeFnRef.current(); + } // eslint-disable-next-line react-hooks/exhaustive-deps }, [...dependencies]); @@ -266,7 +283,11 @@ function useOnyx>( onyxSnapshotCache.invalidateForKey(key); // Finally, we signal that the store changed, making `getSnapshot()` be called again. - onStoreChange(); + // Skipped while paused so background writes don't re-render; the freshest value is still + // read via `getSnapshot()` on the next render. + if (subscribedRef.current) { + onStoreChange(); + } }, reuseConnection: options?.reuseConnection, }); diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index d5b9d0017..dc6a47739 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -1,11 +1,13 @@ import {act, renderHook} from '@testing-library/react-native'; + import type {OnyxCollection, OnyxEntry, OnyxKey} from '../../lib'; +import type {UseOnyxSelector} from '../../lib/useOnyx'; +import type GenericCollection from '../utils/GenericCollection'; + import Onyx, {useOnyx} from '../../lib'; +import onyxSnapshotCache from '../../lib/OnyxSnapshotCache'; import StorageMock from '../../lib/storage'; -import type GenericCollection from '../utils/GenericCollection'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; -import onyxSnapshotCache from '../../lib/OnyxSnapshotCache'; -import type {UseOnyxSelector} from '../../lib/useOnyx'; const ONYXKEYS = { TEST_KEY: 'test', @@ -1318,4 +1320,155 @@ describe('useOnyx', () => { expect(renders.length).toBe(3); }); }); + + describe('subscribed option', () => { + type SubscribedProps = {subscribed?: boolean; tick?: number}; + + // While subscribed is false, a background write should not re-render the consumer. + it('does not re-render on a background write when subscribed is false', async () => { + await Onyx.set(ONYXKEYS.TEST_KEY, 'v1'); + + const renders: Array<{value: unknown; status: string}> = []; + const {result} = renderHook( + ({subscribed}: SubscribedProps) => { + const r = useOnyx(ONYXKEYS.TEST_KEY, {subscribed}); + renders.push({value: r[0], status: r[1].status}); + return r; + }, + {initialProps: {subscribed: false}}, + ); + + // Mount reads the warm value straight from cache + await act(async () => waitForPromisesToResolve()); + expect(result.current[0]).toEqual('v1'); + expect(renders.length).toBe(1); + + // Background write while paused — connection stays open but onStoreChange is gated + await act(async () => { + Onyx.merge(ONYXKEYS.TEST_KEY, 'v2'); + await waitForPromisesToResolve(); + }); + + // No extra render, and the value is intentionally still the old one + expect(renders.length).toBe(1); + expect(result.current[0]).toEqual('v1'); + }); + + // A render from any other cause while paused should serve the latest value, not a stale snapshot. + // Keeping the connection open and invalidating on each write is what makes this pass. + it('serves the latest value on an unrelated re-render while subscribed is false', async () => { + await Onyx.set(ONYXKEYS.TEST_KEY, 'v1'); + + const {result, rerender} = renderHook(({subscribed}: SubscribedProps) => useOnyx(ONYXKEYS.TEST_KEY, {subscribed}), { + initialProps: {subscribed: false, tick: 0} as SubscribedProps, + }); + + await act(async () => waitForPromisesToResolve()); + expect(result.current[0]).toEqual('v1'); + + // Write while paused — no re-render from Onyx + await act(async () => { + Onyx.merge(ONYXKEYS.TEST_KEY, 'v2'); + await waitForPromisesToResolve(); + }); + expect(result.current[0]).toEqual('v1'); // Not yet re-rendered + + // Force an unrelated re-render — subscribed stays false, only tick changes + await act(async () => { + rerender({subscribed: false, tick: 1}); + }); + + // getSnapshot should read fresh: v2, not the stale v1 + expect(result.current[0]).toEqual('v2'); + }); + + // Flipping subscribed from false to true (re-focus) re-renders with the latest value, and a warm + // key shows 'loaded' immediately without a loading flash. + it('catches up to the latest value with no loading flash when flipped back to subscribed', async () => { + await Onyx.set(ONYXKEYS.TEST_KEY, 'v1'); + + const {result, rerender} = renderHook(({subscribed}: SubscribedProps) => useOnyx(ONYXKEYS.TEST_KEY, {subscribed}), {initialProps: {subscribed: false} as SubscribedProps}); + + await act(async () => waitForPromisesToResolve()); + expect(result.current[0]).toEqual('v1'); + + await act(async () => { + Onyx.merge(ONYXKEYS.TEST_KEY, 'v2'); + await waitForPromisesToResolve(); + }); + expect(result.current[0]).toEqual('v1'); // Paused: still stale + + // Re-focus + await act(async () => { + rerender({subscribed: true}); + }); + + expect(result.current[0]).toEqual('v2'); + expect(result.current[1].status).toEqual('loaded'); + + // Once subscribed again, later writes re-render + await act(async () => { + Onyx.merge(ONYXKEYS.TEST_KEY, 'v3'); + await waitForPromisesToResolve(); + }); + expect(result.current[0]).toEqual('v3'); + }); + + // Default (true) is unchanged: writes re-render as before. + it('re-renders on background writes when subscribed is omitted (default true)', async () => { + await Onyx.set(ONYXKEYS.TEST_KEY, 'v1'); + + const renders: Array<{value: unknown; status: string}> = []; + const {result} = renderHook(() => { + const r = useOnyx(ONYXKEYS.TEST_KEY); + renders.push({value: r[0], status: r[1].status}); + return r; + }); + + await act(async () => waitForPromisesToResolve()); + const rendersAfterMount = renders.length; + + await act(async () => { + Onyx.merge(ONYXKEYS.TEST_KEY, 'v2'); + await waitForPromisesToResolve(); + }); + + expect(result.current[0]).toEqual('v2'); + expect(renders.length).toBeGreaterThan(rendersAfterMount); + }); + + // With two subscribers on the same key, pausing one should not stop the other from re-rendering. + it('isolates paused/active subscribers sharing a connection (reuseConnection)', async () => { + await Onyx.set(ONYXKEYS.TEST_KEY, 'v1'); + + const activeRenders: unknown[] = []; + const pausedRenders: unknown[] = []; + + const active = renderHook(() => { + const r = useOnyx(ONYXKEYS.TEST_KEY, {reuseConnection: true}); + activeRenders.push(r[0]); + return r; + }); + const paused = renderHook(() => { + const r = useOnyx(ONYXKEYS.TEST_KEY, {reuseConnection: true, subscribed: false}); + pausedRenders.push(r[0]); + return r; + }); + + await act(async () => waitForPromisesToResolve()); + const activeAfterMount = activeRenders.length; + const pausedAfterMount = pausedRenders.length; + + await act(async () => { + Onyx.merge(ONYXKEYS.TEST_KEY, 'v2'); + await waitForPromisesToResolve(); + }); + + // Active subscriber re-rendered to the new value; paused one did not re-render at all + expect(active.result.current[0]).toEqual('v2'); + expect(activeRenders.length).toBeGreaterThan(activeAfterMount); + expect(pausedRenders.length).toBe(pausedAfterMount); + expect(paused.result.current[0]).toEqual('v1'); + }); + }); }); From f4bd80c507421b29ec7a5ebf7a01a8a000c3e3a3 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Tue, 14 Jul 2026 10:42:11 +0200 Subject: [PATCH 02/17] update docs --- API.md | 2 ++ lib/Onyx.ts | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/API.md b/API.md index e766e551f..6e0237151 100644 --- a/API.md +++ b/API.md @@ -81,6 +81,7 @@ This method will be deprecated soon. Please use `Onyx.connectWithoutView()` inst | connectOptions.key | The Onyx key to subscribe to. | | connectOptions.callback | A function that will be called when the Onyx data we are subscribed changes. | | connectOptions.selector | This will be used to subscribe to a subset of an Onyx key's data. **Only used inside `useOnyx()` hook.** Using this setting on `useOnyx()` can have very positive performance benefits because the component will only re-render when the subset of data changes. Otherwise, any change of data on any property would normally cause the component to re-render (and that can be expensive from a performance standpoint). | +| connectOptions.subscribed | Defaults to `true`. **Only used inside `useOnyx()` hook.** When `false`, keeps the connection open (value stays cache-warm) but stops re-rendering on background writes. It defers the render trigger, not the value: any other render still reads the latest value, and flipping back to `true` re-renders. | **Example** ```ts @@ -103,6 +104,7 @@ Connects to an Onyx key given the options passed and listens to its changes. | connectOptions.key | The Onyx key to subscribe to. | | connectOptions.callback | A function that will be called when the Onyx data we are subscribed changes. | | connectOptions.selector | This will be used to subscribe to a subset of an Onyx key's data. **Only used inside `useOnyx()` hook.** Using this setting on `useOnyx()` can have very positive performance benefits because the component will only re-render when the subset of data changes. Otherwise, any change of data on any property would normally cause the component to re-render (and that can be expensive from a performance standpoint). | +| connectOptions.subscribed | Defaults to `true`. **Only used inside `useOnyx()` hook.** When `false`, keeps the connection open (value stays cache-warm) but stops re-rendering on background writes. It defers the render trigger, not the value: any other render still reads the latest value, and flipping back to `true` re-renders. | **Example** ```ts diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 16a0f6ec6..ffaf33b30 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -98,6 +98,9 @@ function init({ * Using this setting on `useOnyx()` can have very positive performance benefits because the component will only re-render * when the subset of data changes. Otherwise, any change of data on any property would normally * cause the component to re-render (and that can be expensive from a performance standpoint). + * @param connectOptions.subscribed Defaults to `true`. **Only used inside `useOnyx()` hook.** When `false`, keeps the connection open + * (value stays cache-warm) but stops re-rendering on background writes. It defers the render trigger, not the value: any other + * render still reads the latest value, and flipping back to `true` re-renders. * @returns The connection object to use when calling `Onyx.disconnect()`. */ function connect(connectOptions: ConnectOptions): Connection { @@ -122,6 +125,9 @@ function connect(connectOptions: ConnectOptions): Co * Using this setting on `useOnyx()` can have very positive performance benefits because the component will only re-render * when the subset of data changes. Otherwise, any change of data on any property would normally * cause the component to re-render (and that can be expensive from a performance standpoint). + * @param connectOptions.subscribed Defaults to `true`. **Only used inside `useOnyx()` hook.** When `false`, keeps the connection open + * (value stays cache-warm) but stops re-rendering on background writes. It defers the render trigger, not the value: any other + * render still reads the latest value, and flipping back to `true` re-renders. * @returns The connection object to use when calling `Onyx.disconnect()`. */ function connectWithoutView(connectOptions: ConnectOptions): Connection { From 76830e4e9012a6dfd66d97faf36d7ec27a59cf74 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Tue, 14 Jul 2026 10:54:39 +0200 Subject: [PATCH 03/17] restore import order --- tests/unit/useOnyxTest.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index dc6a47739..7733028fc 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -1,13 +1,11 @@ import {act, renderHook} from '@testing-library/react-native'; - import type {OnyxCollection, OnyxEntry, OnyxKey} from '../../lib'; -import type {UseOnyxSelector} from '../../lib/useOnyx'; -import type GenericCollection from '../utils/GenericCollection'; - import Onyx, {useOnyx} from '../../lib'; -import onyxSnapshotCache from '../../lib/OnyxSnapshotCache'; import StorageMock from '../../lib/storage'; +import type GenericCollection from '../utils/GenericCollection'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; +import onyxSnapshotCache from '../../lib/OnyxSnapshotCache'; +import type {UseOnyxSelector} from '../../lib/useOnyx'; const ONYXKEYS = { TEST_KEY: 'test', From 9e6e74e41e1f77272ae958126a38a1c23938863a Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Tue, 14 Jul 2026 10:56:56 +0200 Subject: [PATCH 04/17] prettier fix --- lib/useOnyx.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 2298b4d06..66d704aa0 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -29,7 +29,7 @@ type UseOnyxOptions = { /** * Defaults to `true`. When `false`, keeps the connection open (value stays cache-warm) but stops - * re-rendering on background writes. It defers the render trigger, not the value: any other render still reads the latest value. + * re-rendering on background writes. It defers the render trigger, not the value: any other render still reads the latest value. * Flipping back to `true` re-renders. */ subscribed?: boolean; From 9b8bcea76eeb0b6a37d316e5d2ec2c6f332d06b8 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Wed, 15 Jul 2026 10:44:52 +0200 Subject: [PATCH 05/17] remove subscribed guard on subscribedRef --- lib/useOnyx.ts | 5 +---- tests/unit/useOnyxTest.ts | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 66d704aa0..05ff3f575 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -164,10 +164,7 @@ function useOnyx>( // Invalidate cache when dependencies change so selector runs with new closure values onyxSnapshotCache.invalidateForKey(key); shouldGetCachedValueRef.current = true; - // Skip the re-render while paused; the next render picks up the new dependencies via `getSnapshot()`. - if (subscribedRef.current) { - onStoreChangeFnRef.current(); - } + onStoreChangeFnRef.current(); // eslint-disable-next-line react-hooks/exhaustive-deps }, [...dependencies]); diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 7733028fc..fe3dff758 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -1380,6 +1380,38 @@ describe('useOnyx', () => { expect(result.current[0]).toEqual('v2'); }); + // A dependencies change is consumer-driven, not a background write, so subscribed: false must not defer + // it. Uses a stable selector whose output depends on an external value fed via `dependencies` — the only + // shape where the deps-effect notify is load-bearing (getSnapshot's hasSelectorChanged can't recompute it). + it('applies a dependencies change while subscribed is false', async () => { + await Onyx.set(ONYXKEYS.TEST_KEY, 'base'); + + // Stable selector reference; its output closes over `dep`, signalled via `dependencies` + let dep = 'A'; + const selector = (value: unknown) => `${value as string}-${dep}`; + + // `dependencies` is [dep] only; `subscribed` is a prop purely to force re-renders + const {result, rerender} = renderHook(({subscribed}: SubscribedProps) => useOnyx(ONYXKEYS.TEST_KEY, {subscribed, selector}, [dep]), { + initialProps: {subscribed: false} as SubscribedProps, + }); + + await act(async () => waitForPromisesToResolve()); + expect(result.current[0]).toEqual('base-A'); + + // Warm-up re-render (dep unchanged) to clear the "read fresh from cache" flag the connect callback left set + await act(async () => rerender({subscribed: false})); + expect(result.current[0]).toEqual('base-A'); + + // Change the dependency while paused — the Onyx value is untouched, so the deps change is the only signal + await act(async () => { + dep = 'B'; + rerender({subscribed: false}); + }); + + // getSnapshot should recompute with the new dependency: base-B, not the stale base-A + expect(result.current[0]).toEqual('base-B'); + }); + // Flipping subscribed from false to true (re-focus) re-renders with the latest value, and a warm // key shows 'loaded' immediately without a loading flash. it('catches up to the latest value with no loading flash when flipped back to subscribed', async () => { From 52c7e8f3cee7f520c5abe6bdfb15161f81be56d4 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Wed, 15 Jul 2026 14:49:43 +0200 Subject: [PATCH 06/17] fix initial edge case --- lib/useOnyx.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 05ff3f575..702888a4d 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -1,12 +1,15 @@ +import type {DependencyList} from 'react'; + import {deepEqual, shallowEqual} from 'fast-equals'; import {useCallback, useEffect, useMemo, useRef, useSyncExternalStore} from 'react'; -import type {DependencyList} from 'react'; -import OnyxCache, {TASK} from './OnyxCache'; + import type {Connection} from './OnyxConnectionManager'; -import connectionManager from './OnyxConnectionManager'; -import OnyxUtils from './OnyxUtils'; import type {CollectionKeyBase, OnyxKey, OnyxValue} from './types'; + +import OnyxCache, {TASK} from './OnyxCache'; +import connectionManager from './OnyxConnectionManager'; import onyxSnapshotCache from './OnyxSnapshotCache'; +import OnyxUtils from './OnyxUtils'; import useLiveRef from './useLiveRef'; type UseOnyxSelector> = (data: OnyxValue | undefined) => TReturnValue; @@ -280,9 +283,11 @@ function useOnyx>( onyxSnapshotCache.invalidateForKey(key); // Finally, we signal that the store changed, making `getSnapshot()` be called again. - // Skipped while paused so background writes don't re-render; the freshest value is still - // read via `getSnapshot()` on the next render. - if (subscribedRef.current) { + // Background writes are skipped while paused so they don't re-render; the freshest value is + // still read via `getSnapshot()` on the next render. The INITIAL load is always delivered + // though (status still 'loading'), otherwise a cold key mounted with `subscribed: false` would + // stay stuck at 'loading' until some unrelated render — only subsequent updates should pause. + if (subscribedRef.current || resultRef.current?.[1]?.status === 'loading') { onStoreChange(); } }, From e12bd8663b3eb95b73aad685644336c2213f2c33 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Wed, 15 Jul 2026 15:25:03 +0200 Subject: [PATCH 07/17] use useLiveRef for subscribedRef --- lib/useOnyx.ts | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 702888a4d..569da1dad 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -55,12 +55,10 @@ function useOnyx>( const currentDependenciesRef = useLiveRef(dependencies); const selector = options?.selector; - // Read via a ref inside the Onyx callback so toggling `subscribed` never re-subscribes. + // Read via a ref inside the Onyx callback so toggling `subscribed` never re-subscribes. Synced during + // render (not in an effect) so a `false`→`true` flip can't miss a write that lands before effects run. const subscribed = options?.subscribed !== false; - const subscribedRef = useRef(subscribed); - useEffect(() => { - subscribedRef.current = subscribed; - }, [subscribed]); + const subscribedRef = useLiveRef(subscribed); // Create memoized version of selector for performance const memoizedSelector = useMemo((): UseOnyxSelector | null => { @@ -282,11 +280,8 @@ function useOnyx>( // Invalidate snapshot cache for this key when data changes onyxSnapshotCache.invalidateForKey(key); - // Finally, we signal that the store changed, making `getSnapshot()` be called again. - // Background writes are skipped while paused so they don't re-render; the freshest value is - // still read via `getSnapshot()` on the next render. The INITIAL load is always delivered - // though (status still 'loading'), otherwise a cold key mounted with `subscribed: false` would - // stay stuck at 'loading' until some unrelated render — only subsequent updates should pause. + // Trigger a re-render, except for paused background writes. The initial load is never paused + // though, otherwise a cold `subscribed: false` key would stay stuck 'loading' until some render. if (subscribedRef.current || resultRef.current?.[1]?.status === 'loading') { onStoreChange(); } @@ -305,7 +300,7 @@ function useOnyx>( onStoreChangeFnRef.current = null; }; }, - [key, options?.reuseConnection], + [key, options?.reuseConnection, subscribedRef], ); const result = useSyncExternalStore>(subscribe, getSnapshot); From b51368c6d2b3158abfbf8c244d839df589f6461e Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Wed, 15 Jul 2026 15:37:55 +0200 Subject: [PATCH 08/17] restore useEffect --- lib/useOnyx.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 569da1dad..507700e8f 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -55,10 +55,12 @@ function useOnyx>( const currentDependenciesRef = useLiveRef(dependencies); const selector = options?.selector; - // Read via a ref inside the Onyx callback so toggling `subscribed` never re-subscribes. Synced during - // render (not in an effect) so a `false`→`true` flip can't miss a write that lands before effects run. + // Read via a ref inside the Onyx callback so toggling `subscribed` never re-subscribes. const subscribed = options?.subscribed !== false; - const subscribedRef = useLiveRef(subscribed); + const subscribedRef = useRef(subscribed); + useEffect(() => { + subscribedRef.current = subscribed; + }, [subscribed]); // Create memoized version of selector for performance const memoizedSelector = useMemo((): UseOnyxSelector | null => { @@ -300,7 +302,7 @@ function useOnyx>( onStoreChangeFnRef.current = null; }; }, - [key, options?.reuseConnection, subscribedRef], + [key, options?.reuseConnection], ); const result = useSyncExternalStore>(subscribe, getSnapshot); From 13f825e360fe4634fe9de314e9a7e3914a957382 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Wed, 15 Jul 2026 15:42:00 +0200 Subject: [PATCH 09/17] update unit tests --- tests/unit/useOnyxTest.ts | 46 ++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index fe3dff758..fc7adb5fa 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -1,11 +1,13 @@ import {act, renderHook} from '@testing-library/react-native'; + import type {OnyxCollection, OnyxEntry, OnyxKey} from '../../lib'; +import type {UseOnyxSelector} from '../../lib/useOnyx'; +import type GenericCollection from '../utils/GenericCollection'; + import Onyx, {useOnyx} from '../../lib'; +import onyxSnapshotCache from '../../lib/OnyxSnapshotCache'; import StorageMock from '../../lib/storage'; -import type GenericCollection from '../utils/GenericCollection'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; -import onyxSnapshotCache from '../../lib/OnyxSnapshotCache'; -import type {UseOnyxSelector} from '../../lib/useOnyx'; const ONYXKEYS = { TEST_KEY: 'test', @@ -1352,8 +1354,7 @@ describe('useOnyx', () => { expect(result.current[0]).toEqual('v1'); }); - // A render from any other cause while paused should serve the latest value, not a stale snapshot. - // Keeping the connection open and invalidating on each write is what makes this pass. + // A render from any other cause while paused serves the latest value, not a stale snapshot. it('serves the latest value on an unrelated re-render while subscribed is false', async () => { await Onyx.set(ONYXKEYS.TEST_KEY, 'v1'); @@ -1380,9 +1381,7 @@ describe('useOnyx', () => { expect(result.current[0]).toEqual('v2'); }); - // A dependencies change is consumer-driven, not a background write, so subscribed: false must not defer - // it. Uses a stable selector whose output depends on an external value fed via `dependencies` — the only - // shape where the deps-effect notify is load-bearing (getSnapshot's hasSelectorChanged can't recompute it). + // A dependencies change is consumer-driven, not a background write, so subscribed: false must not defer it. it('applies a dependencies change while subscribed is false', async () => { await Onyx.set(ONYXKEYS.TEST_KEY, 'base'); @@ -1412,8 +1411,35 @@ describe('useOnyx', () => { expect(result.current[0]).toEqual('base-B'); }); - // Flipping subscribed from false to true (re-focus) re-renders with the latest value, and a warm - // key shows 'loaded' immediately without a loading flash. + // A cold key mounted with subscribed: false must still complete its initial load (only later writes pause). + it('delivers the initial load for a cold key even while subscribed is false', async () => { + await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'storage_value'); + + let renderCount = 0; + const {result} = renderHook(() => { + renderCount++; + return useOnyx(ONYXKEYS.TEST_KEY, {subscribed: false}); + }); + + // Nothing in cache yet → starts loading. + expect(result.current[1].status).toEqual('loading'); + + // The initial load is delivered while paused — no refocus, no unrelated render needed. + await act(async () => waitForPromisesToResolve()); + expect(result.current[0]).toEqual('storage_value'); + expect(result.current[1].status).toEqual('loaded'); + + // A SUBSEQUENT background write is still suppressed while paused (no re-render, value unchanged). + const rendersAfterLoad = renderCount; + await act(async () => { + Onyx.merge(ONYXKEYS.TEST_KEY, 'updated_value'); + await waitForPromisesToResolve(); + }); + expect(result.current[0]).toEqual('storage_value'); + expect(renderCount).toBe(rendersAfterLoad); + }); + + // Flipping back to subscribed re-renders with the latest value; a warm key shows 'loaded' with no flash. it('catches up to the latest value with no loading flash when flipped back to subscribed', async () => { await Onyx.set(ONYXKEYS.TEST_KEY, 'v1'); From c034d60402b18acb9647ba432f5391a211d0cab0 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Thu, 16 Jul 2026 10:00:31 +0200 Subject: [PATCH 10/17] add Catch-up for the commit effect gap --- lib/useOnyx.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 507700e8f..78f6e412f 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -60,6 +60,15 @@ function useOnyx>( const subscribedRef = useRef(subscribed); useEffect(() => { subscribedRef.current = subscribed; + + // Catch-up for the commit→effect gap: a write can land after the `false`→`true` flip render commits + // but before this effect syncs the ref, and the still-stale ref gates its `onStoreChange()`. The gated + // callback leaves `shouldGetCachedValueRef` set, so deliver it now (mirrors TanStack's + // `observer.updateResult()` after subscribing). No-op when nothing was gated: `getSnapshot()` then + // returns the same cached reference and `useSyncExternalStore` bails out. + if (subscribed && shouldGetCachedValueRef.current) { + onStoreChangeFnRef.current?.(); + } }, [subscribed]); // Create memoized version of selector for performance From 578edade372c71a256fc6505070027d8effa73bb Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Thu, 16 Jul 2026 12:46:08 +0200 Subject: [PATCH 11/17] update tests and comments --- lib/useOnyx.ts | 21 ++++++++++---------- tests/unit/useOnyxTest.ts | 41 +++++++++++++++++++++++++++++---------- 2 files changed, 41 insertions(+), 21 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 78f6e412f..3cecc9021 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -31,9 +31,9 @@ type UseOnyxOptions = { selector?: UseOnyxSelector; /** - * Defaults to `true`. When `false`, keeps the connection open (value stays cache-warm) but stops - * re-rendering on background writes. It defers the render trigger, not the value: any other render still reads the latest value. - * Flipping back to `true` re-renders. + * Defaults to `true`. When `false`, the connection stays open (the value stays cache-warm) but background + * writes no longer trigger a re-render. Only the render trigger is deferred, not the value: any render still + * reads the latest value, and flipping back to `true` re-renders with it. */ subscribed?: boolean; }; @@ -55,17 +55,16 @@ function useOnyx>( const currentDependenciesRef = useLiveRef(dependencies); const selector = options?.selector; - // Read via a ref inside the Onyx callback so toggling `subscribed` never re-subscribes. + // The Onyx callback reads `subscribed` via a ref so toggling it never re-subscribes. The ref is synced in an + // effect so the gate only ever reflects committed renders. const subscribed = options?.subscribed !== false; const subscribedRef = useRef(subscribed); useEffect(() => { subscribedRef.current = subscribed; - // Catch-up for the commit→effect gap: a write can land after the `false`→`true` flip render commits - // but before this effect syncs the ref, and the still-stale ref gates its `onStoreChange()`. The gated - // callback leaves `shouldGetCachedValueRef` set, so deliver it now (mirrors TanStack's - // `observer.updateResult()` after subscribing). No-op when nothing was gated: `getSnapshot()` then - // returns the same cached reference and `useSyncExternalStore` bails out. + // A write can land between the flip render (`false` to `true`) and this effect, while the stale ref still + // gates it. The gated callback leaves `shouldGetCachedValueRef` set, so deliver it now. No-op otherwise: + // `getSnapshot()` returns the same cached reference and `useSyncExternalStore` bails out. if (subscribed && shouldGetCachedValueRef.current) { onStoreChangeFnRef.current?.(); } @@ -291,8 +290,8 @@ function useOnyx>( // Invalidate snapshot cache for this key when data changes onyxSnapshotCache.invalidateForKey(key); - // Trigger a re-render, except for paused background writes. The initial load is never paused - // though, otherwise a cold `subscribed: false` key would stay stuck 'loading' until some render. + // Trigger a re-render unless paused. The initial load is never paused — gating it would leave + // a cold `subscribed: false` key stuck at 'loading' until some unrelated render. if (subscribedRef.current || resultRef.current?.[1]?.status === 'loading') { onStoreChange(); } diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index fc7adb5fa..cbd37f4d3 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -1324,7 +1324,6 @@ describe('useOnyx', () => { describe('subscribed option', () => { type SubscribedProps = {subscribed?: boolean; tick?: number}; - // While subscribed is false, a background write should not re-render the consumer. it('does not re-render on a background write when subscribed is false', async () => { await Onyx.set(ONYXKEYS.TEST_KEY, 'v1'); @@ -1354,7 +1353,6 @@ describe('useOnyx', () => { expect(result.current[0]).toEqual('v1'); }); - // A render from any other cause while paused serves the latest value, not a stale snapshot. it('serves the latest value on an unrelated re-render while subscribed is false', async () => { await Onyx.set(ONYXKEYS.TEST_KEY, 'v1'); @@ -1407,11 +1405,9 @@ describe('useOnyx', () => { rerender({subscribed: false}); }); - // getSnapshot should recompute with the new dependency: base-B, not the stale base-A expect(result.current[0]).toEqual('base-B'); }); - // A cold key mounted with subscribed: false must still complete its initial load (only later writes pause). it('delivers the initial load for a cold key even while subscribed is false', async () => { await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'storage_value'); @@ -1421,7 +1417,6 @@ describe('useOnyx', () => { return useOnyx(ONYXKEYS.TEST_KEY, {subscribed: false}); }); - // Nothing in cache yet → starts loading. expect(result.current[1].status).toEqual('loading'); // The initial load is delivered while paused — no refocus, no unrelated render needed. @@ -1429,7 +1424,7 @@ describe('useOnyx', () => { expect(result.current[0]).toEqual('storage_value'); expect(result.current[1].status).toEqual('loaded'); - // A SUBSEQUENT background write is still suppressed while paused (no re-render, value unchanged). + // A subsequent background write is still suppressed while paused (no re-render, value unchanged). const rendersAfterLoad = renderCount; await act(async () => { Onyx.merge(ONYXKEYS.TEST_KEY, 'updated_value'); @@ -1439,7 +1434,6 @@ describe('useOnyx', () => { expect(renderCount).toBe(rendersAfterLoad); }); - // Flipping back to subscribed re-renders with the latest value; a warm key shows 'loaded' with no flash. it('catches up to the latest value with no loading flash when flipped back to subscribed', async () => { await Onyx.set(ONYXKEYS.TEST_KEY, 'v1'); @@ -1454,7 +1448,6 @@ describe('useOnyx', () => { }); expect(result.current[0]).toEqual('v1'); // Paused: still stale - // Re-focus await act(async () => { rerender({subscribed: true}); }); @@ -1470,7 +1463,36 @@ describe('useOnyx', () => { expect(result.current[0]).toEqual('v3'); }); - // Default (true) is unchanged: writes re-render as before. + // The production pattern (`subscribed: isFocused`) mounts subscribed and blurs later, so gating must + // engage via the ref sync on the flip — mounting already-paused (covered above) doesn't exercise it. + it('stops re-rendering on background writes after flipping subscribed to false', async () => { + await Onyx.set(ONYXKEYS.TEST_KEY, 'v1'); + + const renders: unknown[] = []; + const {result, rerender} = renderHook( + ({subscribed}: SubscribedProps) => { + const r = useOnyx(ONYXKEYS.TEST_KEY, {subscribed}); + renders.push(r[0]); + return r; + }, + {initialProps: {subscribed: true} as SubscribedProps}, + ); + + await act(async () => waitForPromisesToResolve()); + expect(result.current[0]).toEqual('v1'); + + await act(async () => rerender({subscribed: false})); + const rendersAfterFlip = renders.length; + + await act(async () => { + Onyx.merge(ONYXKEYS.TEST_KEY, 'v2'); + await waitForPromisesToResolve(); + }); + + expect(renders.length).toBe(rendersAfterFlip); + expect(result.current[0]).toEqual('v1'); + }); + it('re-renders on background writes when subscribed is omitted (default true)', async () => { await Onyx.set(ONYXKEYS.TEST_KEY, 'v1'); @@ -1493,7 +1515,6 @@ describe('useOnyx', () => { expect(renders.length).toBeGreaterThan(rendersAfterMount); }); - // With two subscribers on the same key, pausing one should not stop the other from re-rendering. it('isolates paused/active subscribers sharing a connection (reuseConnection)', async () => { await Onyx.set(ONYXKEYS.TEST_KEY, 'v1'); From a2842671832a9cbd81b233c11f6f834e80f4f80e Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Mon, 20 Jul 2026 19:56:27 +0200 Subject: [PATCH 12/17] wWhile unsubscribed, keep returning the last delivered result --- lib/useOnyx.ts | 7 +++++++ tests/unit/useOnyxTest.ts | 18 ++++++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 3cecc9021..48b32951f 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -185,6 +185,13 @@ function useOnyx>( const lastComputedSelectorRef = useRef(memoizedSelector); const getSnapshot = useCallback(() => { + // While unsubscribed, keep returning the last delivered result so incidental re-renders don't leak fresh data past the gate. Pending writes leave `shouldGetCachedValueRef` + // set, and the `subscribed` flip effect delivers the fresh value once the consumer resubscribes. + // First connection and `loading` are exempt so a cold `subscribed: false` key still resolves. + if (!subscribedRef.current && connectedKeyRef.current === key && resultRef.current[1].status !== 'loading') { + return resultRef.current; + } + // Check if we have any cache for this Onyx key // Don't use cache during active data updates (when shouldGetCachedValueRef is true) const isFirstConnection = connectedKeyRef.current !== key; diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index cbd37f4d3..5b35bf6c7 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -1353,7 +1353,7 @@ describe('useOnyx', () => { expect(result.current[0]).toEqual('v1'); }); - it('serves the latest value on an unrelated re-render while subscribed is false', async () => { + it('keeps returning the last delivered value on an unrelated re-render while subscribed is false, then delivers on resubscribe', async () => { await Onyx.set(ONYXKEYS.TEST_KEY, 'v1'); const {result, rerender} = renderHook(({subscribed}: SubscribedProps) => useOnyx(ONYXKEYS.TEST_KEY, {subscribed}), { @@ -1375,12 +1375,19 @@ describe('useOnyx', () => { rerender({subscribed: false, tick: 1}); }); - // getSnapshot should read fresh: v2, not the stale v1 + // The snapshot stays frozen at the last delivered value: incidental re-renders must not leak + // fresh data past the gate (otherwise any parent/focus re-render defeats the pause). + expect(result.current[0]).toEqual('v1'); + + // Resubscribing delivers the pending fresh value + await act(async () => { + rerender({subscribed: true, tick: 1}); + }); expect(result.current[0]).toEqual('v2'); }); // A dependencies change is consumer-driven, not a background write, so subscribed: false must not defer it. - it('applies a dependencies change while subscribed is false', async () => { + it('defers a dependencies change while subscribed is false and applies it on resubscribe', async () => { await Onyx.set(ONYXKEYS.TEST_KEY, 'base'); // Stable selector reference; its output closes over `dep`, signalled via `dependencies` @@ -1399,12 +1406,15 @@ describe('useOnyx', () => { await act(async () => rerender({subscribed: false})); expect(result.current[0]).toEqual('base-A'); - // Change the dependency while paused — the Onyx value is untouched, so the deps change is the only signal + // Change the dependency while paused — the snapshot stays frozen at the last delivered value await act(async () => { dep = 'B'; rerender({subscribed: false}); }); + expect(result.current[0]).toEqual('base-A'); + // Resubscribing recomputes with the new dependency closure + await act(async () => rerender({subscribed: true})); expect(result.current[0]).toEqual('base-B'); }); From db077127234d79e41a9085e41b81a80eda0961e9 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Mon, 20 Jul 2026 20:25:09 +0200 Subject: [PATCH 13/17] add subscribed hasSelectorChanged guard --- lib/useOnyx.ts | 8 +++++++- tests/unit/useOnyxTest.ts | 28 ++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 48b32951f..dd743202e 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -188,7 +188,14 @@ function useOnyx>( // While unsubscribed, keep returning the last delivered result so incidental re-renders don't leak fresh data past the gate. Pending writes leave `shouldGetCachedValueRef` // set, and the `subscribed` flip effect delivers the fresh value once the consumer resubscribes. // First connection and `loading` are exempt so a cold `subscribed: false` key still resolves. + const hasSelectorChanged = lastComputedSelectorRef.current !== memoizedSelector; + if (!subscribedRef.current && connectedKeyRef.current === key && resultRef.current[1].status !== 'loading') { + // A selector-identity change is a consumer-driven dirtying signal with no Onyx write behind it, + // so mark the snapshot dirty; otherwise the catch-up effect can't detect it on resubscribe. + if (hasSelectorChanged) { + shouldGetCachedValueRef.current = true; + } return resultRef.current; } @@ -206,7 +213,6 @@ function useOnyx>( // We get the value from cache while the first connection to Onyx is being made or if the key has changed, // so we can return any cached value right away. For the case where the key has changed, If we don't return the cached value right away, then the UI will show the incorrect (previous) value for a brief period which looks like a UI glitch to the user. After the connection is made, we only // update `newValueRef` when `Onyx.connect()` callback is fired. - const hasSelectorChanged = lastComputedSelectorRef.current !== memoizedSelector; if (isFirstConnection || shouldGetCachedValueRef.current || hasSelectorChanged) { // Gets the value from cache and maps it with selector. It changes `null` to `undefined` for `useOnyx` compatibility. const value = OnyxUtils.tryGetCachedValue(key) as OnyxValue; diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 5b35bf6c7..735437dd3 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -1557,5 +1557,33 @@ describe('useOnyx', () => { expect(pausedRenders.length).toBe(pausedAfterMount); expect(paused.result.current[0]).toEqual('v1'); }); + + // A selector-identity change while paused is a consumer-driven dirtying signal with no Onyx write behind + // it. The paused fast path must mark the snapshot dirty so resubscribe recomputes; otherwise the hook + // keeps returning the old selector output until some later store update or parent render. + it('recomputes a selector change on resubscribe, not while paused', async () => { + await Onyx.set(ONYXKEYS.TEST_KEY, 'v1'); + + const selectorA = (value: unknown) => `A:${value as string}`; + const selectorB = (value: unknown) => `B:${value as string}`; + + type SelectorProps = {subscribed?: boolean; selector: (value: unknown) => string}; + const {result, rerender} = renderHook(({subscribed, selector}: SelectorProps) => useOnyx(ONYXKEYS.TEST_KEY, {subscribed, selector}), { + initialProps: {subscribed: false, selector: selectorA} as SelectorProps, + }); + + await act(async () => waitForPromisesToResolve()); + expect(result.current[0]).toEqual('A:v1'); + + // Selector identity changes while paused — no Onyx write, no dependency change + await act(async () => rerender({subscribed: false, selector: selectorB})); + + // Frozen: the selector change is deferred, not applied while paused + expect(result.current[0]).toEqual('A:v1'); + + // Resubscribing must catch up to the new selector output + await act(async () => rerender({subscribed: true, selector: selectorB})); + expect(result.current[0]).toEqual('B:v1'); + }); }); }); From a5f78e58fb830925e7c2f1b52ce4ce15b2872ccf Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Tue, 21 Jul 2026 10:19:30 +0200 Subject: [PATCH 14/17] update comments --- lib/useOnyx.ts | 5 +++-- tests/unit/useOnyxTest.ts | 7 +++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index dd743202e..bf1359507 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -32,8 +32,9 @@ type UseOnyxOptions = { /** * Defaults to `true`. When `false`, the connection stays open (the value stays cache-warm) but background - * writes no longer trigger a re-render. Only the render trigger is deferred, not the value: any render still - * reads the latest value, and flipping back to `true` re-renders with it. + * writes no longer trigger a re-render, and the value is frozen: incidental re-renders keep returning the + * last delivered result instead of the latest one. The initial load is exempt, so a cold key still resolves. + * Flipping back to `true` re-renders and catches up to the latest value. */ subscribed?: boolean; }; diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 735437dd3..1b0d32ead 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -1558,10 +1558,9 @@ describe('useOnyx', () => { expect(paused.result.current[0]).toEqual('v1'); }); - // A selector-identity change while paused is a consumer-driven dirtying signal with no Onyx write behind - // it. The paused fast path must mark the snapshot dirty so resubscribe recomputes; otherwise the hook - // keeps returning the old selector output until some later store update or parent render. - it('recomputes a selector change on resubscribe, not while paused', async () => { + // A selector-identity change while paused must be frozen like any other update: the paused fast path + // returns the last delivered output instead of recomputing. On resubscribe the new selector applies + it('freezes a selector change while paused and applies it on resubscribe', async () => { await Onyx.set(ONYXKEYS.TEST_KEY, 'v1'); const selectorA = (value: unknown) => `A:${value as string}`; From a513e2a2f6f0b09941f99b8d4c7cfdd1f4f3406f Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Mon, 27 Jul 2026 09:11:08 +0200 Subject: [PATCH 15/17] Merge branch 'main' of github.com:Expensify/react-native-onyx into feat/useonyx-subscribed --- lib/OnyxCache.ts | 33 ++++-- lib/memoizedShallowEqual.ts | 41 +++++++ lib/storage/providers/SQLiteProvider.ts | 106 ++++++++++++++++-- lib/useOnyx.ts | 7 +- lib/utils.ts | 20 ++++ package-lock.json | 4 +- package.json | 2 +- tests/unit/memoizedShallowEqualTest.ts | 76 +++++++++++++ tests/unit/mocks/sqliteMock.ts | 63 ++++++----- tests/unit/onyxCacheTest.tsx | 21 ++++ .../storage/providers/SQLiteProviderTest.ts | 93 +++++++++++++++ tests/unit/utilsTest.ts | 47 ++++++++ 12 files changed, 462 insertions(+), 51 deletions(-) create mode 100644 lib/memoizedShallowEqual.ts create mode 100644 tests/unit/memoizedShallowEqualTest.ts diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index 23fae2130..e105a0ab6 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -385,7 +385,7 @@ class OnyxCache { // Initialize frozen snapshots for collection keys for (const collectionKey of collectionKeys) { if (!this.collectionSnapshots.has(collectionKey)) { - this.collectionSnapshots.set(collectionKey, Object.freeze({})); + this.collectionSnapshots.set(collectionKey, FROZEN_EMPTY_COLLECTION); } } } @@ -403,6 +403,7 @@ class OnyxCache { const members: NonUndefined> = {}; let hasMemberChanges = false; + let hasMembers = false; // Use the indexed forward lookup for O(collectionMembers) iteration. // Falls back to scanning all storageKeys if the index isn't populated yet. @@ -421,6 +422,7 @@ class OnyxCache { // and should not be included in the frozen collection snapshot. if (val !== undefined && val !== null) { members[key] = val; + hasMembers = true; // Check if this member's reference changed from the old snapshot if (!hasMemberChanges && (!previousSnapshot || previousSnapshot[key] !== val)) { @@ -449,6 +451,14 @@ class OnyxCache { return; } + // When the collection has no members, reuse one shared empty object for every empty + // collection. That way reads can tell a collection is empty with a quick `===` check + // instead of looping over its keys every time. + if (!hasMembers) { + this.collectionSnapshots.set(collectionKey, FROZEN_EMPTY_COLLECTION); + return; + } + Object.freeze(members); this.collectionSnapshots.set(collectionKey, members); @@ -466,18 +476,21 @@ class OnyxCache { } const snapshot = this.collectionSnapshots.get(collectionKey); - if (utils.isEmptyObject(snapshot)) { - // We check storageKeys.size (not collection-specific keys) to distinguish - // "init complete, this collection is genuinely empty" from "init not done yet." - // During init, setAllKeys loads ALL keys at once — so if any key exists, - // the full storage picture is loaded and an empty collection is truly empty. - // Returning undefined before init prevents subscribers from seeing a false empty state. - if (this.storageKeys.size > 0) { - return FROZEN_EMPTY_COLLECTION; - } + + // We never stored anything for this collection key. + if (snapshot === undefined) { return undefined; } + // The collection is empty (it holds our shared empty object). But "empty" is ambiguous + // during startup: we can't tell an actually-empty collection apart from one whose data + // hasn't loaded yet. Once any key exists, we know setAllKeys has run and loaded everything, + // so an empty collection really is empty. Before that, return undefined so subscribers + // don't briefly see a collection as empty when it just hasn't loaded. + if (snapshot === FROZEN_EMPTY_COLLECTION) { + return this.storageKeys.size > 0 ? FROZEN_EMPTY_COLLECTION : undefined; + } + return snapshot; } } diff --git a/lib/memoizedShallowEqual.ts b/lib/memoizedShallowEqual.ts new file mode 100644 index 000000000..4a015f3d9 --- /dev/null +++ b/lib/memoizedShallowEqual.ts @@ -0,0 +1,41 @@ +import {shallowEqual} from 'fast-equals'; + +/** + * Memoizes shallowEqual verdicts by the identity of the compared objects. Onyx values are + * treated as immutable (merge/set replace objects, never mutate), so a (prev, next) reference + * pair always yields the same verdict. In the hot case — N no-selector hooks on the same big + * key — every hook compares the exact same two cache-owned objects, so the first hook pays for + * the O(keys) walk and the rest resolve in O(1). WeakMap keys make stale entries impossible to + * read (lookup requires holding both exact objects) and let GC reclaim them. + */ +const shallowEqualVerdicts = new WeakMap>(); + +/** + * Identity-pair-memoized shallowEqual: same (a, b) references → cached verdict, no walk. + */ +function memoizedShallowEqual(a: unknown, b: unknown): boolean { + // Only object pairs are memoizable (WeakMap keys) — anything else is O(1) to compare anyway. + if (typeof a !== 'object' || a === null || typeof b !== 'object' || b === null) { + return shallowEqual(a, b); + } + + let verdictsForA = shallowEqualVerdicts.get(a); + + if (!verdictsForA) { + verdictsForA = new WeakMap(); + shallowEqualVerdicts.set(a, verdictsForA); + } + + const cachedVerdict = verdictsForA.get(b); + + if (cachedVerdict !== undefined) { + return cachedVerdict; + } + + const verdict = shallowEqual(a, b); + verdictsForA.set(b, verdict); + + return verdict; +} + +export default memoizedShallowEqual; diff --git a/lib/storage/providers/SQLiteProvider.ts b/lib/storage/providers/SQLiteProvider.ts index ff1fbf4d6..c03efb66e 100644 --- a/lib/storage/providers/SQLiteProvider.ts +++ b/lib/storage/providers/SQLiteProvider.ts @@ -2,7 +2,7 @@ * The SQLiteStorage provider stores everything in a key/value store by * converting the value to a JSON string */ -import type {BatchQueryCommand, NitroSQLiteConnection} from 'react-native-nitro-sqlite'; +import type {BatchQueryCommand, NitroSQLiteConnection, QueryResult} from 'react-native-nitro-sqlite'; import {open} from 'react-native-nitro-sqlite'; import {getFreeDiskStorage} from 'react-native-device-info'; import type {FastMergeReplaceNullPatch} from '../../utils'; @@ -11,6 +11,13 @@ import type StorageProvider from './types'; import type {StorageKeyList, StorageKeyValuePair} from './types'; import classifySQLiteError from './classifySQLiteError'; +/** + * The result of the `PRAGMA compile_options`, which lists SQLite compile-time options + */ +type CompileOptionsResult = { + compile_options: string; +}; + /** * The type of the key-value pair stored in the SQLite database * @property record_key - the key of the record @@ -36,6 +43,40 @@ type PageCountResult = { }; const DB_NAME = 'OnyxDB'; +const SQLITE_MAX_VARIABLE_NUMBER = 32766; +const COMPILE_OPTIONS = { + MAX_VARIABLE_NUMBER: 'MAX_VARIABLE_NUMBER', +} as const; + +/** SQLite's maximum number of bound parameters per statement, read once from PRAGMA compile_options in init(). */ +let sqliteMaxVariableNumber = SQLITE_MAX_VARIABLE_NUMBER; + +/** + * Returns the value of a compile option from the rows returned by `PRAGMA compile_options`. + * For flag-only options (e.g. `ENABLE_FTS3`), returns an empty string when the option is present. + */ +function getCompileOptionValue(compileOptionsResult: QueryResult, optionName: string): string | undefined { + const optionPrefix = `${optionName}=`; + const rowCount = compileOptionsResult.rows?.length ?? 0; + + for (let index = 0; index < rowCount; index++) { + const compileOption = compileOptionsResult.rows?.item(index)?.compile_options; + + if (!compileOption) { + continue; + } + + if (compileOption === optionName) { + return ''; + } + + if (compileOption.startsWith(optionPrefix)) { + return compileOption.slice(optionPrefix.length); + } + } + + return undefined; +} /** * Prevents the stringifying of the object markers. @@ -81,6 +122,13 @@ const provider: StorageProvider = { provider.store.execute('PRAGMA CACHE_SIZE=-20000;'); provider.store.execute('PRAGMA synchronous=NORMAL;'); provider.store.execute('PRAGMA journal_mode=WAL;'); + + const compileOptionsResult = provider.store.execute('PRAGMA compile_options;'); + + // Get the value of MAX_VARIABLE_NUMBER from the compile options and + // stores it in a global variable, that is going to be used during runtime. + const maxVariableNumber = Number(getCompileOptionValue(compileOptionsResult, COMPILE_OPTIONS.MAX_VARIABLE_NUMBER)); + sqliteMaxVariableNumber = maxVariableNumber > 0 ? maxVariableNumber : SQLITE_MAX_VARIABLE_NUMBER; }, getItem(key) { if (!provider.store) { @@ -105,12 +153,29 @@ const provider: StorageProvider = { throw new Error('Store is not initialized!'); } - const placeholders = keys.map(() => '?').join(','); - const command = `SELECT record_key, valueJSON FROM keyvaluepairs WHERE record_key IN (${placeholders});`; - return provider.store.executeAsync(command, keys).then(({rows}) => { - // eslint-disable-next-line no-underscore-dangle - const result = rows?._array.map((row) => [row.record_key, JSON.parse(row.valueJSON)]); - return (result ?? []) as StorageKeyValuePair[]; + if (keys.length === 0) { + return Promise.resolve([]); + } + + const keyChunks = utils.chunkArray(keys, sqliteMaxVariableNumber); + + return Promise.all( + keyChunks.map((keyChunk) => { + if (!provider.store) { + throw new Error('Store is not initialized!'); + } + + const placeholders = keyChunk.map(() => '?').join(','); + const command = `SELECT record_key, valueJSON FROM keyvaluepairs WHERE record_key IN (${placeholders});`; + return provider.store.executeAsync(command, keyChunk); + }), + ).then((results) => { + const result = results.flatMap( + ({rows}) => + // eslint-disable-next-line no-underscore-dangle + rows?._array.map((row) => [row.record_key, JSON.parse(row.valueJSON)]) ?? [], + ); + return result as StorageKeyValuePair[]; }); }, setItem(key, value) { @@ -126,7 +191,7 @@ const provider: StorageProvider = { } const query = 'REPLACE INTO keyvaluepairs (record_key, valueJSON) VALUES (?, ?);'; - const params = pairs.map((pair) => [pair[0], JSON.stringify(pair[1] === undefined ? null : pair[1])]); + const params = pairs.map(([key, value]) => [key, JSON.stringify(value === undefined ? null : value)]); if (utils.isEmptyObject(params)) { return Promise.resolve(); } @@ -225,9 +290,28 @@ const provider: StorageProvider = { throw new Error('Store is not initialized!'); } - const placeholders = keys.map(() => '?').join(','); - const query = `DELETE FROM keyvaluepairs WHERE record_key IN (${placeholders});`; - return provider.store.executeAsync(query, keys).then(() => undefined); + if (keys.length === 0) { + return Promise.resolve(); + } + + const keyChunks = utils.chunkArray(keys, sqliteMaxVariableNumber); + + const buildDeleteQuery = (keyChunk: readonly string[]) => { + const placeholders = keyChunk.map(() => '?').join(','); + return `DELETE FROM keyvaluepairs WHERE record_key IN (${placeholders});`; + }; + + if (keyChunks.length === 1) { + const keyChunk = keyChunks[0]; + return provider.store.executeAsync(buildDeleteQuery(keyChunk), keyChunk).then(() => undefined); + } + + const commands: BatchQueryCommand[] = keyChunks.map((keyChunk) => ({ + query: buildDeleteQuery(keyChunk), + params: keyChunk, + })); + + return provider.store.executeBatchAsync(commands).then(() => undefined); }, clear() { if (!provider.store) { diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index bf1359507..994ff2168 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -10,6 +10,7 @@ import OnyxCache, {TASK} from './OnyxCache'; import connectionManager from './OnyxConnectionManager'; import onyxSnapshotCache from './OnyxSnapshotCache'; import OnyxUtils from './OnyxUtils'; +import memoizedShallowEqual from './memoizedShallowEqual'; import useLiveRef from './useLiveRef'; type UseOnyxSelector> = (data: OnyxValue | undefined) => TReturnValue; @@ -240,9 +241,11 @@ function useOnyx>( // shallowEqual checks === first (O(1) for frozen snapshots and stable selector references), // then falls back to comparing top-level properties for individual keys that may have - // new references with equivalent content. + // new references with equivalent content. The comparison is memoized by object identity + // (see `memoizedShallowEqual`) so N hooks comparing the same two cache objects pay for + // one walk in total instead of one walk each. // Normalize null to undefined to ensure consistent comparison (both represent "no value"). - const areValuesEqual = shallowEqual(previousValueRef.current ?? undefined, newValueRef.current ?? undefined); + const areValuesEqual = memoizedShallowEqual(previousValueRef.current ?? undefined, newValueRef.current ?? undefined); // We update the cached value and the result in the following conditions: // We will update the cached value and the result in any of the following situations: diff --git a/lib/utils.ts b/lib/utils.ts index b469c1acd..287f4cf26 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -322,6 +322,25 @@ function omit(obj: Record, condition: string | string[] return filterObject(obj, condition, false); } +/** + * Splits an array into chunks no larger than maxChunkSize. + */ +function chunkArray(items: readonly T[], maxChunkSize: number): T[][] { + if (items.length === 0) { + return []; + } + + if (items.length <= maxChunkSize) { + return [items as T[]]; + } + + const chunks: T[][] = []; + for (let i = 0; i < items.length; i += maxChunkSize) { + chunks.push(items.slice(i, i + maxChunkSize)); + } + return chunks; +} + export default { fastMerge, isEmptyObject, @@ -330,6 +349,7 @@ export default { checkCompatibilityWithExistingValue, pick, omit, + chunkArray, ONYX_INTERNALS__REPLACE_OBJECT_MARK, }; export type {FastMergeResult, FastMergeReplaceNullPatch, FastMergeOptions}; diff --git a/package-lock.json b/package-lock.json index a16aa9a95..5697ed399 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "react-native-onyx", - "version": "3.0.91", + "version": "3.0.94", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "react-native-onyx", - "version": "3.0.91", + "version": "3.0.94", "license": "MIT", "dependencies": { "ascii-table": "0.0.9", diff --git a/package.json b/package.json index 08e0b3cd9..bdc06f78a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "react-native-onyx", - "version": "3.0.91", + "version": "3.0.94", "author": "Expensify, Inc.", "homepage": "https://expensify.com", "description": "State management for React Native", diff --git a/tests/unit/memoizedShallowEqualTest.ts b/tests/unit/memoizedShallowEqualTest.ts new file mode 100644 index 000000000..15b76159d --- /dev/null +++ b/tests/unit/memoizedShallowEqualTest.ts @@ -0,0 +1,76 @@ +import memoizedShallowEqual from '../../lib/memoizedShallowEqual'; + +describe('memoizedShallowEqual', () => { + describe('shallowEqual semantics', () => { + it('should return true for the same reference', () => { + const obj = {a: 1}; + expect(memoizedShallowEqual(obj, obj)).toBe(true); + }); + + it('should return true for different references with shallowly-equal content', () => { + const member = {name: 'John'}; + expect(memoizedShallowEqual({a: 1, member}, {a: 1, member})).toBe(true); + }); + + it('should return false when a top-level value differs', () => { + expect(memoizedShallowEqual({a: 1}, {a: 2})).toBe(false); + }); + + it('should return false when key counts differ', () => { + expect(memoizedShallowEqual({a: 1}, {a: 1, b: 2})).toBe(false); + }); + + it('should return false for equal deep content with different nested references', () => { + // Shallow, not deep: nested objects are compared by reference. + expect(memoizedShallowEqual({member: {name: 'John'}}, {member: {name: 'John'}})).toBe(false); + }); + + it('should handle non-object inputs', () => { + expect(memoizedShallowEqual(undefined, undefined)).toBe(true); + expect(memoizedShallowEqual(undefined, {})).toBe(false); + expect(memoizedShallowEqual('a', 'a')).toBe(true); + expect(memoizedShallowEqual('a', 'b')).toBe(false); + expect(memoizedShallowEqual(1, 1)).toBe(true); + expect(memoizedShallowEqual(NaN, NaN)).toBe(true); + }); + + it('should handle arrays', () => { + expect(memoizedShallowEqual([1, 2], [1, 2])).toBe(true); + expect(memoizedShallowEqual([1, 2], [1, 3])).toBe(false); + }); + }); + + describe('memoization', () => { + it('should return the cached verdict for the same object pair without re-comparing', () => { + const a = {name: 'John'}; + const b = {name: 'Jane'}; + expect(memoizedShallowEqual(a, b)).toBe(false); + + // Mutate `b` so the objects are now content-equal. Onyx values are immutable, + // so the memo is expected to keep returning the verdict computed for this exact + // (a, b) pair — proving the second call resolved from the cache, not a re-compare. + b.name = 'John'; + expect(memoizedShallowEqual(a, b)).toBe(false); + }); + + it('should cache verdicts per pair, not per object', () => { + const a = {x: 1}; + const equalToA = {x: 1}; + const differentFromA = {x: 2}; + + expect(memoizedShallowEqual(a, equalToA)).toBe(true); + expect(memoizedShallowEqual(a, differentFromA)).toBe(false); + + // Both verdicts are retained independently for the same `a`. + expect(memoizedShallowEqual(a, equalToA)).toBe(true); + expect(memoizedShallowEqual(a, differentFromA)).toBe(false); + }); + + it('should not memoize non-object inputs', () => { + // Primitives cannot be WeakMap keys; these calls must not throw and must compare directly. + expect(memoizedShallowEqual(1, {})).toBe(false); + expect(memoizedShallowEqual({}, 1)).toBe(false); + expect(memoizedShallowEqual(null, null)).toBe(true); + }); + }); +}); diff --git a/tests/unit/mocks/sqliteMock.ts b/tests/unit/mocks/sqliteMock.ts index 8b5cd1d45..d7282e80b 100644 --- a/tests/unit/mocks/sqliteMock.ts +++ b/tests/unit/mocks/sqliteMock.ts @@ -7,13 +7,13 @@ * - open({name}) * - connection.execute(sql) * - connection.executeAsync(sql, params?) - * - connection.executeBatchAsync([{query, params}]) + * - connection.executeBatchAsync([{query, params}, ...]) * * Result rows are shaped to match Nitro: `{rows: {_array, item, length}}`. */ import BetterSqlite3 from 'better-sqlite3'; import type {Database} from 'better-sqlite3'; -import type {NitroSQLiteConnection, NitroSQLiteQueryResultRows, QueryResult, QueryResultRow, SQLiteQueryParams} from 'react-native-nitro-sqlite'; +import type {BatchQueryCommand, NitroSQLiteConnection, NitroSQLiteQueryResultRows, QueryResult, QueryResultRow, SQLiteQueryParams} from 'react-native-nitro-sqlite'; // `better-sqlite3` is declared as `export = Database` (CommonJS), so the type is // derived from the default import's namespace rather than via a named type import. @@ -54,7 +54,7 @@ function wrapRows(rowsArray: TRow[]): NitroSQLiteQu }; } -function prepareAndBind(database: Database, sql: string, parameters?: SQLiteQueryParams) { +function prepareAndBind(database: Database, sql: BatchQueryCommand['query'], parameters?: BatchQueryCommand['params']) { const namedOrder = extractNamedParameterOrder(sql); if (namedOrder) { // Map positional parameters array to named bindings object — NitroSQLite's @@ -71,6 +71,35 @@ function prepareAndBind(database: Database, sql: string, parameters?: SQLiteQuer return {statement: database.prepare(sql), boundArguments: parameters ?? []}; } +/** + * Expands batch commands the same way NitroSQLite does in `batchParamsToCommands`: + * `params` is either one binding set for the query, or an array of binding sets + * (same query executed once per row). + */ +function batchParamsToCommands(commands: BatchQueryCommand[]): BatchQueryCommand[] { + const expanded: BatchQueryCommand[] = []; + + for (const command of commands) { + const {query, params} = command; + + if (!params) { + expanded.push({query}); + continue; + } + + if (Array.isArray(params[0])) { + for (const rowParams of params as SQLiteQueryParams[]) { + expanded.push({query, params: rowParams}); + } + continue; + } + + expanded.push({query, params: params as SQLiteQueryParams}); + } + + return expanded; +} + function runOne(database: Database, sql: string, parameters?: SQLiteQueryParams): QueryResult { // Multi-statement (CREATE TABLE; SELECT ...; etc.) — better-sqlite3 cannot // prepare more than one statement at a time. SQLiteProvider's init() issues @@ -120,29 +149,13 @@ function makeConnection(name: string): Pick { - for (const command of commands) { - const namedOrder = extractNamedParameterOrder(command.query); - const statement = connection.prepare(command.query); - const parameterRows = (command.params ?? []) as SQLiteQueryParams[]; - if (parameterRows.length === 0) { - const info = statement.run(); - total += info.changes; - continue; - } - for (const row of parameterRows) { - if (namedOrder) { - const bindings: Record = {}; - for (let index = 0; index < namedOrder.length; index++) { - bindings[namedOrder[index]] = row[index]; - } - const info = statement.run(bindings); - total += info.changes; - } else { - const info = statement.run(...row); - total += info.changes; - } - } + for (const command of expandedCommands) { + const {statement, boundArguments} = prepareAndBind(connection, command.query, command.params); + const info = statement.run(...(boundArguments as unknown[])); + total += info.changes; } })(); return Promise.resolve({rowsAffected: total}); diff --git a/tests/unit/onyxCacheTest.tsx b/tests/unit/onyxCacheTest.tsx index 713232459..e7a3bc7af 100644 --- a/tests/unit/onyxCacheTest.tsx +++ b/tests/unit/onyxCacheTest.tsx @@ -625,6 +625,27 @@ describe('Onyx', () => { }); }); + it('should return the stable empty reference after all members are removed', async () => { + await initOnyx(); + // Unrelated key keeps storageKeys non-empty so an empty collection is treated as "loaded and empty" + await Onyx.set(ONYX_KEYS.TEST_KEY, 'value'); + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1}); + + const populated = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + expect(Object.keys(populated!)).toHaveLength(1); + + // Remove the last member — the collection becomes empty + await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, null); + + const first = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + const second = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION); + + expect(first).toBeDefined(); + expect(Object.keys(first!)).toHaveLength(0); + // Reads must return the same empty reference so useSyncExternalStore doesn't re-render + expect(first).toBe(second); + }); + it('should preserve unchanged member references when a sibling is updated', async () => { await initOnyx(); const member1Value = {id: 1, name: 'unchanged'}; diff --git a/tests/unit/storage/providers/SQLiteProviderTest.ts b/tests/unit/storage/providers/SQLiteProviderTest.ts index acc2e1d63..4d82b2e31 100644 --- a/tests/unit/storage/providers/SQLiteProviderTest.ts +++ b/tests/unit/storage/providers/SQLiteProviderTest.ts @@ -329,6 +329,99 @@ describe('SQLiteProvider', () => { }); }); + describe('query splitting', () => { + const CHUNK_SIZE = 2; + const originalChunkArray = utils.chunkArray; + + const createKeyValueEntries = (count: number): Array<[string, unknown]> => Array.from({length: count}, (_, index) => [`chunk_key_${index}`, index]); + + beforeEach(() => { + resetAllDatabases(); + SQLiteProvider.init(); + + jest.spyOn(utils, 'chunkArray').mockImplementation((items, _maxChunkSize) => originalChunkArray(items, CHUNK_SIZE)); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe('multiGet', () => { + it('should return all values when keys exceed MAX_VARIABLE_NUMBER', async () => { + const entries = createKeyValueEntries(5); + await SQLiteProvider.multiSet(entries); + + const keys = entries.map(([key]) => key); + const result = await SQLiteProvider.multiGet(keys); + + expect(result).toEqual(expect.arrayContaining(entries)); + expect(result).toHaveLength(entries.length); + }); + + it('should issue one IN query per chunk', async () => { + const entries = createKeyValueEntries(5); + await SQLiteProvider.multiSet(entries); + + const executeAsyncSpy = jest.spyOn(SQLiteProvider.store!, 'executeAsync'); + executeAsyncSpy.mockClear(); + + const keys = entries.map(([key]) => key); + await SQLiteProvider.multiGet(keys); + + const inQueries = executeAsyncSpy.mock.calls.filter(([sql]) => typeof sql === 'string' && sql.includes('WHERE record_key IN')); + expect(inQueries).toHaveLength(3); + }); + }); + + describe('removeItems', () => { + it('should remove all keys when keys exceed MAX_VARIABLE_NUMBER', async () => { + const entries = createKeyValueEntries(5); + await SQLiteProvider.multiSet(entries); + + const keys = entries.map(([key]) => key); + await SQLiteProvider.removeItems(keys); + + expect(await SQLiteProvider.getAllKeys()).toEqual([]); + }); + + it('should use executeAsync when keys fit in a single chunk', async () => { + const entries = createKeyValueEntries(2); + await SQLiteProvider.multiSet(entries); + + const executeAsyncSpy = jest.spyOn(SQLiteProvider.store!, 'executeAsync'); + const executeBatchAsyncSpy = jest.spyOn(SQLiteProvider.store!, 'executeBatchAsync'); + executeAsyncSpy.mockClear(); + executeBatchAsyncSpy.mockClear(); + + const keys = entries.map(([key]) => key); + await SQLiteProvider.removeItems(keys); + + expect(executeAsyncSpy).toHaveBeenCalledTimes(1); + expect(executeBatchAsyncSpy).not.toHaveBeenCalled(); + }); + + it('should use executeBatchAsync when keys span multiple chunks', async () => { + const entries = createKeyValueEntries(5); + await SQLiteProvider.multiSet(entries); + + const executeAsyncSpy = jest.spyOn(SQLiteProvider.store!, 'executeAsync'); + const executeBatchAsyncSpy = jest.spyOn(SQLiteProvider.store!, 'executeBatchAsync'); + executeAsyncSpy.mockClear(); + executeBatchAsyncSpy.mockClear(); + + const keys = entries.map(([key]) => key); + await SQLiteProvider.removeItems(keys); + + expect(executeAsyncSpy).not.toHaveBeenCalled(); + expect(executeBatchAsyncSpy).toHaveBeenCalledTimes(1); + + const batchCommands = executeBatchAsyncSpy.mock.calls[0][0]; + expect(batchCommands).toHaveLength(3); + expect(batchCommands.every((command) => command.query.includes('DELETE FROM keyvaluepairs WHERE record_key IN'))).toBe(true); + }); + }); + }); + // SQLite-specific: the IN-list is parameterised, so a key containing SQL // fragments must be treated as a literal record_key. describe('SQL-injection safety', () => { diff --git a/tests/unit/utilsTest.ts b/tests/unit/utilsTest.ts index 48e51d598..6d25dabaf 100644 --- a/tests/unit/utilsTest.ts +++ b/tests/unit/utilsTest.ts @@ -390,6 +390,53 @@ describe('utils', () => { }); }); + describe('chunkArray', () => { + it('should return an empty array when given an empty array', () => { + expect(utils.chunkArray([], 3)).toEqual([]); + }); + + it('should return a single chunk when the array length is less than maxChunkSize', () => { + const items = [1, 2]; + const result = utils.chunkArray(items, 3); + + expect(result).toEqual([[1, 2]]); + expect(result[0]).toBe(items); + }); + + it('should return a single chunk when the array length equals maxChunkSize', () => { + const items = [1, 2, 3]; + const result = utils.chunkArray(items, 3); + + expect(result).toEqual([[1, 2, 3]]); + expect(result[0]).toBe(items); + }); + + it('should split the array into evenly sized chunks', () => { + expect(utils.chunkArray([1, 2, 3, 4, 5, 6], 3)).toEqual([ + [1, 2, 3], + [4, 5, 6], + ]); + }); + + it('should include a smaller final chunk when the array length is not divisible by maxChunkSize', () => { + expect(utils.chunkArray([1, 2, 3, 4, 5], 2)).toEqual([[1, 2], [3, 4], [5]]); + }); + + it('should create one item per chunk when maxChunkSize is 1', () => { + expect(utils.chunkArray(['a', 'b', 'c'], 1)).toEqual([['a'], ['b'], ['c']]); + }); + + it('should work with readonly arrays and preserve element types', () => { + const items = Object.freeze(['x', 'y', 'z', 'w']) as readonly string[]; + const result = utils.chunkArray(items, 2); + + expect(result).toEqual([ + ['x', 'y'], + ['z', 'w'], + ]); + }); + }); + describe('isEmptyObject', () => { it('should return true for an empty object', () => { expect(utils.isEmptyObject({})).toBe(true); From c1a8c774d69cfc8961ba69c6a80eec9e1bcd9656 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Mon, 27 Jul 2026 09:11:27 +0200 Subject: [PATCH 16/17] clean up comments --- API.md | 2 -- lib/Onyx.ts | 6 ------ 2 files changed, 8 deletions(-) diff --git a/API.md b/API.md index 6e0237151..e766e551f 100644 --- a/API.md +++ b/API.md @@ -81,7 +81,6 @@ This method will be deprecated soon. Please use `Onyx.connectWithoutView()` inst | connectOptions.key | The Onyx key to subscribe to. | | connectOptions.callback | A function that will be called when the Onyx data we are subscribed changes. | | connectOptions.selector | This will be used to subscribe to a subset of an Onyx key's data. **Only used inside `useOnyx()` hook.** Using this setting on `useOnyx()` can have very positive performance benefits because the component will only re-render when the subset of data changes. Otherwise, any change of data on any property would normally cause the component to re-render (and that can be expensive from a performance standpoint). | -| connectOptions.subscribed | Defaults to `true`. **Only used inside `useOnyx()` hook.** When `false`, keeps the connection open (value stays cache-warm) but stops re-rendering on background writes. It defers the render trigger, not the value: any other render still reads the latest value, and flipping back to `true` re-renders. | **Example** ```ts @@ -104,7 +103,6 @@ Connects to an Onyx key given the options passed and listens to its changes. | connectOptions.key | The Onyx key to subscribe to. | | connectOptions.callback | A function that will be called when the Onyx data we are subscribed changes. | | connectOptions.selector | This will be used to subscribe to a subset of an Onyx key's data. **Only used inside `useOnyx()` hook.** Using this setting on `useOnyx()` can have very positive performance benefits because the component will only re-render when the subset of data changes. Otherwise, any change of data on any property would normally cause the component to re-render (and that can be expensive from a performance standpoint). | -| connectOptions.subscribed | Defaults to `true`. **Only used inside `useOnyx()` hook.** When `false`, keeps the connection open (value stays cache-warm) but stops re-rendering on background writes. It defers the render trigger, not the value: any other render still reads the latest value, and flipping back to `true` re-renders. | **Example** ```ts diff --git a/lib/Onyx.ts b/lib/Onyx.ts index ffaf33b30..16a0f6ec6 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -98,9 +98,6 @@ function init({ * Using this setting on `useOnyx()` can have very positive performance benefits because the component will only re-render * when the subset of data changes. Otherwise, any change of data on any property would normally * cause the component to re-render (and that can be expensive from a performance standpoint). - * @param connectOptions.subscribed Defaults to `true`. **Only used inside `useOnyx()` hook.** When `false`, keeps the connection open - * (value stays cache-warm) but stops re-rendering on background writes. It defers the render trigger, not the value: any other - * render still reads the latest value, and flipping back to `true` re-renders. * @returns The connection object to use when calling `Onyx.disconnect()`. */ function connect(connectOptions: ConnectOptions): Connection { @@ -125,9 +122,6 @@ function connect(connectOptions: ConnectOptions): Co * Using this setting on `useOnyx()` can have very positive performance benefits because the component will only re-render * when the subset of data changes. Otherwise, any change of data on any property would normally * cause the component to re-render (and that can be expensive from a performance standpoint). - * @param connectOptions.subscribed Defaults to `true`. **Only used inside `useOnyx()` hook.** When `false`, keeps the connection open - * (value stays cache-warm) but stops re-rendering on background writes. It defers the render trigger, not the value: any other - * render still reads the latest value, and flipping back to `true` re-renders. * @returns The connection object to use when calling `Onyx.disconnect()`. */ function connectWithoutView(connectOptions: ConnectOptions): Connection { From e244efc936337d26d34e0e3c65eeefe6af336f07 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Mon, 27 Jul 2026 09:40:37 +0200 Subject: [PATCH 17/17] add more units tests --- tests/unit/useOnyxTest.ts | 99 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 1b0d32ead..5d2fcd133 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -1558,6 +1558,105 @@ describe('useOnyx', () => { expect(paused.result.current[0]).toEqual('v1'); }); + // A key change is consumer-driven (like a dependencies change), so subscribed: false must not block it: + // the hook re-subscribes and treats the new key as a first connection, which is exempt from the gate. + it('loads the new key value on a key change while subscribed is false, then keeps gating background writes', async () => { + await Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}1000`, 'report_1000_value'); + await Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}1001`, 'report_1001_value'); + + type KeyProps = {key: string}; + const {result, rerender} = renderHook(({key}: KeyProps) => useOnyx(key, {subscribed: false}), { + initialProps: {key: `${ONYXKEYS.COLLECTION.TEST_KEY}1000`} as KeyProps, + }); + + await act(async () => waitForPromisesToResolve()); + expect(result.current[0]).toEqual('report_1000_value'); + expect(result.current[1].status).toEqual('loaded'); + + // Switch to another collection member while paused + await act(async () => { + rerender({key: `${ONYXKEYS.COLLECTION.TEST_KEY}1001`}); + await waitForPromisesToResolve(); + }); + + expect(result.current[0]).toEqual('report_1001_value'); + expect(result.current[1].status).toEqual('loaded'); + + // Still paused on the new key: background writes remain suppressed + await act(async () => { + Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}1001`, 'report_1001_updated'); + await waitForPromisesToResolve(); + }); + expect(result.current[0]).toEqual('report_1001_value'); + + // And writes to the old key are ignored entirely + await act(async () => { + Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}1000`, 'report_1000_updated'); + await waitForPromisesToResolve(); + }); + expect(result.current[0]).toEqual('report_1001_value'); + }); + + // Unlike the warm switch above, a cold member has no cached value to read during the switch render, + // so resolving it depends on the connect callback's initial-load exemption firing after the storage read. + it('resolves a cold key through loading on a key change while subscribed is false', async () => { + await StorageMock.setItem(`${ONYXKEYS.COLLECTION.TEST_KEY}1000`, {id: 1000}); + await StorageMock.setItem(`${ONYXKEYS.COLLECTION.TEST_KEY}1001`, {id: 1001}); + + type KeyProps = {key: string}; + const {result, rerender} = renderHook(({key}: KeyProps) => useOnyx(key, {subscribed: false}), { + initialProps: {key: `${ONYXKEYS.COLLECTION.TEST_KEY}1000`} as KeyProps, + }); + + await act(async () => waitForPromisesToResolve()); + expect(result.current[0]).toEqual({id: 1000}); + + // Switch to a member that only exists in storage + rerender({key: `${ONYXKEYS.COLLECTION.TEST_KEY}1001`}); + expect(result.current[1].status).toEqual('loading'); + + await act(async () => waitForPromisesToResolve()); + expect(result.current[0]).toEqual({id: 1001}); + expect(result.current[1].status).toEqual('loaded'); + }); + + // The pause freezes per subscription, not per key: writes suppressed while paused must surface + // when the consumer switches away and back, since each key change is a fresh first connection. + it('shows values written while paused after switching away and back to the key', async () => { + await Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}1000`, 'report_1000_value'); + await Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}1001`, 'report_1001_value'); + + type KeyProps = {key: string}; + const {result, rerender} = renderHook(({key}: KeyProps) => useOnyx(key, {subscribed: false}), { + initialProps: {key: `${ONYXKEYS.COLLECTION.TEST_KEY}1000`} as KeyProps, + }); + + await act(async () => waitForPromisesToResolve()); + expect(result.current[0]).toEqual('report_1000_value'); + + // While paused: a write to the connected key is suppressed, a write to the future key lands unconnected + await act(async () => { + Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}1000`, 'report_1000_updated'); + Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}1001`, 'report_1001_updated'); + await waitForPromisesToResolve(); + }); + expect(result.current[0]).toEqual('report_1000_value'); + + // Switching to the other member shows its latest value, regardless of write timing + await act(async () => { + rerender({key: `${ONYXKEYS.COLLECTION.TEST_KEY}1001`}); + await waitForPromisesToResolve(); + }); + expect(result.current[0]).toEqual('report_1001_updated'); + + // Switching back shows the write that was suppressed while paused + await act(async () => { + rerender({key: `${ONYXKEYS.COLLECTION.TEST_KEY}1000`}); + await waitForPromisesToResolve(); + }); + expect(result.current[0]).toEqual('report_1000_updated'); + }); + // A selector-identity change while paused must be frozen like any other update: the paused fast path // returns the last delivered output instead of recomputing. On resubscribe the new selector applies it('freezes a selector change while paused and applies it on resubscribe', async () => {