Don't coerce empty arrays for merge when value is not an object, and improve logging

Author: chuckdries

lib/Onyx.ts

@@ -236,12 +236,11 @@ function merge<TKey extends OnyxKey>(key: TKey, changes: OnyxMergeInput<TKey>):
             }
 
             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));
                     }

lib/OnyxUtils.ts

@@ -1576,8 +1576,11 @@ function mergeCollectionWithPatches<TKey extends CollectionKeyBase>(
             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;

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';