Skip to content

Commit 224e750

Browse files
fabioh8010fabioh8010
committed
Remove canBeMissing option from Onyx
1 parent e5b57d7 commit 224e750

6 files changed

Lines changed: 4 additions & 201 deletions

File tree

‎README.md‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -206,21 +206,6 @@ export default App;
206206

207207
It's also beneficial to use a [selector](https://github.com/Expensify/react-native-onyx/blob/main/API.md#connectmapping--number) with the mapping in case you need to grab a single item in a collection (like a single report action).
208208

209-
### useOnyx()'s `canBeMissing` option
210-
211-
You must pass the `canBeMissing` configuration flag to `useOnyx` if you want the hook to log an alert when data is missing from Onyx store. Regarding usage in `Expensify/App` repo, if the component calling this is the one loading the data by calling an action, then you should set this to `true`. If the component calling this does not load the data then you should set it to `false`, which means that if the data is not there, it will log an alert, as it means we are using data that no one loaded and that's most probably a bug.
212-
213-
```javascript
214-
const Component = ({reportID}) => {
215-
// This hook will log an alert (via `Logger.logAlert()`) if `report` is `undefined`.
216-
const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {canBeMissing: false});
217-
218-
// rest of the component's code.
219-
};
220-
221-
export default Component;
222-
```
223-
224209
## Collections
225210

226211
Collections allow keys with similar value types to be subscribed together by subscribing to the collection key. To define one, it must be included in the `ONYXKEYS.COLLECTION` object and it must be suffixed with an underscore. Member keys should use a unique identifier or index after the collection key prefix (e.g. `report_42`).

‎lib/OnyxSnapshotCache.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,19 +60,17 @@ class OnyxSnapshotCache {
6060
* - `selector`: Different selectors produce different results, so each selector needs its own cache entry
6161
* - `initWithStoredValues`: This flag changes the initial loading behavior and affects the returned fetch status
6262
* - `allowStaleData`: Controls whether stale data can be returned during pending merges, affecting result timing
63-
* - `canBeMissing`: Determines logging behavior for missing data, but doesn't affect the actual data returned
6463
*
6564
* Other options like `canEvict`, `reuseConnection`, and `allowDynamicKey` don't affect the data transformation
6665
* or timing behavior of getSnapshot, so they're excluded from the cache key for better cache hit rates.
6766
*/
68-
registerConsumer<TKey extends OnyxKey, TReturnValue>(options: Pick<UseOnyxOptions<TKey, TReturnValue>, 'selector' | 'initWithStoredValues' | 'allowStaleData' | 'canBeMissing'>): string {
67+
registerConsumer<TKey extends OnyxKey, TReturnValue>(options: Pick<UseOnyxOptions<TKey, TReturnValue>, 'selector' | 'initWithStoredValues' | 'allowStaleData'>): string {
6968
const selectorID = options?.selector ? this.getSelectorID(options.selector) : 'no_selector';
7069

7170
// Create options hash without expensive JSON.stringify
7271
const initWithStoredValues = options?.initWithStoredValues ?? true;
7372
const allowStaleData = options?.allowStaleData ?? false;
74-
const canBeMissing = options?.canBeMissing ?? true;
75-
const cacheKey = `${selectorID}_${initWithStoredValues}_${allowStaleData}_${canBeMissing}`;
73+
const cacheKey = `${selectorID}_${initWithStoredValues}_${allowStaleData}`;
7674

7775
// Increment reference count for this cache key
7876
const currentCount = this.cacheKeyRefCounts.get(cacheKey) || 0;

‎lib/useOnyx.ts‎

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import * as GlobalSettings from './GlobalSettings';
99
import type {CollectionKeyBase, OnyxKey, OnyxValue} from './types';
1010
import usePrevious from './usePrevious';
1111
import decorateWithMetrics from './metrics';
12-
import * as Logger from './Logger';
1312
import onyxSnapshotCache from './OnyxSnapshotCache';
1413
import useLiveRef from './useLiveRef';
1514

@@ -43,14 +42,6 @@ type UseOnyxOptions<TKey extends OnyxKey, TReturnValue> = {
4342
*/
4443
allowDynamicKey?: boolean;
4544

46-
/**
47-
* If the component calling this is the one loading the data by calling an action, then you should set this to `true`.
48-
*
49-
* If the component calling this does not load the data then you should set it to `false`, which means that if the data
50-
* is not there, it will log an alert, as it means we are using data that no one loaded and that's most probably a bug.
51-
*/
52-
canBeMissing?: boolean;
53-
5445
/**
5546
* This will be used to subscribe to a subset of an Onyx key's data.
5647
* Using this setting on `useOnyx` can have very positive performance benefits because the component will only re-render
@@ -156,9 +147,8 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
156147
selector: options?.selector,
157148
initWithStoredValues: options?.initWithStoredValues,
158149
allowStaleData: options?.allowStaleData,
159-
canBeMissing: options?.canBeMissing,
160150
}),
161-
[options?.selector, options?.initWithStoredValues, options?.allowStaleData, options?.canBeMissing],
151+
[options?.selector, options?.initWithStoredValues, options?.allowStaleData],
162152
);
163153

164154
useEffect(() => () => onyxSnapshotCache.deregisterConsumer(key, cacheKey), [key, cacheKey]);
@@ -242,8 +232,6 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
242232
}
243233
}
244234

245-
let isOnyxValueDefined = true;
246-
247235
// We return the initial result right away during the first connection if `initWithStoredValues` is set to `false`.
248236
if (isFirstConnectionRef.current && options?.initWithStoredValues === false) {
249237
const result = resultRef.current;
@@ -262,10 +250,6 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
262250
const selectedValue = memoizedSelector ? memoizedSelector(value) : value;
263251
newValueRef.current = (selectedValue ?? undefined) as TReturnValue | undefined;
264252

265-
// This flag is `false` when the original Onyx value (without selector) is not defined yet.
266-
// It will be used later to check if we need to log an alert that the value is missing.
267-
isOnyxValueDefined = value !== null && value !== undefined;
268-
269253
// We set this flag to `false` again since we don't want to get the newest cached value every time `getSnapshot()` is executed,
270254
// and only when `Onyx.connect()` callback is fired.
271255
shouldGetCachedValueRef.current = false;
@@ -317,21 +301,14 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
317301
sourceValue: sourceValueRef.current,
318302
},
319303
];
320-
321-
// If `canBeMissing` is set to `false` and the Onyx value of that key is not defined,
322-
// we log an alert so it can be acknowledged by the consumer. Additionally, we won't log alerts
323-
// if there's a `Onyx.clear()` task in progress.
324-
if (options?.canBeMissing === false && newFetchStatus === 'loaded' && !isOnyxValueDefined && !OnyxCache.hasPendingTask(TASK.CLEAR)) {
325-
Logger.logAlert(`useOnyx returned no data for key with canBeMissing set to false for key ${key}`, {showAlert: true});
326-
}
327304
}
328305

329306
if (newFetchStatus !== 'loading') {
330307
onyxSnapshotCache.setCachedResult<UseOnyxResult<TReturnValue>>(key, cacheKey, resultRef.current);
331308
}
332309

333310
return resultRef.current;
334-
}, [options?.initWithStoredValues, options?.allowStaleData, options?.canBeMissing, key, memoizedSelector, cacheKey, previousKey]);
311+
}, [options?.initWithStoredValues, options?.allowStaleData, key, memoizedSelector, cacheKey, previousKey]);
335312

336313
const subscribe = useCallback(
337314
(onStoreChange: () => void) => {

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,12 @@ const selectorOptions: UseOnyxOptions<string, number | undefined> = {
3939
selector: simpleSelector,
4040
initWithStoredValues: true,
4141
allowStaleData: false,
42-
canBeMissing: true,
4342
};
4443

4544
const complexSelectorOptions: UseOnyxOptions<string, ComplexSelectorResult> = {
4645
selector: complexSelector,
4746
initWithStoredValues: true,
4847
allowStaleData: false,
49-
canBeMissing: true,
5048
};
5149

5250
// Mock results

‎tests/unit/OnyxSnapshotCacheTest.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,10 @@ describe('OnyxSnapshotCache', () => {
4040
selector,
4141
initWithStoredValues: true,
4242
allowStaleData: false,
43-
canBeMissing: true,
4443
};
4544
const optionsWithoutSelector: UseOnyxOptions<OnyxKey, string> = {
4645
initWithStoredValues: false,
4746
allowStaleData: true,
48-
canBeMissing: false,
4947
};
5048
const keyWithSelector = cache.registerConsumer(optionsWithSelector);
5149
const keyWithoutSelector = cache.registerConsumer(optionsWithoutSelector);

‎tests/unit/useOnyxTest.ts‎

Lines changed: 0 additions & 153 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import OnyxCache from '../../lib/OnyxCache';
55
import StorageMock from '../../lib/storage';
66
import type GenericCollection from '../utils/GenericCollection';
77
import waitForPromisesToResolve from '../utils/waitForPromisesToResolve';
8-
import * as Logger from '../../lib/Logger';
98
import onyxSnapshotCache from '../../lib/OnyxSnapshotCache';
109
import type {UseOnyxSelector} from '../../lib/useOnyx';
1110

@@ -1019,158 +1018,6 @@ describe('useOnyx', () => {
10191018
});
10201019
});
10211020

1022-
describe('canBeMissing', () => {
1023-
let logAlertFn = jest.fn();
1024-
const alertMessage = 'useOnyx returned no data for key with canBeMissing set to false for key test';
1025-
1026-
beforeEach(() => {
1027-
logAlertFn = jest.fn();
1028-
jest.spyOn(Logger, 'logAlert').mockImplementation(logAlertFn);
1029-
});
1030-
1031-
afterEach(() => {
1032-
(Logger.logAlert as unknown as jest.SpyInstance<void, Parameters<typeof Logger.logAlert>>).mockRestore();
1033-
});
1034-
1035-
it('should not log an alert if Onyx doesn\'t return data in loaded state and "canBeMissing" property is not provided', async () => {
1036-
const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY));
1037-
1038-
expect(result1.current[0]).toBeUndefined();
1039-
expect(result1.current[1].status).toEqual('loading');
1040-
1041-
await act(async () => waitForPromisesToResolve());
1042-
1043-
expect(result1.current[0]).toBeUndefined();
1044-
expect(result1.current[1].status).toEqual('loaded');
1045-
expect(logAlertFn).not.toBeCalled();
1046-
1047-
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test'));
1048-
1049-
expect(result1.current[0]).toBe('test');
1050-
1051-
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, null));
1052-
1053-
expect(result1.current[0]).toBeUndefined();
1054-
expect(logAlertFn).not.toBeCalled();
1055-
});
1056-
1057-
it('should not log an alert if Onyx doesn\'t return data, "canBeMissing" property is false but "initWithStoredValues" is also false', async () => {
1058-
const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY, {canBeMissing: false, initWithStoredValues: false}));
1059-
1060-
expect(result1.current[0]).toBeUndefined();
1061-
expect(result1.current[1].status).toEqual('loaded');
1062-
1063-
expect(logAlertFn).not.toBeCalled();
1064-
});
1065-
1066-
it('should log an alert if Onyx doesn\'t return data in loaded state and "canBeMissing" property is false', async () => {
1067-
const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY, {canBeMissing: false}));
1068-
1069-
expect(result1.current[0]).toBeUndefined();
1070-
expect(result1.current[1].status).toEqual('loading');
1071-
expect(logAlertFn).not.toBeCalled();
1072-
1073-
await act(async () => waitForPromisesToResolve());
1074-
1075-
expect(result1.current[0]).toBeUndefined();
1076-
expect(result1.current[1].status).toEqual('loaded');
1077-
expect(logAlertFn).toHaveBeenCalledTimes(1);
1078-
expect(logAlertFn).toHaveBeenNthCalledWith(1, alertMessage, {showAlert: true});
1079-
1080-
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test'));
1081-
1082-
expect(result1.current[0]).toBe('test');
1083-
1084-
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, null));
1085-
1086-
expect(result1.current[0]).toBeUndefined();
1087-
expect(logAlertFn).toHaveBeenCalledTimes(2);
1088-
expect(logAlertFn).toHaveBeenNthCalledWith(2, alertMessage, {showAlert: true});
1089-
});
1090-
1091-
it('should log an alert if Onyx doesn\'t return selected data in loaded state and "canBeMissing" property is false', async () => {
1092-
const {result: result1} = renderHook(() =>
1093-
useOnyx(ONYXKEYS.TEST_KEY, {
1094-
selector: ((entry: OnyxEntry<string>) => (entry ? `${entry}_changed` : undefined)) as UseOnyxSelector<OnyxKey, string | undefined>,
1095-
canBeMissing: false,
1096-
}),
1097-
);
1098-
1099-
expect(result1.current[0]).toBeUndefined();
1100-
expect(result1.current[1].status).toEqual('loading');
1101-
expect(logAlertFn).not.toBeCalled();
1102-
1103-
await act(async () => waitForPromisesToResolve());
1104-
1105-
expect(result1.current[0]).toBeUndefined();
1106-
expect(result1.current[1].status).toEqual('loaded');
1107-
expect(logAlertFn).toHaveBeenCalledTimes(1);
1108-
expect(logAlertFn).toHaveBeenNthCalledWith(1, alertMessage, {showAlert: true});
1109-
1110-
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test'));
1111-
1112-
expect(result1.current[0]).toBe('test_changed');
1113-
1114-
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, null));
1115-
1116-
expect(result1.current[0]).toBeUndefined();
1117-
expect(logAlertFn).toHaveBeenCalledTimes(2);
1118-
expect(logAlertFn).toHaveBeenNthCalledWith(2, alertMessage, {showAlert: true});
1119-
});
1120-
1121-
it('should log an alert if Onyx doesn\'t return data but there is a selector that always return something and "canBeMissing" property is false', async () => {
1122-
const {result: result1} = renderHook(() =>
1123-
useOnyx(ONYXKEYS.TEST_KEY, {
1124-
// This selector will always return a value, even if the Onyx data is missing.
1125-
selector: ((entry: OnyxEntry<string>) => `${entry}_changed`) as UseOnyxSelector<OnyxKey, string>,
1126-
canBeMissing: false,
1127-
}),
1128-
);
1129-
1130-
await act(async () => waitForPromisesToResolve());
1131-
1132-
expect(result1.current[0]).toBe('undefined_changed');
1133-
expect(result1.current[1].status).toEqual('loaded');
1134-
expect(logAlertFn).toHaveBeenCalledTimes(1);
1135-
expect(logAlertFn).toHaveBeenNthCalledWith(1, alertMessage, {showAlert: true});
1136-
1137-
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test'));
1138-
1139-
expect(result1.current[0]).toBe('test_changed');
1140-
1141-
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, null));
1142-
1143-
expect(result1.current[0]).toBe('undefined_changed');
1144-
expect(logAlertFn).toHaveBeenCalledTimes(2);
1145-
expect(logAlertFn).toHaveBeenNthCalledWith(2, alertMessage, {showAlert: true});
1146-
});
1147-
1148-
it('should not log an alert if "canBeMissing" property is false but there is a Onyx.clear() task in progress', async () => {
1149-
const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY, {canBeMissing: false}));
1150-
1151-
expect(result1.current[0]).toBeUndefined();
1152-
expect(result1.current[1].status).toEqual('loading');
1153-
expect(logAlertFn).not.toBeCalled();
1154-
1155-
await act(async () => waitForPromisesToResolve());
1156-
1157-
expect(result1.current[0]).toBeUndefined();
1158-
expect(result1.current[1].status).toEqual('loaded');
1159-
expect(logAlertFn).toHaveBeenCalledTimes(1);
1160-
expect(logAlertFn).toHaveBeenNthCalledWith(1, alertMessage, {showAlert: true});
1161-
1162-
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test'));
1163-
1164-
expect(result1.current[0]).toBe('test');
1165-
1166-
logAlertFn.mockReset();
1167-
await act(async () => Onyx.clear());
1168-
1169-
expect(result1.current[0]).toBeUndefined();
1170-
expect(logAlertFn).not.toBeCalled();
1171-
});
1172-
});
1173-
11741021
// This test suite must be the last one to avoid problems when running the other tests here.
11751022
describe('canEvict', () => {
11761023
const error = (key: string) => `canEvict can't be used on key '${key}'. This key must explicitly be flagged as safe for removal by adding it to Onyx.init({evictableKeys: []}).`;

0 commit comments

Comments
 (0)