From ee7c31592c41c361437d4be48d9b5af75ea9104b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 5 Mar 2026 13:55:17 +0000 Subject: [PATCH 1/5] Dont allow set / merge of collection keys --- lib/Onyx.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index a508c4043..3a5470a3b 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -165,7 +165,7 @@ function disconnect(connection: Connection): void { * @param value value to store * @param options optional configuration object */ -function set(key: TKey, value: OnyxSetInput, options?: SetOptions): Promise { +function set(key: TKey extends CollectionKeyBase ? never : TKey, value: OnyxSetInput, options?: SetOptions): Promise { return OnyxUtils.afterInit(() => OnyxUtils.setWithRetry({key, value, options})); } @@ -196,7 +196,7 @@ function multiSet(data: OnyxMultiSetInput): Promise { * Onyx.merge(ONYXKEYS.POLICY, {id: 1}); // -> {id: 1} * Onyx.merge(ONYXKEYS.POLICY, {name: 'My Workspace'}); // -> {id: 1, name: 'My Workspace'} */ -function merge(key: TKey, changes: OnyxMergeInput): Promise { +function merge(key: TKey extends CollectionKeyBase ? never : TKey, changes: OnyxMergeInput): Promise { return OnyxUtils.afterInit(() => { const skippableCollectionMemberIDs = OnyxUtils.getSkippableCollectionMemberIDs(); if (skippableCollectionMemberIDs.size) { From 80b22548ad44a349da795902dca948a5952d373a Mon Sep 17 00:00:00 2001 From: Yauheni Pasiukevich Date: Tue, 10 Mar 2026 11:02:08 +0100 Subject: [PATCH 2/5] Add collection key operation alerts and update type handling in Onyx --- lib/Onyx.ts | 12 ++++++++++-- lib/logMessages.ts | 2 ++ lib/types.ts | 2 +- tests/unit/onyxTest.ts | 34 +++++++++++++++++++++++++++++++--- 4 files changed, 44 insertions(+), 6 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 3a5470a3b..ba9dfd29e 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -165,7 +165,11 @@ function disconnect(connection: Connection): void { * @param value value to store * @param options optional configuration object */ -function set(key: TKey extends CollectionKeyBase ? never : TKey, value: OnyxSetInput, options?: SetOptions): Promise { +function set(key: string extends CollectionKeyBase ? TKey : TKey extends CollectionKeyBase ? never : TKey, value: OnyxSetInput, options?: SetOptions): Promise { + 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 { * Onyx.merge(ONYXKEYS.POLICY, {id: 1}); // -> {id: 1} * Onyx.merge(ONYXKEYS.POLICY, {name: 'My Workspace'}); // -> {id: 1, name: 'My Workspace'} */ -function merge(key: TKey extends CollectionKeyBase ? never : TKey, changes: OnyxMergeInput): Promise { +function merge(key: string extends CollectionKeyBase ? TKey : TKey extends CollectionKeyBase ? never : TKey, changes: OnyxMergeInput): Promise { + 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) { diff --git a/lib/logMessages.ts b/lib/logMessages.ts index 5a4b84d6d..1aaea9908 100644 --- a/lib/logMessages.ts +++ b/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; diff --git a/lib/types.ts b/lib/types.ts index 92396fffe..f747c9ec2 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -327,7 +327,7 @@ type OnyxSetCollectionInput = Collection = TKey extends CollectionKeyBase ? NoInfer<`${TKey}${string}`> : TKey; +type ExpandOnyxKeys = string extends CollectionKeyBase ? TKey : TKey extends CollectionKeyBase ? never : TKey; /** * OnyxUpdate type includes all onyx methods used in OnyxMethodValueMap. diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 0ed0751a9..15f4b1cdd 100644 --- a/tests/unit/onyxTest.ts +++ b/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. From 3782c0715484845374fd8b1a367046c27a6ef514 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Wed, 25 Mar 2026 10:02:15 +0100 Subject: [PATCH 3/5] fix prettier --- lib/Onyx.ts | 6 +++++- lib/logMessages.ts | 3 +-- tests/unit/onyxTest.ts | 1 - 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 1ee0a0ca2..44405a5a5 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -168,7 +168,11 @@ function disconnect(connection: Connection): void { * @param value value to store * @param options optional configuration object */ -function set(key: string extends CollectionKeyBase ? TKey : TKey extends CollectionKeyBase ? never : TKey, value: OnyxSetInput, options?: SetOptions): Promise { +function set( + key: string extends CollectionKeyBase ? TKey : TKey extends CollectionKeyBase ? never : TKey, + value: OnyxSetInput, + options?: SetOptions, +): Promise { if (OnyxUtils.isCollectionKey(key as OnyxKey)) { Logger.logInfo(logMessages.collectionKeyOperationAlert(key as string, 'set')); return Promise.resolve(); diff --git a/lib/logMessages.ts b/lib/logMessages.ts index 1aaea9908..ce132bdd8 100644 --- a/lib/logMessages.ts +++ b/lib/logMessages.ts @@ -1,8 +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.`, + collectionKeyOperationAlert: (key: string, operation: 'set' | 'merge') => `Trying to use "${operation}" with collection key "${key}". Use setCollection / mergeCollection instead.`, }; export default logMessages; diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 3079659d4..843151a71 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -3208,7 +3208,6 @@ 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. From e51df05a26a69c1c7a9b40491ccdf24b1765d9db Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Wed, 25 Mar 2026 13:37:28 +0100 Subject: [PATCH 4/5] add and update OnyxUpdate types test after set/merge collection restriction --- tests/types/OnyxUpdate.ts | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/tests/types/OnyxUpdate.ts b/tests/types/OnyxUpdate.ts index c9ab44a50..e7a4b197e 100644 --- a/tests/types/OnyxUpdate.ts +++ b/tests/types/OnyxUpdate.ts @@ -8,7 +8,7 @@ const onyxUpdate: OnyxUpdate<'test'> = { value: 'string', }; -const onyxUpdateCollectionMember: OnyxUpdate = { +const onyxUpdateCollectionMember: OnyxUpdate<'test_1'> = { onyxMethod: 'set', key: `${ONYX_KEYS.COLLECTION.TEST_KEY}1`, value: { @@ -16,6 +16,14 @@ const onyxUpdateCollectionMember: OnyxUpdate = { + onyxMethod: 'merge', + key: `${ONYX_KEYS.COLLECTION.TEST_KEY}1`, + value: { + str: 'test1', + }, +}; + const onyxUpdateError: OnyxUpdate<'test'> = { onyxMethod: 'set', key: ONYX_KEYS.TEST_KEY, @@ -23,6 +31,20 @@ const onyxUpdateError: OnyxUpdate<'test'> = { 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', + }, }, }, { From 29f997e6403fc3b6e073befc3509a7e6f8bce744 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Wed, 25 Mar 2026 13:37:38 +0100 Subject: [PATCH 5/5] update docs after set/merge collection restriction --- API.md | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/API.md b/API.md index 545adbfab..7f78787b8 100644 --- a/API.md +++ b/API.md @@ -136,22 +136,32 @@ Onyx.disconnect(connection); ## 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'} +``` + ## 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} ```