diff --git a/lib/utils.ts b/lib/utils.ts index 69e4ad99..3651e434 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -16,8 +16,8 @@ function isEmptyObject(obj: T | EmptyValue): obj is EmptyValue { * Checks whether the given value can be merged. It has to be an object, but not an array, RegExp or Date. */ function isMergeableObject(value: unknown): value is Record { - const nonNullObject = value != null ? typeof value === 'object' : false; - return nonNullObject && Object.prototype.toString.call(value) !== '[object RegExp]' && Object.prototype.toString.call(value) !== '[object Date]' && !Array.isArray(value); + const isNonNullObject = value != null ? typeof value === 'object' : false; + return isNonNullObject && Object.prototype.toString.call(value) !== '[object RegExp]' && Object.prototype.toString.call(value) !== '[object Date]' && !Array.isArray(value); } /** @@ -29,43 +29,54 @@ function isMergeableObject(value: unknown): value is Record { */ function mergeObject>(target: TObject | null, source: TObject, shouldRemoveNullObjectValues = true): TObject { const destination: Record = {}; + + // First we want to copy over all keys from the target into the destination object, + // in case "target" is a mergable object. + // If "shouldRemoveNullObjectValues" is true, we want to remove null values from the merged object + // and therefore we need to omit keys where either the source or target value is null. if (isMergeableObject(target)) { - // lodash adds a small overhead so we don't use it here const targetKeys = Object.keys(target); for (let i = 0; i < targetKeys.length; ++i) { const key = targetKeys[i]; const sourceValue = source?.[key]; const targetValue = target?.[key]; - // If shouldRemoveNullObjectValues is true, we want to remove null values from the merged object + // If "shouldRemoveNullObjectValues" is true, we want to remove null values from the merged object. + // Therefore, if either target or source value is null, we want to prevent the key from being set. const isSourceOrTargetNull = targetValue === null || sourceValue === null; - const shouldOmitSourceKey = shouldRemoveNullObjectValues && isSourceOrTargetNull; + const shouldOmitTargetKey = shouldRemoveNullObjectValues && isSourceOrTargetNull; - if (!shouldOmitSourceKey) { + if (!shouldOmitTargetKey) { destination[key] = targetValue; } } } + // After copying over all keys from the target object, we want to merge the source object into the destination object. const sourceKeys = Object.keys(source); for (let i = 0; i < sourceKeys.length; ++i) { const key = sourceKeys[i]; const sourceValue = source?.[key]; const targetValue = target?.[key]; - // If shouldRemoveNullObjectValues is true, we want to remove null values from the merged object - const shouldOmitSourceKey = shouldRemoveNullObjectValues && sourceValue === null; - - // If we pass undefined as the updated value for a key, we want to generally ignore it - const isSourceKeyUndefined = sourceValue === undefined; - - if (!isSourceKeyUndefined && !shouldOmitSourceKey) { - const isSourceKeyMergable = isMergeableObject(sourceValue); - - if (isSourceKeyMergable && targetValue) { - // eslint-disable-next-line no-use-before-define - destination[key] = fastMerge(targetValue as TObject, sourceValue, shouldRemoveNullObjectValues); - } else if (!shouldRemoveNullObjectValues || sourceValue !== null) { + // If undefined is passed as the source value for a key, we want to generally ignore it. + // If "shouldRemoveNullObjectValues" is set to true and the source value is null, + // we don't want to set/merge the source value into the merged object. + const shouldIgnoreNullSourceValue = shouldRemoveNullObjectValues && sourceValue === null; + const shouldOmitSourceKey = sourceValue === undefined || shouldIgnoreNullSourceValue; + + if (!shouldOmitSourceKey) { + // If the source value is a mergable object, we want to merge it into the target value. + // If "shouldRemoveNullObjectValues" is true, "fastMerge" will recursively + // remove nested null values from the merged object. + // If source value is any other value we need to set the source value it directly. + if (isMergeableObject(sourceValue)) { + // If the target value is null or undefined, we need to fallback to an empty object, + // so that we can still use "fastMerge" to merge the source value, + // to ensure that nested null values are removed from the merged object. + const targetValueWithFallback = (targetValue ?? {}) as TObject; + destination[key] = fastMerge(targetValueWithFallback, sourceValue, shouldRemoveNullObjectValues); + } else { destination[key] = sourceValue; } } diff --git a/tests/unit/fastMergeTest.js b/tests/unit/fastMergeTest.js index 707011f3..d2c5d27f 100644 --- a/tests/unit/fastMergeTest.js +++ b/tests/unit/fastMergeTest.js @@ -69,6 +69,12 @@ describe('fastMerge', () => { }); }); + it('should merge an object with an empty object and remove deeply nested null values', () => { + const result = utils.fastMerge({}, testObjectWithNullishValues); + + expect(result).toEqual(testObjectWithNullValuesRemoved); + }); + it('should remove null values by merging two identical objects with fastMerge', () => { const result = utils.removeNestedNullValues(testObjectWithNullishValues); diff --git a/tests/unit/onyxTest.js b/tests/unit/onyxTest.js index 18f9c0d1..e50fc620 100644 --- a/tests/unit/onyxTest.js +++ b/tests/unit/onyxTest.js @@ -1013,6 +1013,80 @@ describe('Onyx', () => { }); }); + it('should merge a non-existing key with a nested null removed', () => { + let testKeyValue; + + connectionID = Onyx.connect({ + key: ONYX_KEYS.TEST_KEY, + initWithStoredValues: false, + callback: (value) => { + testKeyValue = value; + }, + }); + + return Onyx.merge(ONYX_KEYS.TEST_KEY, { + waypoints: { + 1: 'Home', + 2: 'Work', + 3: null, + }, + }).then(() => { + expect(testKeyValue).toEqual({ + waypoints: { + 1: 'Home', + 2: 'Work', + }, + }); + }); + }); + + it('should omit nested null values', () => { + let result; + + const routineRoute = `${ONYX_KEYS.COLLECTION.ROUTES}routine`; + const holidayRoute = `${ONYX_KEYS.COLLECTION.ROUTES}holiday`; + + connectionID = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.ROUTES, + initWithStoredValues: false, + callback: (value) => (result = value), + waitForCollectionCallback: true, + }); + + return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.ROUTES, { + [routineRoute]: { + waypoints: { + 1: 'Home', + 2: 'Work', + 3: 'Gym', + }, + }, + [holidayRoute]: { + waypoints: { + 1: 'Home', + 2: 'Beach', + 3: null, + }, + }, + }).then(() => { + expect(result).toEqual({ + [routineRoute]: { + waypoints: { + 1: 'Home', + 2: 'Work', + 3: 'Gym', + }, + }, + [holidayRoute]: { + waypoints: { + 1: 'Home', + 2: 'Beach', + }, + }, + }); + }); + }); + it('should apply updates in order with Onyx.update', () => { let testKeyValue;