From 98e4511b896ab57b4bec4c50dc3d28edec0dbcc5 Mon Sep 17 00:00:00 2001 From: Chuck Dries Date: Wed, 18 Mar 2026 11:43:24 -0700 Subject: [PATCH 01/10] Allow MERGE operations into empty array values --- lib/OnyxUtils.ts | 2 +- lib/utils.ts | 6 ++++++ tests/unit/onyxTest.ts | 42 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 3e884e2b3..7a88e5251 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1060,7 +1060,7 @@ function mergeInternal | undefined, TChange ex return modifiedData; }, { - result: (existingValue ?? {}) as TChange, + result: ((Array.isArray(existingValue) && existingValue.length === 0 ? {} : existingValue) ?? {}) as TChange, replaceNullPatches: [], }, ); diff --git a/lib/utils.ts b/lib/utils.ts index d7d296531..20cf4f5d4 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -213,6 +213,12 @@ function checkCompatibilityWithExistingValue(value: unknown, existingValue: unkn isCompatible: true, }; } + // Empty arrays are compatible with objects — PHP's json_encode produces [] + // for empty associative arrays that should be {}. + if (Array.isArray(existingValue) && existingValue.length === 0 && !Array.isArray(value)) { + return {isCompatible: true}; + } + const existingValueType = Array.isArray(existingValue) ? 'array' : 'non-array'; const newValueType = Array.isArray(value) ? 'array' : 'non-array'; if (existingValueType !== newValueType) { diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 67e6d1cb9..467071217 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -182,6 +182,48 @@ describe('Onyx', () => { }); }); + it('should merge an object into an empty array (treating [] as {})', () => { + let testKeyValue: unknown; + + connection = Onyx.connect({ + key: ONYX_KEYS.TEST_KEY, + initWithStoredValues: false, + callback: (value) => { + testKeyValue = value; + }, + }); + + return Onyx.set(ONYX_KEYS.TEST_KEY, []) + .then(() => { + expect(testKeyValue).toStrictEqual([]); + return Onyx.merge(ONYX_KEYS.TEST_KEY, {test: 'value'}); + }) + .then(() => { + expect(testKeyValue).toStrictEqual({test: 'value'}); + }); + }); + + it('should still reject merging an object into a non-empty array', () => { + let testKeyValue: unknown; + + connection = Onyx.connect({ + key: ONYX_KEYS.TEST_KEY, + initWithStoredValues: false, + callback: (value) => { + testKeyValue = value; + }, + }); + + return Onyx.merge(ONYX_KEYS.TEST_KEY, ['existing']) + .then(() => { + expect(testKeyValue).toStrictEqual(['existing']); + return Onyx.merge(ONYX_KEYS.TEST_KEY, {test: 'value'}); + }) + .then(() => { + expect(testKeyValue).toStrictEqual(['existing']); + }); + }); + it('should notify subscribers when data has been cleared', () => { let testKeyValue: unknown; connection = Onyx.connect({ From 479bc113d0273527305c44a5352627b8a484c619 Mon Sep 17 00:00:00 2001 From: Chuck Dries Date: Wed, 18 Mar 2026 13:22:52 -0700 Subject: [PATCH 02/10] Log alert when merging into an empty array --- lib/Onyx.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 2b836bb19..7ec77ec63 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -236,6 +236,10 @@ function merge(key: TKey, changes: OnyxMergeInput): } try { + if (Array.isArray(existingValue) && existingValue.length === 0) { + Logger.logAlert(`[ENSURE_BUGBOT] Onyx merge called on key "${key}" whose existing value is an empty array. Will coerce to object.`); + } + const validChanges = mergeQueue[key].filter((change) => { const {isCompatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(change, existingValue); if (!isCompatible) { From 6eec8b49e2722fee2e252adc63fa5100797ab747 Mon Sep 17 00:00:00 2001 From: Chuck Dries Date: Wed, 18 Mar 2026 16:38:45 -0700 Subject: [PATCH 03/10] Don't coerce empty arrays for merge when value is not an object, and improve logging --- lib/Onyx.ts | 9 ++++----- lib/OnyxUtils.ts | 5 ++++- lib/utils.ts | 11 ++++++----- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 7ec77ec63..5149638c1 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -236,12 +236,11 @@ function merge(key: TKey, changes: OnyxMergeInput): } try { - if (Array.isArray(existingValue) && existingValue.length === 0) { - Logger.logAlert(`[ENSURE_BUGBOT] Onyx merge called on key "${key}" whose existing value is an empty array. Will coerce to object.`); - } - const validChanges = mergeQueue[key].filter((change) => { - const {isCompatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(change, existingValue); + const {isCompatible, existingValueType, newValueType, isEmptyArrayCoercion} = utils.checkCompatibilityWithExistingValue(change, existingValue); + if (isEmptyArrayCoercion) { + Logger.logAlert(`[ENSURE_BUGBOT] Onyx merge called on key "${key}" whose existing value is an empty array. Will coerce to object.`); + } if (!isCompatible) { Logger.logAlert(logMessages.incompatibleUpdateAlert(key, 'merge', existingValueType, newValueType)); } diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 7a88e5251..b51068886 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1576,8 +1576,11 @@ function mergeCollectionWithPatches( const cachedCollectionForExistingKeys = getCachedCollection(collectionKey, existingKeys); const existingKeyCollection = existingKeys.reduce((obj: OnyxInputKeyValueMapping, key) => { - const {isCompatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(resultCollection[key], cachedCollectionForExistingKeys[key]); + const {isCompatible, existingValueType, newValueType, isEmptyArrayCoercion} = utils.checkCompatibilityWithExistingValue(resultCollection[key], cachedCollectionForExistingKeys[key]); + if (isEmptyArrayCoercion) { + Logger.logAlert(`[ENSURE_BUGBOT] Onyx mergeCollection called on key "${key}" whose existing value is an empty array. Will coerce to object.`); + } if (!isCompatible) { Logger.logAlert(logMessages.incompatibleUpdateAlert(key, 'mergeCollection', existingValueType, newValueType)); return obj; diff --git a/lib/utils.ts b/lib/utils.ts index 20cf4f5d4..9a81efe27 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -207,16 +207,17 @@ function formatActionName(method: string, key?: OnyxKey): string { } /** validate that the update and the existing value are compatible */ -function checkCompatibilityWithExistingValue(value: unknown, existingValue: unknown): {isCompatible: boolean; existingValueType?: string; newValueType?: string} { +function checkCompatibilityWithExistingValue(value: unknown, existingValue: unknown): {isCompatible: boolean; existingValueType?: string; newValueType?: string; isEmptyArrayCoercion?: boolean} { if (!existingValue || !value) { return { isCompatible: true, }; } - // Empty arrays are compatible with objects — PHP's json_encode produces [] - // for empty associative arrays that should be {}. - if (Array.isArray(existingValue) && existingValue.length === 0 && !Array.isArray(value)) { - return {isCompatible: true}; + // An empty array existing value is compatible with an object update. + // PHP's json_encode produces [] for empty associative arrays that should be {}. + const isObjectValue = typeof value === 'object' && !Array.isArray(value); + if (Array.isArray(existingValue) && existingValue.length === 0 && isObjectValue) { + return {isCompatible: true, isEmptyArrayCoercion: true}; } const existingValueType = Array.isArray(existingValue) ? 'array' : 'non-array'; From 8a881f4d42b3445512017e836b82088eae4fdf8e Mon Sep 17 00:00:00 2001 From: Chuck Dries Date: Wed, 18 Mar 2026 16:40:59 -0700 Subject: [PATCH 04/10] Do not allow empty array coercion in setWithRetry --- lib/OnyxUtils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index b51068886..f0da019e7 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1343,8 +1343,8 @@ function setWithRetry({key, value, options}: SetParams Date: Wed, 18 Mar 2026 16:43:46 -0700 Subject: [PATCH 05/10] Actually, allow empty array coercion in setWithRetry. Might as well be consistent --- lib/OnyxUtils.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index f0da019e7..3d68c2316 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1344,7 +1344,10 @@ function setWithRetry({key, value, options}: SetParams Date: Wed, 18 Mar 2026 19:52:02 -0700 Subject: [PATCH 06/10] Prettier diff --- lib/OnyxUtils.ts | 5 ++++- lib/utils.ts | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 3d68c2316..50a7fc952 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1579,7 +1579,10 @@ function mergeCollectionWithPatches( const cachedCollectionForExistingKeys = getCachedCollection(collectionKey, existingKeys); const existingKeyCollection = existingKeys.reduce((obj: OnyxInputKeyValueMapping, key) => { - const {isCompatible, existingValueType, newValueType, isEmptyArrayCoercion} = utils.checkCompatibilityWithExistingValue(resultCollection[key], cachedCollectionForExistingKeys[key]); + const {isCompatible, existingValueType, newValueType, isEmptyArrayCoercion} = utils.checkCompatibilityWithExistingValue( + resultCollection[key], + cachedCollectionForExistingKeys[key], + ); if (isEmptyArrayCoercion) { Logger.logAlert(`[ENSURE_BUGBOT] Onyx mergeCollection called on key "${key}" whose existing value is an empty array. Will coerce to object.`); diff --git a/lib/utils.ts b/lib/utils.ts index 9a81efe27..6bf0d7289 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -207,7 +207,10 @@ function formatActionName(method: string, key?: OnyxKey): string { } /** validate that the update and the existing value are compatible */ -function checkCompatibilityWithExistingValue(value: unknown, existingValue: unknown): {isCompatible: boolean; existingValueType?: string; newValueType?: string; isEmptyArrayCoercion?: boolean} { +function checkCompatibilityWithExistingValue( + value: unknown, + existingValue: unknown, +): {isCompatible: boolean; existingValueType?: string; newValueType?: string; isEmptyArrayCoercion?: boolean} { if (!existingValue || !value) { return { isCompatible: true, From dfaa39b6d33c87bc2bd8e3cc98989e37f5379c04 Mon Sep 17 00:00:00 2001 From: Chuck Dries Date: Thu, 19 Mar 2026 13:57:19 -0700 Subject: [PATCH 07/10] Cover setWithRetry and mergeCollection for empty array coercion behavior --- tests/unit/onyxTest.ts | 64 ++++++++++++++++++++++++++++++------------ 1 file changed, 46 insertions(+), 18 deletions(-) diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 467071217..bfafd0768 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -182,7 +182,7 @@ describe('Onyx', () => { }); }); - it('should merge an object into an empty array (treating [] as {})', () => { + it('should merge an object into an empty array (treating [] as {})', async () => { let testKeyValue: unknown; connection = Onyx.connect({ @@ -193,17 +193,13 @@ describe('Onyx', () => { }, }); - return Onyx.set(ONYX_KEYS.TEST_KEY, []) - .then(() => { - expect(testKeyValue).toStrictEqual([]); - return Onyx.merge(ONYX_KEYS.TEST_KEY, {test: 'value'}); - }) - .then(() => { - expect(testKeyValue).toStrictEqual({test: 'value'}); - }); + await Onyx.set(ONYX_KEYS.TEST_KEY, []); + expect(testKeyValue).toStrictEqual([]); + await Onyx.merge(ONYX_KEYS.TEST_KEY, {test: 'value'}); + expect(testKeyValue).toStrictEqual({test: 'value'}); }); - it('should still reject merging an object into a non-empty array', () => { + it('should still reject merging an object into a non-empty array', async () => { let testKeyValue: unknown; connection = Onyx.connect({ @@ -214,14 +210,46 @@ describe('Onyx', () => { }, }); - return Onyx.merge(ONYX_KEYS.TEST_KEY, ['existing']) - .then(() => { - expect(testKeyValue).toStrictEqual(['existing']); - return Onyx.merge(ONYX_KEYS.TEST_KEY, {test: 'value'}); - }) - .then(() => { - expect(testKeyValue).toStrictEqual(['existing']); - }); + await Onyx.merge(ONYX_KEYS.TEST_KEY, ['existing']); + expect(testKeyValue).toStrictEqual(['existing']); + await Onyx.merge(ONYX_KEYS.TEST_KEY, {test: 'value'}); + expect(testKeyValue).toStrictEqual(['existing']); + }); + + it('should mergeCollection an object into an empty array (treating [] as {})', async () => { + const collectionKey = `${ONYX_KEYS.COLLECTION.TEST_KEY}1`; + let testKeyValue: unknown; + + connection = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_KEY, + initWithStoredValues: false, + waitForCollectionCallback: true, + callback: (value) => { + testKeyValue = value; + }, + }); + + await Onyx.set(collectionKey, []); + expect((testKeyValue as Record)?.[collectionKey]).toStrictEqual([]); + await Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {[collectionKey]: {test: 'value'}}); + expect((testKeyValue as Record)?.[collectionKey]).toStrictEqual({test: 'value'}); + }); + + it('should set an object over an empty array (treating [] as {})', async () => { + let testKeyValue: unknown; + + connection = Onyx.connect({ + key: ONYX_KEYS.TEST_KEY, + initWithStoredValues: false, + callback: (value) => { + testKeyValue = value; + }, + }); + + await Onyx.set(ONYX_KEYS.TEST_KEY, []); + expect(testKeyValue).toStrictEqual([]); + await Onyx.set(ONYX_KEYS.TEST_KEY, {test: 'value'}); + expect(testKeyValue).toStrictEqual({test: 'value'}); }); it('should notify subscribers when data has been cleared', () => { From f00e111620ad68f5f27e3d4ab274893fbf711a24 Mon Sep 17 00:00:00 2001 From: Chuck Dries Date: Thu, 19 Mar 2026 14:02:05 -0700 Subject: [PATCH 08/10] Improve checkCompatibilityWithExistingValue comment --- lib/utils.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/utils.ts b/lib/utils.ts index 6bf0d7289..d057bf418 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -216,8 +216,13 @@ function checkCompatibilityWithExistingValue( isCompatible: true, }; } - // An empty array existing value is compatible with an object update. - // PHP's json_encode produces [] for empty associative arrays that should be {}. + + // PHP's associative arrays cannot distinguish between an empty list and an + // empty object, so it encodes both as []. A key that should hold an + // object may arrive from the server as [] and be stored that way. If + // we then try to MERGE an object into that key, the array-vs-object type check + // would normally block it. Since an empty array carries no data worth + // preserving, we treat it as compatible with an object update and coerce it. const isObjectValue = typeof value === 'object' && !Array.isArray(value); if (Array.isArray(existingValue) && existingValue.length === 0 && isObjectValue) { return {isCompatible: true, isEmptyArrayCoercion: true}; From dad8aa4a5b000611a9bd9de23f53619f9ad8713d Mon Sep 17 00:00:00 2001 From: Chuck Dries Date: Thu, 19 Mar 2026 14:06:10 -0700 Subject: [PATCH 09/10] Revert unnecessary changes to mergeInternal --- lib/OnyxUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 50a7fc952..385dbbf38 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1060,7 +1060,7 @@ function mergeInternal | undefined, TChange ex return modifiedData; }, { - result: ((Array.isArray(existingValue) && existingValue.length === 0 ? {} : existingValue) ?? {}) as TChange, + result: (existingValue ?? {}) as TChange, replaceNullPatches: [], }, ); From 9ce1bc7bf0f7db6ccad9ad8a2d9dcc351b3d6866 Mon Sep 17 00:00:00 2001 From: Chuck Dries Date: Thu, 19 Mar 2026 14:42:07 -0700 Subject: [PATCH 10/10] Better comments for future engineers --- lib/Onyx.ts | 3 +++ lib/OnyxUtils.ts | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 5149638c1..3da6a4dee 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -239,6 +239,9 @@ function merge(key: TKey, changes: OnyxMergeInput): const validChanges = mergeQueue[key].filter((change) => { const {isCompatible, existingValueType, newValueType, isEmptyArrayCoercion} = utils.checkCompatibilityWithExistingValue(change, existingValue); if (isEmptyArrayCoercion) { + // Merging an object into an empty array isn't semantically correct, but we allow it + // in case we accidentally encoded an empty object as an empty array in PHP. If you're + // looking at a bugbot from this message, we're probably missing that key in OnyxKeys::KEYS_REQUIRING_EMPTY_OBJECT Logger.logAlert(`[ENSURE_BUGBOT] Onyx merge called on key "${key}" whose existing value is an empty array. Will coerce to object.`); } if (!isCompatible) { diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 385dbbf38..1a9b11bb8 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1345,6 +1345,9 @@ function setWithRetry({key, value, options}: SetParams( ); if (isEmptyArrayCoercion) { + // Merging an object into an empty array isn't semantically correct, but we allow it + // in case we accidentally encoded an empty object as an empty array in PHP. If you're + // looking at a bugbot from this message, we're probably missing that key in OnyxKeys::KEYS_REQUIRING_EMPTY_OBJECT Logger.logAlert(`[ENSURE_BUGBOT] Onyx mergeCollection called on key "${key}" whose existing value is an empty array. Will coerce to object.`); } if (!isCompatible) {