Skip to content

Commit 9f09be3

Browse files
cristipavalcristipaval
authored
Merge pull request #756 from callstack-internal/JKobrynski/fix/85416-fix-useonyx-state-not-resetting
[Onyx Audit] Reset internal useOnyx state on key change
2 parents 999e63f + 3f3500d commit 9f09be3

3 files changed

Lines changed: 259 additions & 33 deletions

File tree

‎lib/useOnyx.ts‎

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import connectionManager from './OnyxConnectionManager';
77
import OnyxUtils from './OnyxUtils';
88
import OnyxKeys from './OnyxKeys';
99
import type {CollectionKeyBase, OnyxKey, OnyxValue} from './types';
10-
import usePrevious from './usePrevious';
1110
import onyxSnapshotCache from './OnyxSnapshotCache';
1211
import useLiveRef from './useLiveRef';
1312

@@ -56,8 +55,6 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
5655
dependencies: DependencyList = [],
5756
): UseOnyxResult<TReturnValue> {
5857
const connectionRef = useRef<Connection | null>(null);
59-
const previousKey = usePrevious(key);
60-
6158
const currentDependenciesRef = useLiveRef(dependencies);
6259
const selector = options?.selector;
6360

@@ -113,9 +110,12 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
113110
},
114111
]);
115112

116-
// Indicates if it's the first Onyx connection of this hook or not, as we don't want certain use cases
117-
// in `getSnapshot()` to be satisfied several times.
118-
const isFirstConnectionRef = useRef(true);
113+
// Tracks which key has completed its first Onyx connection callback. When this doesn't match the
114+
// current key, getSnapshot() treats the hook as being in its "first connection" state for that key.
115+
// This is key-aware by design: when the key changes, connectedKeyRef still holds the old key (or null
116+
// after cleanup), so the hook automatically enters first-connection mode for the new key without any
117+
// explicit reset logic — eliminating the race condition where cleanup could clobber a boolean flag.
118+
const connectedKeyRef = useRef<OnyxKey | null>(null);
119119

120120
// Indicates if the hook is connecting to an Onyx key.
121121
const isConnectingRef = useRef(false);
@@ -157,7 +157,7 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
157157

158158
previousDependenciesRef.current = dependencies;
159159

160-
if (connectionRef.current === null || isConnectingRef.current || !onStoreChangeFnRef.current) {
160+
if (connectionRef.current === null || isConnectingRef.current || connectedKeyRef.current !== key || !onStoreChangeFnRef.current) {
161161
return;
162162
}
163163

@@ -193,7 +193,8 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
193193
// Check if we have any cache for this Onyx key
194194
// Don't use cache for first connection with initWithStoredValues: false
195195
// Also don't use cache during active data updates (when shouldGetCachedValueRef is true)
196-
if (!(isFirstConnectionRef.current && options?.initWithStoredValues === false) && !shouldGetCachedValueRef.current) {
196+
const isFirstConnection = connectedKeyRef.current !== key;
197+
if (!(isFirstConnection && options?.initWithStoredValues === false) && !shouldGetCachedValueRef.current) {
197198
const cachedResult = onyxSnapshotCache.getCachedResult<UseOnyxResult<TReturnValue>>(key, cacheKey);
198199
if (cachedResult !== undefined) {
199200
resultRef.current = cachedResult;
@@ -202,7 +203,7 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
202203
}
203204

204205
// We return the initial result right away during the first connection if `initWithStoredValues` is set to `false`.
205-
if (isFirstConnectionRef.current && options?.initWithStoredValues === false) {
206+
if (isFirstConnection && options?.initWithStoredValues === false) {
206207
const result = resultRef.current;
207208

208209
// Store result in snapshot cache
@@ -214,7 +215,7 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
214215
// 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
215216
// update `newValueRef` when `Onyx.connect()` callback is fired.
216217
const hasSelectorChanged = lastComputedSelectorRef.current !== memoizedSelector;
217-
if (isFirstConnectionRef.current || shouldGetCachedValueRef.current || key !== previousKey || hasSelectorChanged) {
218+
if (isFirstConnection || shouldGetCachedValueRef.current || hasSelectorChanged) {
218219
// Gets the value from cache and maps it with selector. It changes `null` to `undefined` for `useOnyx` compatibility.
219220
const value = OnyxUtils.tryGetCachedValue(key) as OnyxValue<TKey>;
220221
const selectedValue = memoizedSelector ? memoizedSelector(value) : value;
@@ -233,7 +234,7 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
233234

234235
// If we have pending merge operations for the key during the first connection, we set the new value to `undefined`
235236
// and fetch status to `loading` to simulate that it is still being loaded until we have the most updated data.
236-
if (isFirstConnectionRef.current && OnyxUtils.hasPendingMergeForKey(key)) {
237+
if (isFirstConnection && OnyxUtils.hasPendingMergeForKey(key)) {
237238
newValueRef.current = undefined;
238239
newFetchStatus = 'loading';
239240
}
@@ -258,7 +259,7 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
258259
// OR we have a pending `Onyx.clear()` task (if `Onyx.clear()` is running cache might not be available anymore
259260
// OR the subscriber is triggered (the value is gotten from the storage)
260261
// so we update the cached value/result right away in order to prevent infinite loading state issues).
261-
const shouldUpdateResult = !areValuesEqual || (previousValueRef.current === null && (hasCacheForKey || OnyxCache.hasPendingTask(TASK.CLEAR) || !isFirstConnectionRef.current));
262+
const shouldUpdateResult = !areValuesEqual || (previousValueRef.current === null && (hasCacheForKey || OnyxCache.hasPendingTask(TASK.CLEAR) || !isFirstConnection));
262263
if (shouldUpdateResult) {
263264
previousValueRef.current = newValueRef.current;
264265

@@ -278,10 +279,18 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
278279
}
279280

280281
return resultRef.current;
281-
}, [options?.initWithStoredValues, key, memoizedSelector, cacheKey, previousKey]);
282+
}, [options?.initWithStoredValues, key, memoizedSelector, cacheKey]);
282283

283284
const subscribe = useCallback(
284285
(onStoreChange: () => void) => {
286+
// Reset internal state so the hook properly transitions through loading
287+
// for the new key instead of preserving stale state from the previous one.
288+
previousValueRef.current = null;
289+
newValueRef.current = null;
290+
shouldGetCachedValueRef.current = true;
291+
sourceValueRef.current = undefined;
292+
resultRef.current = [undefined, {status: options?.initWithStoredValues === false ? 'loaded' : 'loading'}];
293+
285294
isConnectingRef.current = true;
286295
onStoreChangeFnRef.current = onStoreChange;
287296

@@ -291,9 +300,9 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
291300
isConnectingRef.current = false;
292301
onStoreChangeFnRef.current = onStoreChange;
293302

294-
// Signals that the first connection was made, so some logics in `getSnapshot()`
295-
// won't be executed anymore.
296-
isFirstConnectionRef.current = false;
303+
// Signals that the first connection was made for this key, so some logics
304+
// in `getSnapshot()` won't be executed anymore.
305+
connectedKeyRef.current = key;
297306

298307
// Signals that we want to get the newest cached value again in `getSnapshot()`.
299308
shouldGetCachedValueRef.current = true;
@@ -320,7 +329,7 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
320329
}
321330

322331
connectionManager.disconnect(connectionRef.current);
323-
isFirstConnectionRef.current = false;
332+
connectedKeyRef.current = null;
324333
isConnectingRef.current = false;
325334
onStoreChangeFnRef.current = null;
326335
};

‎lib/usePrevious.ts‎

Lines changed: 0 additions & 16 deletions
This file was deleted.

‎tests/unit/useOnyxTest.ts‎

Lines changed: 233 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,212 @@ beforeEach(async () => {
3535
});
3636

3737
describe('useOnyx', () => {
38+
describe('dynamic key', () => {
39+
beforeEach(() => {
40+
jest.spyOn(console, 'error').mockImplementation(jest.fn);
41+
});
42+
43+
afterEach(() => {
44+
(console.error as unknown as jest.SpyInstance<void, Parameters<typeof console.error>>).mockRestore();
45+
});
46+
47+
it('should not throw any errors when changing from a collection member key to another one', async () => {
48+
const {rerender} = renderHook((key: string) => useOnyx(key), {initialProps: `${ONYXKEYS.COLLECTION.TEST_KEY}1` as string});
49+
50+
try {
51+
await act(async () => {
52+
rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}2`);
53+
});
54+
} catch (e) {
55+
fail('Expected to not throw any errors.');
56+
}
57+
});
58+
59+
it('should transition through loading when switching between collection member keys that both resolve to undefined', async () => {
60+
const {result, rerender} = renderHook((key: string) => useOnyx(key), {initialProps: `${ONYXKEYS.COLLECTION.TEST_KEY}1` as string});
61+
62+
// Wait for initial key to fully load
63+
await act(async () => waitForPromisesToResolve());
64+
65+
expect(result.current[0]).toBeUndefined();
66+
expect(result.current[1].status).toEqual('loaded');
67+
68+
// Switch to another collection member key that also has no data
69+
rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}2`);
70+
71+
expect(result.current[0]).toBeUndefined();
72+
expect(result.current[1].status).toEqual('loading');
73+
74+
await act(async () => waitForPromisesToResolve());
75+
76+
expect(result.current[0]).toBeUndefined();
77+
expect(result.current[1].status).toEqual('loaded');
78+
});
79+
80+
it('should return cached value immediately with loaded status when switching to a key that has data', async () => {
81+
Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}2`, 'test_value');
82+
83+
const {result, rerender} = renderHook((key: string) => useOnyx(key), {initialProps: `${ONYXKEYS.COLLECTION.TEST_KEY}1` as string});
84+
85+
await act(async () => waitForPromisesToResolve());
86+
87+
expect(result.current[0]).toBeUndefined();
88+
expect(result.current[1].status).toEqual('loaded');
89+
90+
// Switch to a key that has cached data
91+
rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}2`);
92+
93+
// Value and loaded status should be available synchronously, without waiting for promises
94+
expect(result.current[0]).toEqual('test_value');
95+
expect(result.current[1].status).toEqual('loaded');
96+
97+
await act(async () => waitForPromisesToResolve());
98+
99+
expect(result.current[0]).toEqual('test_value');
100+
expect(result.current[1].status).toEqual('loaded');
101+
});
102+
103+
it('should clear previous data and transition through loading when switching from a key with data to one without', async () => {
104+
Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}1`, 'initial_value');
105+
106+
const {result, rerender} = renderHook((key: string) => useOnyx(key), {initialProps: `${ONYXKEYS.COLLECTION.TEST_KEY}1` as string});
107+
108+
await act(async () => waitForPromisesToResolve());
109+
110+
expect(result.current[0]).toEqual('initial_value');
111+
expect(result.current[1].status).toEqual('loaded');
112+
113+
// Switch to a key that has no data
114+
rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}2`);
115+
116+
expect(result.current[0]).toBeUndefined();
117+
expect(result.current[1].status).toEqual('loading');
118+
119+
await act(async () => waitForPromisesToResolve());
120+
121+
expect(result.current[0]).toBeUndefined();
122+
expect(result.current[1].status).toEqual('loaded');
123+
});
124+
125+
it('should return the new value when switching from a key with data to another key with different data', async () => {
126+
Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}1`, 'value_one');
127+
Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}2`, 'value_two');
128+
129+
const {result, rerender} = renderHook((key: string) => useOnyx(key), {initialProps: `${ONYXKEYS.COLLECTION.TEST_KEY}1` as string});
130+
131+
await act(async () => waitForPromisesToResolve());
132+
133+
expect(result.current[0]).toEqual('value_one');
134+
expect(result.current[1].status).toEqual('loaded');
135+
136+
// Switch to another key that also has data
137+
rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}2`);
138+
139+
await act(async () => waitForPromisesToResolve());
140+
141+
expect(result.current[0]).toEqual('value_two');
142+
expect(result.current[1].status).toEqual('loaded');
143+
});
144+
145+
it('should apply the selector against the new key data when switching keys', async () => {
146+
Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}1`, {id: 'entry1_id', name: 'entry1_name'});
147+
Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}2`, {id: 'entry2_id', name: 'entry2_name'});
148+
149+
const selector = ((entry: OnyxEntry<{id: string; name: string}>) => entry?.name) as UseOnyxSelector<OnyxKey, string | undefined>;
150+
151+
const {result, rerender} = renderHook((key: string) => useOnyx(key, {selector}), {initialProps: `${ONYXKEYS.COLLECTION.TEST_KEY}1` as string});
152+
153+
await act(async () => waitForPromisesToResolve());
154+
155+
expect(result.current[0]).toEqual('entry1_name');
156+
expect(result.current[1].status).toEqual('loaded');
157+
158+
// Switch key — selector should run against the new key's data
159+
rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}2`);
160+
161+
await act(async () => waitForPromisesToResolve());
162+
163+
expect(result.current[0]).toEqual('entry2_name');
164+
expect(result.current[1].status).toEqual('loaded');
165+
});
166+
167+
it('should handle rapid key switching and settle on the final key value', async () => {
168+
Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}1`, 'value_one');
169+
Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}2`, 'value_two');
170+
Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}3`, 'value_three');
171+
172+
const {result, rerender} = renderHook((key: string) => useOnyx(key), {initialProps: `${ONYXKEYS.COLLECTION.TEST_KEY}1` as string});
173+
174+
await act(async () => waitForPromisesToResolve());
175+
176+
expect(result.current[0]).toEqual('value_one');
177+
178+
// Rapidly switch keys without waiting for promises between switches
179+
rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}2`);
180+
rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}3`);
181+
182+
await act(async () => waitForPromisesToResolve());
183+
184+
expect(result.current[0]).toEqual('value_three');
185+
expect(result.current[1].status).toEqual('loaded');
186+
});
187+
188+
it('should correctly switch between non-collection keys', async () => {
189+
Onyx.set(ONYXKEYS.TEST_KEY, 'value_one');
190+
Onyx.set(ONYXKEYS.TEST_KEY_2, 'value_two');
191+
192+
const {result, rerender} = renderHook((key: string) => useOnyx(key), {initialProps: ONYXKEYS.TEST_KEY as string});
193+
194+
await act(async () => waitForPromisesToResolve());
195+
196+
expect(result.current[0]).toEqual('value_one');
197+
expect(result.current[1].status).toEqual('loaded');
198+
199+
rerender(ONYXKEYS.TEST_KEY_2);
200+
201+
// Cached value should be available synchronously
202+
expect(result.current[0]).toEqual('value_two');
203+
expect(result.current[1].status).toEqual('loaded');
204+
205+
await act(async () => waitForPromisesToResolve());
206+
207+
expect(result.current[0]).toEqual('value_two');
208+
expect(result.current[1].status).toEqual('loaded');
209+
});
210+
211+
it('should correctly return to a previously used key (A → B → A round-trip)', async () => {
212+
Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}1`, 'value_one');
213+
Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}2`, 'value_two');
214+
215+
const {result, rerender} = renderHook((key: string) => useOnyx(key), {initialProps: `${ONYXKEYS.COLLECTION.TEST_KEY}1` as string});
216+
217+
await act(async () => waitForPromisesToResolve());
218+
219+
expect(result.current[0]).toEqual('value_one');
220+
expect(result.current[1].status).toEqual('loaded');
221+
222+
// A → B
223+
rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}2`);
224+
225+
await act(async () => waitForPromisesToResolve());
226+
227+
expect(result.current[0]).toEqual('value_two');
228+
expect(result.current[1].status).toEqual('loaded');
229+
230+
// B → A
231+
rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}1`);
232+
233+
// Cached value should be available synchronously
234+
expect(result.current[0]).toEqual('value_one');
235+
expect(result.current[1].status).toEqual('loaded');
236+
237+
await act(async () => waitForPromisesToResolve());
238+
239+
expect(result.current[0]).toEqual('value_one');
240+
expect(result.current[1].status).toEqual('loaded');
241+
});
242+
});
243+
38244
describe('misc', () => {
39245
it('should initially return loading state while loading non-existent key, and then return `undefined` and loaded state', async () => {
40246
const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY));
@@ -711,6 +917,33 @@ describe('useOnyx', () => {
711917
expect(result.current[0]).toEqual('test_selected');
712918
expect(result.current[1].status).toEqual('loaded');
713919
});
920+
921+
it('should suppress stored values for the new key when switching keys with initWithStoredValues: false', async () => {
922+
await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'stored_value_one');
923+
await StorageMock.setItem(ONYXKEYS.TEST_KEY_2, 'stored_value_two');
924+
925+
const {result, rerender} = renderHook((key: string) => useOnyx(key, {initWithStoredValues: false}), {initialProps: ONYXKEYS.TEST_KEY as string});
926+
927+
await act(async () => waitForPromisesToResolve());
928+
929+
// initWithStoredValues: false — stored value should be suppressed
930+
expect(result.current[0]).toBeUndefined();
931+
expect(result.current[1].status).toEqual('loaded');
932+
933+
rerender(ONYXKEYS.TEST_KEY_2);
934+
935+
await act(async () => waitForPromisesToResolve());
936+
937+
// Stored value for the new key should also be suppressed
938+
expect(result.current[0]).toBeUndefined();
939+
expect(result.current[1].status).toEqual('loaded');
940+
941+
// But live updates should still come through
942+
await act(async () => Onyx.merge(ONYXKEYS.TEST_KEY_2, 'live_value'));
943+
944+
expect(result.current[0]).toEqual('live_value');
945+
expect(result.current[1].status).toEqual('loaded');
946+
});
714947
});
715948

716949
describe('multiple usage', () => {

0 commit comments

Comments
 (0)