From 6bf3296c94db3310efbe9a791487538d3f71c26e Mon Sep 17 00:00:00 2001 From: dominictb Date: Wed, 17 Apr 2024 18:08:51 +0700 Subject: [PATCH 1/6] fix: check compatible update before onyx ops Signed-off-by: dominictb --- lib/Onyx.ts | 64 +++++++++++++++++++++++++++++++--------------- lib/OnyxUtils.ts | 6 ++--- lib/logMessages.ts | 5 ++++ lib/utils.ts | 10 +++++++- 4 files changed, 61 insertions(+), 24 deletions(-) create mode 100644 lib/logMessages.ts diff --git a/lib/Onyx.ts b/lib/Onyx.ts index ee039a918..c1839b548 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -23,6 +23,7 @@ import type { OnyxValue, } from './types'; import OnyxUtils from './OnyxUtils'; +import logMessages from './logMessages'; // Keeps track of the last connectionID that was used so we can keep incrementing it let lastConnectionID = 0; @@ -209,32 +210,39 @@ function disconnect(connectionID: number, keyToRemoveFromEvictionBlocklist?: Ony */ function set(key: TKey, value: OnyxEntry): Promise { // If the value is null, we remove the key from storage - const {value: valueAfterRemoving, wasRemoved} = OnyxUtils.removeNullValues(key, value); - const valueWithoutNullValues = valueAfterRemoving as OnyxValue; + return OnyxUtils.get(key).then((existingValue) => { + if (!utils.isUpdateCompatibleWithExistingValue(value, existingValue)) { + Logger.logAlert(logMessages.incompatibleUpdateAlerts(key, 'set')); + return Promise.resolve(); + } - if (OnyxUtils.hasPendingMergeForKey(key)) { - delete OnyxUtils.getMergeQueue()[key]; - } + const {value: valueAfterRemoving, wasRemoved} = OnyxUtils.removeNullValues(key, value); + const valueWithoutNullValues = valueAfterRemoving as OnyxValue; - const hasChanged = cache.hasValueChanged(key, valueWithoutNullValues); + if (OnyxUtils.hasPendingMergeForKey(key)) { + delete OnyxUtils.getMergeQueue()[key]; + } - // Logging properties only since values could be sensitive things we don't want to log - Logger.logInfo(`set called for key: ${key}${_.isObject(value) ? ` properties: ${_.keys(value).join(',')}` : ''} hasChanged: ${hasChanged}`); + const hasChanged = cache.hasValueChanged(key, valueWithoutNullValues); - // This approach prioritizes fast UI changes without waiting for data to be stored in device storage. - const updatePromise = OnyxUtils.broadcastUpdate(key, valueWithoutNullValues, hasChanged, wasRemoved); + // Logging properties only since values could be sensitive things we don't want to log + Logger.logInfo(`set called for key: ${key}${_.isObject(value) ? ` properties: ${_.keys(value).join(',')}` : ''} hasChanged: ${hasChanged}`); - // If the value has not changed or the key got removed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead. - if (!hasChanged || wasRemoved) { - return updatePromise; - } + // This approach prioritizes fast UI changes without waiting for data to be stored in device storage. + const updatePromise = OnyxUtils.broadcastUpdate(key, valueWithoutNullValues, hasChanged, wasRemoved); - return Storage.setItem(key, valueWithoutNullValues) - .catch((error) => OnyxUtils.evictStorageAndRetry(error, set, key, valueWithoutNullValues)) - .then(() => { - OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET, key, valueWithoutNullValues); + // If the value has not changed or the key got removed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead. + if (!hasChanged || wasRemoved) { return updatePromise; - }); + } + + return Storage.setItem(key, valueWithoutNullValues) + .catch((error) => OnyxUtils.evictStorageAndRetry(error, set, key, valueWithoutNullValues)) + .then(() => { + OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET, key, valueWithoutNullValues); + return updatePromise; + }); + }); } /** @@ -307,7 +315,16 @@ function merge(key: TKey, changes: OnyxEntry utils.isUpdateCompatibleWithExistingValue(change, existingValue)); + + if (validChanges.length !== mergeQueue[key].length) { + Logger.logAlert(logMessages.incompatibleUpdateAlerts(key, 'merge')); + } + + if (!validChanges.length) { + return undefined; + } + const batchedDeltaChanges = OnyxUtils.applyMerge(undefined, validChanges, false); // Case (1): When there is no existing value in storage, we want to set the value instead of merge it. // Case (2): The presence of a top-level `null` in the merge queue instructs us to drop the whole existing value. @@ -406,10 +423,17 @@ function mergeCollection(collectionKey: TK }); const existingKeys = keys.filter((key) => persistedKeys.has(key)); + + const cachedCollectionForExistingKeys = OnyxUtils.getCachedCollection(collectionKey, existingKeys) + const newKeys = keys.filter((key) => !persistedKeys.has(key)); const existingKeyCollection = existingKeys.reduce((obj: NullableKeyValueMapping, key) => { // eslint-disable-next-line no-param-reassign + if (!utils.isUpdateCompatibleWithExistingValue(mergedCollection[key], cachedCollectionForExistingKeys[key])) { + Logger.logAlert(logMessages.incompatibleUpdateAlerts(key, 'mergeCollection')); + return obj; + } obj[key] = mergedCollection[key]; return obj; }, {}); diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 99e8b0bfc..94c3d85cb 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -401,10 +401,10 @@ function addAllSafeEvictionKeysToRecentlyAccessedList(): Promise { }); } -function getCachedCollection(collectionKey: TKey): NonNullable> { - const collectionMemberKeys = Array.from(cache.getAllKeys()).filter((storedKey) => isCollectionMemberKey(collectionKey, storedKey)); +function getCachedCollection(collectionKey: TKey, collectionMemberKeys?: string[]): NonNullable> { + const resolvedCollectionMemberKeys = collectionMemberKeys || Array.from(cache.getAllKeys()).filter((storedKey) => isCollectionMemberKey(collectionKey, storedKey)); - return collectionMemberKeys.reduce((prev: NonNullable>, key) => { + return resolvedCollectionMemberKeys.reduce((prev: NonNullable>, key) => { const cachedValue = cache.getValue(key); if (!cachedValue) { return prev; diff --git a/lib/logMessages.ts b/lib/logMessages.ts new file mode 100644 index 000000000..403846eaf --- /dev/null +++ b/lib/logMessages.ts @@ -0,0 +1,5 @@ +const logMessages = { + incompatibleUpdateAlerts: (key: string, operation: string) => `A warning occurred while applying ${operation} for key: ${key}, Warning: Trying to set array to object or vice versa`, +}; + +export default logMessages; diff --git a/lib/utils.ts b/lib/utils.ts index 02bbf4e06..a4c2739a5 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -117,4 +117,12 @@ function formatActionName(method: string, key?: OnyxKey): string { return key ? `${method.toUpperCase()}/${key}` : method.toUpperCase(); } -export default {isEmptyObject, fastMerge, formatActionName, removeNestedNullValues}; +/** validate that the update and the existing value are compatible */ +function isUpdateCompatibleWithExistingValue(value: unknown, existingValue: unknown): boolean { + if (existingValue && value && Array.isArray(existingValue) !== Array.isArray(value)) { + return false; + } + return true; +} + +export default {isEmptyObject, fastMerge, formatActionName, removeNestedNullValues, isUpdateCompatibleWithExistingValue}; From 76038958b31868f36cdbc8e9f83763bf3a1e4a55 Mon Sep 17 00:00:00 2001 From: dominictb Date: Thu, 18 Apr 2024 01:10:40 +0700 Subject: [PATCH 2/6] fix: switch to use cache.getValue for set operation Signed-off-by: dominictb --- lib/Onyx.ts | 57 ++++++++++++++++++++++++++--------------------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index c1839b548..96a25a44c 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -209,42 +209,41 @@ function disconnect(connectionID: number, keyToRemoveFromEvictionBlocklist?: Ony * @param value value to store */ function set(key: TKey, value: OnyxEntry): Promise { + // check if the value is compatible with the existing value in the storage + const existingValue = cache.getValue(key, false); + if (!utils.isUpdateCompatibleWithExistingValue(value, existingValue)) { + Logger.logAlert(logMessages.incompatibleUpdateAlerts(key, 'set')); + return Promise.resolve(); + } + // If the value is null, we remove the key from storage - return OnyxUtils.get(key).then((existingValue) => { - if (!utils.isUpdateCompatibleWithExistingValue(value, existingValue)) { - Logger.logAlert(logMessages.incompatibleUpdateAlerts(key, 'set')); - return Promise.resolve(); - } + const {value: valueAfterRemoving, wasRemoved} = OnyxUtils.removeNullValues(key, value); + const valueWithoutNullValues = valueAfterRemoving as OnyxValue; - const {value: valueAfterRemoving, wasRemoved} = OnyxUtils.removeNullValues(key, value); - const valueWithoutNullValues = valueAfterRemoving as OnyxValue; + if (OnyxUtils.hasPendingMergeForKey(key)) { + delete OnyxUtils.getMergeQueue()[key]; + } - if (OnyxUtils.hasPendingMergeForKey(key)) { - delete OnyxUtils.getMergeQueue()[key]; - } + const hasChanged = cache.hasValueChanged(key, valueWithoutNullValues); - const hasChanged = cache.hasValueChanged(key, valueWithoutNullValues); + // Logging properties only since values could be sensitive things we don't want to log + Logger.logInfo(`set called for key: ${key}${_.isObject(value) ? ` properties: ${_.keys(value).join(',')}` : ''} hasChanged: ${hasChanged}`); - // Logging properties only since values could be sensitive things we don't want to log - Logger.logInfo(`set called for key: ${key}${_.isObject(value) ? ` properties: ${_.keys(value).join(',')}` : ''} hasChanged: ${hasChanged}`); + // This approach prioritizes fast UI changes without waiting for data to be stored in device storage. + const updatePromise = OnyxUtils.broadcastUpdate(key, valueWithoutNullValues, hasChanged, wasRemoved); - // This approach prioritizes fast UI changes without waiting for data to be stored in device storage. - const updatePromise = OnyxUtils.broadcastUpdate(key, valueWithoutNullValues, hasChanged, wasRemoved); + // If the value has not changed or the key got removed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead. + if (!hasChanged || wasRemoved) { + return updatePromise; + } - // If the value has not changed or the key got removed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead. - if (!hasChanged || wasRemoved) { + return Storage.setItem(key, valueWithoutNullValues) + .catch((error) => OnyxUtils.evictStorageAndRetry(error, set, key, valueWithoutNullValues)) + .then(() => { + OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET, key, valueWithoutNullValues); return updatePromise; - } - - return Storage.setItem(key, valueWithoutNullValues) - .catch((error) => OnyxUtils.evictStorageAndRetry(error, set, key, valueWithoutNullValues)) - .then(() => { - OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET, key, valueWithoutNullValues); - return updatePromise; - }); - }); + }); } - /** * Sets multiple keys and values * @@ -424,16 +423,16 @@ function mergeCollection(collectionKey: TK const existingKeys = keys.filter((key) => persistedKeys.has(key)); - const cachedCollectionForExistingKeys = OnyxUtils.getCachedCollection(collectionKey, existingKeys) + const cachedCollectionForExistingKeys = OnyxUtils.getCachedCollection(collectionKey, existingKeys); const newKeys = keys.filter((key) => !persistedKeys.has(key)); const existingKeyCollection = existingKeys.reduce((obj: NullableKeyValueMapping, key) => { - // eslint-disable-next-line no-param-reassign if (!utils.isUpdateCompatibleWithExistingValue(mergedCollection[key], cachedCollectionForExistingKeys[key])) { Logger.logAlert(logMessages.incompatibleUpdateAlerts(key, 'mergeCollection')); return obj; } + // eslint-disable-next-line no-param-reassign obj[key] = mergedCollection[key]; return obj; }, {}); From cd21bb2baad6712274266eace272fe823ee26049 Mon Sep 17 00:00:00 2001 From: dominictb Date: Thu, 18 Apr 2024 01:49:55 +0700 Subject: [PATCH 3/6] fix: skip notifying subscribers for incompatible update keys mergeCollection operation Signed-off-by: dominictb --- lib/Onyx.ts | 8 ++++++-- tests/unit/onyxTest.js | 44 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 96a25a44c..dce5e9c9e 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -442,6 +442,7 @@ function mergeCollection(collectionKey: TK obj[key] = mergedCollection[key]; return obj; }, {}); + const keyValuePairsForExistingCollection = OnyxUtils.prepareKeyValuePairsForStorage(existingKeyCollection); const keyValuePairsForNewCollection = OnyxUtils.prepareKeyValuePairsForStorage(newCollection); @@ -457,11 +458,14 @@ function mergeCollection(collectionKey: TK promises.push(Storage.multiSet(keyValuePairsForNewCollection)); } + // finalMergedCollection contains all the keys that were merged, without the keys of incompatible updates + const finalMergedCollection = {...existingKeyCollection, ...newCollection}; + // Prefill cache if necessary by calling get() on any existing keys and then merge original data to cache // and update all subscribers const promiseUpdate = Promise.all(existingKeys.map(OnyxUtils.get)).then(() => { - cache.merge(mergedCollection); - return OnyxUtils.scheduleNotifyCollectionSubscribers(collectionKey, mergedCollection); + cache.merge(finalMergedCollection); + return OnyxUtils.scheduleNotifyCollectionSubscribers(collectionKey, finalMergedCollection); }); return Promise.all(promises) diff --git a/tests/unit/onyxTest.js b/tests/unit/onyxTest.js index a75fd6bb1..9646c25e3 100644 --- a/tests/unit/onyxTest.js +++ b/tests/unit/onyxTest.js @@ -72,6 +72,28 @@ describe('Onyx', () => { }); }); + it('should not set the key if the value is incompatible (array vs object)', () => { + let testKeyValue; + + connectionID = Onyx.connect({ + key: ONYX_KEYS.TEST_KEY, + initWithStoredValues: false, + callback: (value) => { + testKeyValue = value; + }, + }); + + return Onyx.set(ONYX_KEYS.TEST_KEY, ['test']) + .then(() => { + expect(testKeyValue).toStrictEqual(['test']); + return Onyx.set(ONYX_KEYS.TEST_KEY, {test: 'test'}); + }) + .then(() => { + expect(testKeyValue).toStrictEqual(['test']); + }); + + }) + it('should merge an object with another object', () => { let testKeyValue; @@ -93,6 +115,27 @@ describe('Onyx', () => { }); }); + it('should not merge if the value is incompatible (array vs object)', () => { + let testKeyValue; + + connectionID = Onyx.connect({ + key: ONYX_KEYS.TEST_KEY, + initWithStoredValues: false, + callback: (value) => { + testKeyValue = value; + }, + }); + + return Onyx.merge(ONYX_KEYS.TEST_KEY, ['test']) + .then(() => { + expect(testKeyValue).toStrictEqual(['test']); + return Onyx.merge(ONYX_KEYS.TEST_KEY, { test2: 'test2' }); + }) + .then(() => { + expect(testKeyValue).toStrictEqual(['test']); + }); + }) + it('should notify subscribers when data has been cleared', () => { let testKeyValue; connectionID = Onyx.connect({ @@ -389,6 +432,7 @@ describe('Onyx', () => { ID: 234, value: 'four', }, + test_3: ['abc', 'xyz'], // This shouldn't be merged since it's an array, and the original value is an object {ID, value} test_4: { ID: 456, value: 'two', From 6bc91f72ba8f76a805312465915e63b8379fd10a Mon Sep 17 00:00:00 2001 From: dominictb Date: Thu, 18 Apr 2024 10:34:47 +0700 Subject: [PATCH 4/6] chore: improve details of log message Signed-off-by: dominictb --- lib/Onyx.ts | 22 +++++++++++++--------- lib/logMessages.ts | 4 +++- lib/utils.ts | 23 ++++++++++++++++++----- tests/unit/onyxTest.js | 7 +++---- 4 files changed, 37 insertions(+), 19 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index dce5e9c9e..4b7918c68 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -211,8 +211,9 @@ function disconnect(connectionID: number, keyToRemoveFromEvictionBlocklist?: Ony function set(key: TKey, value: OnyxEntry): Promise { // check if the value is compatible with the existing value in the storage const existingValue = cache.getValue(key, false); - if (!utils.isUpdateCompatibleWithExistingValue(value, existingValue)) { - Logger.logAlert(logMessages.incompatibleUpdateAlerts(key, 'set')); + const {compatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(value, existingValue); + if (!compatible) { + Logger.logAlert(logMessages.incompatibleUpdateAlert(key, 'set', existingValueType, newValueType)); return Promise.resolve(); } @@ -314,11 +315,13 @@ function merge(key: TKey, changes: OnyxEntry utils.isUpdateCompatibleWithExistingValue(change, existingValue)); - - if (validChanges.length !== mergeQueue[key].length) { - Logger.logAlert(logMessages.incompatibleUpdateAlerts(key, 'merge')); - } + const validChanges = mergeQueue[key].filter((change) => { + const {compatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(change, existingValue); + if (!compatible) { + Logger.logAlert(logMessages.incompatibleUpdateAlert(key, 'merge', existingValueType, newValueType)); + } + return compatible; + }); if (!validChanges.length) { return undefined; @@ -428,8 +431,9 @@ function mergeCollection(collectionKey: TK const newKeys = keys.filter((key) => !persistedKeys.has(key)); const existingKeyCollection = existingKeys.reduce((obj: NullableKeyValueMapping, key) => { - if (!utils.isUpdateCompatibleWithExistingValue(mergedCollection[key], cachedCollectionForExistingKeys[key])) { - Logger.logAlert(logMessages.incompatibleUpdateAlerts(key, 'mergeCollection')); + const {compatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(mergedCollection[key], cachedCollectionForExistingKeys[key]); + if (!compatible) { + Logger.logAlert(logMessages.incompatibleUpdateAlert(key, 'mergeCollection', existingValueType, newValueType)); return obj; } // eslint-disable-next-line no-param-reassign diff --git a/lib/logMessages.ts b/lib/logMessages.ts index 403846eaf..67c97aabf 100644 --- a/lib/logMessages.ts +++ b/lib/logMessages.ts @@ -1,5 +1,7 @@ const logMessages = { - incompatibleUpdateAlerts: (key: string, operation: string) => `A warning occurred while applying ${operation} for key: ${key}, Warning: Trying to set array to object or vice versa`, + incompatibleUpdateAlert: (key: string, operation: string, existingValueType?: string, newValueType?: string) => { + return `Warning: Trying to apply "${operation}" with ${newValueType ?? 'unknown'} type to ${existingValueType ?? 'unknown'} type in the key ${key}`; + }, }; export default logMessages; diff --git a/lib/utils.ts b/lib/utils.ts index a4c2739a5..567a9ea96 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -118,11 +118,24 @@ function formatActionName(method: string, key?: OnyxKey): string { } /** validate that the update and the existing value are compatible */ -function isUpdateCompatibleWithExistingValue(value: unknown, existingValue: unknown): boolean { - if (existingValue && value && Array.isArray(existingValue) !== Array.isArray(value)) { - return false; +function checkCompatibilityWithExistingValue(value: unknown, existingValue: unknown): {compatible: boolean; existingValueType?: string; newValueType?: string} { + if (!existingValue || !value) { + return { + compatible: true, + }; } - return true; + const existingValueType = Array.isArray(existingValue) ? 'array' : 'non-array'; + const newValueType = Array.isArray(value) ? 'array' : 'non-array'; + if (existingValueType !== newValueType) { + return { + compatible: false, + existingValueType, + newValueType, + }; + } + return { + compatible: true, + }; } -export default {isEmptyObject, fastMerge, formatActionName, removeNestedNullValues, isUpdateCompatibleWithExistingValue}; +export default {isEmptyObject, fastMerge, formatActionName, removeNestedNullValues, checkCompatibilityWithExistingValue}; diff --git a/tests/unit/onyxTest.js b/tests/unit/onyxTest.js index 9646c25e3..0e86727ff 100644 --- a/tests/unit/onyxTest.js +++ b/tests/unit/onyxTest.js @@ -91,8 +91,7 @@ describe('Onyx', () => { .then(() => { expect(testKeyValue).toStrictEqual(['test']); }); - - }) + }); it('should merge an object with another object', () => { let testKeyValue; @@ -129,12 +128,12 @@ describe('Onyx', () => { return Onyx.merge(ONYX_KEYS.TEST_KEY, ['test']) .then(() => { expect(testKeyValue).toStrictEqual(['test']); - return Onyx.merge(ONYX_KEYS.TEST_KEY, { test2: 'test2' }); + return Onyx.merge(ONYX_KEYS.TEST_KEY, {test2: 'test2'}); }) .then(() => { expect(testKeyValue).toStrictEqual(['test']); }); - }) + }); it('should notify subscribers when data has been cleared', () => { let testKeyValue; From e81e210966aa9aab76fde4b3b3f073a16f31c986 Mon Sep 17 00:00:00 2001 From: dominictb Date: Fri, 19 Apr 2024 09:18:58 +0700 Subject: [PATCH 5/6] fix: small update on log message Signed-off-by: dominictb --- lib/logMessages.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/logMessages.ts b/lib/logMessages.ts index 67c97aabf..97a249da7 100644 --- a/lib/logMessages.ts +++ b/lib/logMessages.ts @@ -1,6 +1,6 @@ const logMessages = { incompatibleUpdateAlert: (key: string, operation: string, existingValueType?: string, newValueType?: string) => { - return `Warning: Trying to apply "${operation}" with ${newValueType ?? 'unknown'} type to ${existingValueType ?? 'unknown'} type in the key ${key}`; + return `Warning: Trying to apply "${operation}" with ${newValueType ?? 'unknown'} type to ${existingValueType ?? 'unknown'} type in the key "${key}"`; }, }; From affab42e98a32dcf06da3fe6cd5ea77c7db53bf0 Mon Sep 17 00:00:00 2001 From: dominictb Date: Mon, 22 Apr 2024 09:36:05 +0700 Subject: [PATCH 6/6] fix: update variable name isCompatible Signed-off-by: dominictb --- lib/Onyx.ts | 15 ++++++++------- lib/utils.ts | 8 ++++---- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 4b7918c68..c486ab2d9 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -211,8 +211,8 @@ function disconnect(connectionID: number, keyToRemoveFromEvictionBlocklist?: Ony function set(key: TKey, value: OnyxEntry): Promise { // check if the value is compatible with the existing value in the storage const existingValue = cache.getValue(key, false); - const {compatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(value, existingValue); - if (!compatible) { + const {isCompatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(value, existingValue); + if (!isCompatible) { Logger.logAlert(logMessages.incompatibleUpdateAlert(key, 'set', existingValueType, newValueType)); return Promise.resolve(); } @@ -245,6 +245,7 @@ function set(key: TKey, value: OnyxEntry(key: TKey, changes: OnyxEntry { - const {compatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(change, existingValue); - if (!compatible) { + const {isCompatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(change, existingValue); + if (!isCompatible) { Logger.logAlert(logMessages.incompatibleUpdateAlert(key, 'merge', existingValueType, newValueType)); } - return compatible; + return isCompatible; }); if (!validChanges.length) { @@ -431,8 +432,8 @@ function mergeCollection(collectionKey: TK const newKeys = keys.filter((key) => !persistedKeys.has(key)); const existingKeyCollection = existingKeys.reduce((obj: NullableKeyValueMapping, key) => { - const {compatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(mergedCollection[key], cachedCollectionForExistingKeys[key]); - if (!compatible) { + const {isCompatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(mergedCollection[key], cachedCollectionForExistingKeys[key]); + if (!isCompatible) { Logger.logAlert(logMessages.incompatibleUpdateAlert(key, 'mergeCollection', existingValueType, newValueType)); return obj; } diff --git a/lib/utils.ts b/lib/utils.ts index 567a9ea96..a3c10afab 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -118,23 +118,23 @@ function formatActionName(method: string, key?: OnyxKey): string { } /** validate that the update and the existing value are compatible */ -function checkCompatibilityWithExistingValue(value: unknown, existingValue: unknown): {compatible: boolean; existingValueType?: string; newValueType?: string} { +function checkCompatibilityWithExistingValue(value: unknown, existingValue: unknown): {isCompatible: boolean; existingValueType?: string; newValueType?: string} { if (!existingValue || !value) { return { - compatible: true, + isCompatible: true, }; } const existingValueType = Array.isArray(existingValue) ? 'array' : 'non-array'; const newValueType = Array.isArray(value) ? 'array' : 'non-array'; if (existingValueType !== newValueType) { return { - compatible: false, + isCompatible: false, existingValueType, newValueType, }; } return { - compatible: true, + isCompatible: true, }; }