Add collection key operation alerts and update type handling in Onyx

Author: pasyukevich

lib/Onyx.ts

@@ -165,7 +165,11 @@ function disconnect(connection: Connection): void {
  * @param value value to store
  * @param options optional configuration object
  */
-function set<TKey extends OnyxKey>(key: TKey extends CollectionKeyBase ? never : TKey, value: OnyxSetInput<TKey>, options?: SetOptions): Promise<void> {
+function set<TKey extends OnyxKey>(key: string extends CollectionKeyBase ? TKey : TKey extends CollectionKeyBase ? never : TKey, value: OnyxSetInput<TKey>, options?: SetOptions): Promise<void> {
+    if (OnyxUtils.isCollectionKey(key as OnyxKey)) {
+        Logger.logInfo(logMessages.collectionKeyOperationAlert(key as string, 'set'));
+        return Promise.resolve();
+    }
     return OnyxUtils.afterInit(() => OnyxUtils.setWithRetry({key, value, options}));
 }
 
@@ -196,7 +200,11 @@ function multiSet(data: OnyxMultiSetInput): Promise<void> {
  * Onyx.merge(ONYXKEYS.POLICY, {id: 1}); // -> {id: 1}
  * Onyx.merge(ONYXKEYS.POLICY, {name: 'My Workspace'}); // -> {id: 1, name: 'My Workspace'}
  */
-function merge<TKey extends OnyxKey>(key: TKey extends CollectionKeyBase ? never : TKey, changes: OnyxMergeInput<TKey>): Promise<void> {
+function merge<TKey extends OnyxKey>(key: string extends CollectionKeyBase ? TKey : TKey extends CollectionKeyBase ? never : TKey, changes: OnyxMergeInput<TKey>): Promise<void> {
+    if (OnyxUtils.isCollectionKey(key as OnyxKey)) {
+        Logger.logInfo(logMessages.collectionKeyOperationAlert(key as string, 'merge'));
+        return Promise.resolve();
+    }
     return OnyxUtils.afterInit(() => {
         const skippableCollectionMemberIDs = OnyxUtils.getSkippableCollectionMemberIDs();
         if (skippableCollectionMemberIDs.size) {

lib/logMessages.ts

@@ -1,6 +1,8 @@
 const logMessages = {
     incompatibleUpdateAlert: (key: string, operation: string, existingValueType?: string, newValueType?: string) =>
         `Warning: Trying to apply "${operation}" with ${newValueType ?? 'unknown'} type to ${existingValueType ?? 'unknown'} type in the key "${key}"`,
+    collectionKeyOperationAlert: (key: string, operation: 'set' | 'merge') =>
+        `Trying to use "${operation}" with collection key "${key}". Use setCollection / mergeCollection instead.`,
 };
 
 export default logMessages;

lib/types.ts

@@ -327,7 +327,7 @@ type OnyxSetCollectionInput<TKey extends OnyxKey> = Collection<TKey, OnyxInput<T
 
 type OnyxMethodMap = typeof OnyxUtils.METHOD;
 
-type ExpandOnyxKeys<TKey extends OnyxKey> = TKey extends CollectionKeyBase ? NoInfer<`${TKey}${string}`> : TKey;
+type ExpandOnyxKeys<TKey extends OnyxKey> = string extends CollectionKeyBase ? TKey : TKey extends CollectionKeyBase ? never : TKey;
 
 /**
  * OnyxUpdate type includes all onyx methods used in OnyxMethodValueMap.

tests/unit/onyxTest.ts

@@ -594,18 +594,19 @@ describe('Onyx', () => {
         });
     });
 
-    it('should overwrite an array key nested inside an object when using merge on a collection', () => {
+    it('should overwrite an array key nested inside an object when using mergeCollection', () => {
         let testKeyValue: unknown;
         connection = Onyx.connect({
             key: ONYX_KEYS.COLLECTION.TEST_KEY,
+            waitForCollectionCallback: true,
             initWithStoredValues: false,
             callback: (value) => {
                 testKeyValue = value;
             },
         });
 
-        Onyx.merge(ONYX_KEYS.COLLECTION.TEST_KEY, {test_1: {something: [1, 2, 3]}});
-        return Onyx.merge(ONYX_KEYS.COLLECTION.TEST_KEY, {test_1: {something: [4]}}).then(() => {
+        Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {test_1: {something: [1, 2, 3]}});
+        return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {test_1: {something: [4]}}).then(() => {
             expect(testKeyValue).toEqual({test_1: {something: [4]}});
         });
     });
@@ -3006,6 +3007,32 @@ describe('Onyx', () => {
             expect(cache.get(ONYX_KEYS.RAM_ONLY_WITH_INITIAL_VALUE)).toEqual('default');
         });
     });
+
+    describe('collection key protection', () => {
+        it('set with a collection key should log an alert and not store data', async () => {
+            const logInfoSpy = jest.spyOn(Logger, 'logInfo');
+
+            await Onyx.set(ONYX_KEYS.COLLECTION.TEST_KEY as unknown as typeof ONYX_KEYS.TEST_KEY, {str: 'should not be stored'} as unknown as string);
+
+            expect(logInfoSpy).toHaveBeenCalledWith(expect.stringContaining('"set"'));
+            expect(logInfoSpy).toHaveBeenCalledWith(expect.stringContaining(ONYX_KEYS.COLLECTION.TEST_KEY));
+            expect(cache.get(ONYX_KEYS.COLLECTION.TEST_KEY)).toBeUndefined();
+
+            logInfoSpy.mockRestore();
+        });
+
+        it('merge with a collection key should log an alert and not store data', async () => {
+            const logInfoSpy = jest.spyOn(Logger, 'logInfo');
+
+            await Onyx.merge(ONYX_KEYS.COLLECTION.TEST_KEY as unknown as typeof ONYX_KEYS.TEST_KEY, {str: 'should not be stored'} as unknown as string);
+
+            expect(logInfoSpy).toHaveBeenCalledWith(expect.stringContaining('"merge"'));
+            expect(logInfoSpy).toHaveBeenCalledWith(expect.stringContaining(ONYX_KEYS.COLLECTION.TEST_KEY));
+            expect(cache.get(ONYX_KEYS.COLLECTION.TEST_KEY)).toBeUndefined();
+
+            logInfoSpy.mockRestore();
+        });
+    });
 });
 
 // Separate describe block for Onyx.init to control initialization during each test.
@@ -3111,6 +3138,7 @@ describe('Onyx.init', () => {
             expect(cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}entry1`)).toEqual('test_1');
         });
     });
+
 });
 
 // Separate describe block to control Onyx.init() per-test so we can pre-seed storage before init.