Feature/dont allow collection key set merge

API.md

@@ -136,22 +136,32 @@ Onyx.disconnect(connection);
 <a name="set"></a>
 
 ## set(key, value, options)
-Write a value to our store with the given key
+Write a value to our store with the given key.
 
-**Kind**: global function  
+Cannot be used with bare collection keys like `report_` (i.e. `ONYXKEYS.COLLECTION.REPORT`).
+Use `setCollection()` for collection-wide operations.
+Setting individual collection members like `report_123` is allowed.
+
+**Kind**: global function
 
 | Param | Description |
 | --- | --- |
-| key | ONYXKEY to set |
+| key | ONYXKEY to set (must not be a collection key) |
 | value | value to store |
 | options | optional configuration object |
 
+**Example**
+```js
+Onyx.set(ONYXKEYS.SESSION, {token: 'abc'}); // -> {token: 'abc'}
+Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {name: 'My Report'}); // -> {name: 'My Report'}
+```
+
 <a name="multiSet"></a>
 
 ## multiSet(data)
 Sets multiple keys and values
 
-**Kind**: global function  
+**Kind**: global function
 
 | Param | Description |
 | --- | --- |
@@ -173,13 +183,18 @@ Calls to `Onyx.merge()` are batched so that any calls performed in a single tick
 applied in the order they were called. Note: `Onyx.set()` calls do not work this way so use caution when mixing
 `Onyx.merge()` and `Onyx.set()`.
 
-**Kind**: global function  
-**Example**  
+Cannot be used with bare collection keys like `report_` (i.e. `ONYXKEYS.COLLECTION.REPORT`).
+Use `mergeCollection()` for collection-wide operations. 
+Merging individual collection members like `report_123` is allowed.
+
+**Kind**: global function
+**Example**
 ```js
 Onyx.merge(ONYXKEYS.EMPLOYEE_LIST, ['Joe']); // -> ['Joe']
 Onyx.merge(ONYXKEYS.EMPLOYEE_LIST, ['Jack']); // -> ['Joe', 'Jack']
 Onyx.merge(ONYXKEYS.POLICY, {id: 1}); // -> {id: 1}
 Onyx.merge(ONYXKEYS.POLICY, {name: 'My Workspace'}); // -> {id: 1, name: 'My Workspace'}
+Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {name: 'Updated Report'}); // -> {name: 'Updated Report', ...existingData}
 ```
 <a name="mergeCollection"></a>
 

lib/Onyx.ts

@@ -168,7 +168,15 @@ function disconnect(connection: Connection): void {
  * @param value value to store
  * @param options optional configuration object
  */
-function set<TKey extends OnyxKey>(key: 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}));
 }
 
@@ -199,7 +207,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, 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,7 @@
 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/types/OnyxUpdate.ts

@@ -8,21 +8,43 @@ const onyxUpdate: OnyxUpdate<'test'> = {
     value: 'string',
 };
 
-const onyxUpdateCollectionMember: OnyxUpdate<typeof ONYX_KEYS.COLLECTION.TEST_KEY> = {
+const onyxUpdateCollectionMember: OnyxUpdate<'test_1'> = {
     onyxMethod: 'set',
     key: `${ONYX_KEYS.COLLECTION.TEST_KEY}1`,
     value: {
         str: 'test1',
     },
 };
 
+const onyxUpdateCollectionMemberMerge: OnyxUpdate<'test_1'> = {
+    onyxMethod: 'merge',
+    key: `${ONYX_KEYS.COLLECTION.TEST_KEY}1`,
+    value: {
+        str: 'test1',
+    },
+};
+
 const onyxUpdateError: OnyxUpdate<'test'> = {
     onyxMethod: 'set',
     key: ONYX_KEYS.TEST_KEY,
     // @ts-expect-error TEST_KEY is a string, not a number
     value: 2,
 };
 
+// @ts-expect-error setting a bare collection key via 'set' is not allowed
+const onyxUpdateSetCollectionKeyError: OnyxUpdate<'test_'> = {
+    onyxMethod: 'set',
+    key: ONYX_KEYS.COLLECTION.TEST_KEY,
+    value: {str: 'test1'},
+};
+
+// @ts-expect-error merging a bare collection key via 'merge' is not allowed
+const onyxUpdateMergeCollectionKeyError: OnyxUpdate<'test_'> = {
+    onyxMethod: 'merge',
+    key: ONYX_KEYS.COLLECTION.TEST_KEY,
+    value: {str: 'test1'},
+};
+
 const onyxUpdateCollection: OnyxUpdate<'test_'> = {
     onyxMethod: 'mergecollection',
     key: ONYX_KEYS.COLLECTION.TEST_KEY,
@@ -105,10 +127,12 @@ Onyx.update([
 
 Onyx.update([
     {
-        onyxMethod: 'merge',
+        onyxMethod: 'mergecollection',
         key: ONYX_KEYS.COLLECTION.TEST_KEY,
         value: {
-            str: 'test1',
+            [`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]: {
+                str: 'test1',
+            },
         },
     },
     {

tests/unit/onyxTest.ts

@@ -664,18 +664,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]}});
         });
     });
@@ -3076,6 +3077,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.