From 7fbd5a3f2e4c8b5b28232667425e076868b098ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 18 Feb 2025 21:40:33 +0000 Subject: [PATCH 1/9] Add test --- tests/unit/onyxTest.ts | 45 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 44400e715..bcff2468d 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -1686,6 +1686,51 @@ describe('Onyx', () => { }); }); + it('should 1', async () => { + let result: unknown; + connection = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_UPDATE, + waitForCollectionCallback: true, + callback: (value) => { + result = value; + }, + }); + + await Onyx.set(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, { + sub_entry1: { + id: 'sub_entry1', + someKey: 'someValue', + }, + }); + + const queuedUpdates: OnyxUpdate[] = [ + { + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, + onyxMethod: 'merge', + value: { + sub_entry1: null, + }, + }, + { + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, + onyxMethod: 'merge', + value: { + sub_entry1: { + pendingAction: null, + }, + }, + }, + ]; + + await Onyx.update(queuedUpdates); + + expect(result).toEqual({ + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: { + sub_entry1: {}, + }, + }); + }); + describe('merge', () => { it('should remove a deeply nested null when merging an existing key', () => { let result: unknown; From beed909a722f58dde312d98fdafa0c3d46b8d8ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 24 Feb 2025 17:31:14 +0000 Subject: [PATCH 2/9] Add test case for null priorization in merges --- tests/unit/onyxTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index bcff2468d..c98d8dfe3 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -1686,7 +1686,7 @@ describe('Onyx', () => { }); }); - it('should 1', async () => { + it('should prioritize null merges in nested properties when batching updates', async () => { let result: unknown; connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_UPDATE, From b92a37483d71df08b76ede18f13adaa659b62072 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 25 Feb 2025 11:13:23 +0000 Subject: [PATCH 3/9] Early version of null merges prioritization --- lib/Onyx.ts | 8 ++++---- lib/OnyxUtils.ts | 3 ++- lib/utils.ts | 24 ++++++++++++++++++++---- tests/unit/onyxTest.ts | 4 +--- 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index baf7c0682..7df052318 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -320,7 +320,7 @@ function merge(key: TKey, changes: OnyxMergeInput): if (!validChanges.length) { return Promise.resolve(); } - const batchedDeltaChanges = OnyxUtils.applyMerge(undefined, validChanges, false); + const batchedDeltaChanges = OnyxUtils.applyMerge(undefined, validChanges, false, true); // 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. @@ -350,7 +350,7 @@ function merge(key: TKey, changes: OnyxMergeInput): // The "preMergedValue" will be directly "set" in storage instead of being merged // Therefore we merge the batched changes with the existing value to get the final merged value that will be stored. // We can remove null values from the "preMergedValue", because "null" implicates that the user wants to remove a value from storage. - const preMergedValue = OnyxUtils.applyMerge(shouldSetValue ? undefined : existingValue, [batchedDeltaChanges], true); + const preMergedValue = OnyxUtils.applyMerge(shouldSetValue ? undefined : existingValue, [batchedDeltaChanges], true, false); // In cache, we don't want to remove the key if it's null to improve performance and speed up the next merge. const hasChanged = cache.hasValueChanged(key, preMergedValue); @@ -765,7 +765,7 @@ function update(data: OnyxUpdate[]): Promise { // Remove the collection-related key from the updateQueue so that it won't be processed individually. delete updateQueue[key]; - const updatedValue = OnyxUtils.applyMerge(undefined, operations, false); + const updatedValue = OnyxUtils.applyMerge(undefined, operations, false, true); if (operations[0] === null) { // eslint-disable-next-line no-param-reassign queue.set[key] = updatedValue; @@ -790,7 +790,7 @@ function update(data: OnyxUpdate[]): Promise { }); Object.entries(updateQueue).forEach(([key, operations]) => { - const batchedChanges = OnyxUtils.applyMerge(undefined, operations, false); + const batchedChanges = OnyxUtils.applyMerge(undefined, operations, false, true); if (operations[0] === null) { promises.push(() => set(key, batchedChanges)); diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index a3ef656a8..dacd69f73 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1249,6 +1249,7 @@ function applyMerge | undefined, TChange exten existingValue: TValue, changes: TChange[], shouldRemoveNestedNulls: boolean, + isBatchingMergeChanges: boolean, ): TChange { const lastChange = changes?.at(-1); @@ -1258,7 +1259,7 @@ function applyMerge | undefined, TChange exten if (changes.some((change) => change && typeof change === 'object')) { // Object values are then merged one after the other - return changes.reduce((modifiedData, change) => utils.fastMerge(modifiedData, change, shouldRemoveNestedNulls), (existingValue || {}) as TChange); + return changes.reduce((modifiedData, change) => utils.fastMerge(modifiedData, change, shouldRemoveNestedNulls, isBatchingMergeChanges), (existingValue || {}) as TChange); } // If we have anything else we can't merge it so we'll diff --git a/lib/utils.ts b/lib/utils.ts index cdd04a54e..f8831b72f 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -27,7 +27,12 @@ function isMergeableObject(value: unknown): value is Record { * @param shouldRemoveNestedNulls - If true, null object values will be removed. * @returns - The merged object. */ -function mergeObject>(target: TObject | unknown | null | undefined, source: TObject, shouldRemoveNestedNulls = true): TObject { +function mergeObject>( + target: TObject | unknown | null | undefined, + source: TObject, + shouldRemoveNestedNulls = true, + isBatchingMergeChanges = false, +): TObject { const destination: Record = {}; const targetObject = isMergeableObject(target) ? target : undefined; @@ -78,7 +83,7 @@ function mergeObject>(target: TObject | // 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, shouldRemoveNestedNulls); + destination[key] = fastMerge(targetValueWithFallback, sourceValue, shouldRemoveNestedNulls, isBatchingMergeChanges, true, targetValue === null); } else { destination[key] = sourceValue; } @@ -95,7 +100,18 @@ function mergeObject>(target: TObject | * On native, when merging an existing value with new changes, SQLite will use JSON_PATCH, which removes top-level nullish values. * To be consistent with the behaviour for merge, we'll also want to remove null values for "set" operations. */ -function fastMerge(target: TValue, source: TValue, shouldRemoveNestedNulls = true): TValue { +function fastMerge( + target: TValue, + source: TValue, + shouldRemoveNestedNulls = true, + isBatchingMergeChanges = false, + isMergingNestedProperties = false, + isTargetOriginallyNull = false, +): TValue { + if (isBatchingMergeChanges && isMergingNestedProperties && isTargetOriginallyNull) { + return null as TValue; + } + // We have to ignore arrays and nullish values here, // otherwise "mergeObject" will throw an error, // because it expects an object as "source" @@ -103,7 +119,7 @@ function fastMerge(target: TValue, source: TValue, shouldRemoveNestedNul return source; } - return mergeObject(target, source as Record, shouldRemoveNestedNulls) as TValue; + return mergeObject(target, source as Record, shouldRemoveNestedNulls, isBatchingMergeChanges) as TValue; } /** Deep removes the nested null values from the given value. */ diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index c98d8dfe3..4a178bf67 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -1725,9 +1725,7 @@ describe('Onyx', () => { await Onyx.update(queuedUpdates); expect(result).toEqual({ - [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: { - sub_entry1: {}, - }, + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: {}, }); }); From 2a28b32a2cff184f78572e7d7ea06f23aa00783d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 25 Feb 2025 11:45:32 +0000 Subject: [PATCH 4/9] Add test case for deep nested properties --- tests/unit/onyxTest.ts | 98 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 94 insertions(+), 4 deletions(-) diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 4a178bf67..8e2a01419 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -1696,10 +1696,24 @@ describe('Onyx', () => { }, }); - await Onyx.set(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, { - sub_entry1: { - id: 'sub_entry1', - someKey: 'someValue', + await Onyx.multiSet({ + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: { + sub_entry1: { + id: 'sub_entry1', + someKey: 'someValue', + }, + }, + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`]: { + sub_entry2: { + id: 'sub_entry2', + someKey: 'someValue', + someNestedObject: { + someNestedKey: 'someNestedValue', + anotherNestedObject: { + anotherNestedKey: 'anotherNestedValue', + }, + }, + }, }, }); @@ -1720,12 +1734,88 @@ describe('Onyx', () => { }, }, }, + + { + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`, + onyxMethod: 'merge', + value: { + sub_entry2: { + someKey: 'someValueChanged', + }, + }, + }, + { + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`, + onyxMethod: 'merge', + value: { + sub_entry2: { + someNestedObject: { + anotherNestedObject: { + anotherNestedKey: 'anotherNestedValueChanged', + }, + anotherNestedObject2: { + anotherNestedKey2: 'anotherNestedValue2', + }, + }, + }, + }, + }, + { + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`, + onyxMethod: 'merge', + value: { + sub_entry2: { + someNestedObject: { + anotherNestedObject: { + anotherNestedKey: null, + }, + }, + }, + }, + }, + { + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`, + onyxMethod: 'merge', + value: { + sub_entry2: { + someNestedObject: { + anotherNestedObject: { + anotherNestedKey: 'anotherNestedValueChanged2', + }, + anotherNestedObject2: null, + }, + }, + }, + }, + { + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`, + onyxMethod: 'merge', + value: { + sub_entry2: { + someNestedObject: { + anotherNestedObject2: { + anotherNestedKey2: 'anotherNestedValue2', + }, + }, + }, + }, + }, ]; await Onyx.update(queuedUpdates); expect(result).toEqual({ [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: {}, + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`]: { + sub_entry2: { + id: 'sub_entry2', + someKey: 'someValueChanged', + someNestedObject: { + someNestedKey: 'someNestedValue', + anotherNestedObject: {}, + }, + }, + }, }); }); From 416c623bb133f94cf896ec29cd168982d5768800 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 25 Feb 2025 18:03:14 +0000 Subject: [PATCH 5/9] Preserve null values when merging simple properties during update batching --- lib/utils.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/utils.ts b/lib/utils.ts index f8831b72f..172357ad5 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -84,6 +84,8 @@ function mergeObject>( // to ensure that nested null values are removed from the merged object. const targetValueWithFallback = (targetValue ?? {}) as TObject; destination[key] = fastMerge(targetValueWithFallback, sourceValue, shouldRemoveNestedNulls, isBatchingMergeChanges, true, targetValue === null); + } else if (isBatchingMergeChanges && destination[key] === null) { + destination[key] = null; } else { destination[key] = sourceValue; } From 7c9b135fc0cf761339a974ebb3629b76a7230761 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 25 Feb 2025 18:05:40 +0000 Subject: [PATCH 6/9] Simplify logic --- lib/utils.ts | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/lib/utils.ts b/lib/utils.ts index 172357ad5..011473968 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -79,11 +79,15 @@ function mergeObject>( // 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, shouldRemoveNestedNulls, isBatchingMergeChanges, true, targetValue === null); + if (isBatchingMergeChanges && targetValue === null) { + destination[key] = null; + } else { + // 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, shouldRemoveNestedNulls, isBatchingMergeChanges); + } } else if (isBatchingMergeChanges && destination[key] === null) { destination[key] = null; } else { @@ -102,18 +106,7 @@ function mergeObject>( * On native, when merging an existing value with new changes, SQLite will use JSON_PATCH, which removes top-level nullish values. * To be consistent with the behaviour for merge, we'll also want to remove null values for "set" operations. */ -function fastMerge( - target: TValue, - source: TValue, - shouldRemoveNestedNulls = true, - isBatchingMergeChanges = false, - isMergingNestedProperties = false, - isTargetOriginallyNull = false, -): TValue { - if (isBatchingMergeChanges && isMergingNestedProperties && isTargetOriginallyNull) { - return null as TValue; - } - +function fastMerge(target: TValue, source: TValue, shouldRemoveNestedNulls = true, isBatchingMergeChanges = false): TValue { // We have to ignore arrays and nullish values here, // otherwise "mergeObject" will throw an error, // because it expects an object as "source" From c95d9e243aa40d9f1a9fd0cddceda1d24fee58fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 3 Mar 2025 11:33:16 +0000 Subject: [PATCH 7/9] Added tests for merge() --- tests/unit/onyxTest.ts | 125 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 125 insertions(+) diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 8e2a01419..6a0d8b4fc 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -1718,10 +1718,13 @@ describe('Onyx', () => { }); const queuedUpdates: OnyxUpdate[] = [ + // entry1 { key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, onyxMethod: 'merge', value: { + // Removing "sub_entry1" property in this update. + // Any subsequent changes to this property should be ignored. sub_entry1: null, }, }, @@ -1729,12 +1732,14 @@ describe('Onyx', () => { key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, onyxMethod: 'merge', value: { + // This change should be ignored because we previously changed "sub_entry1" to null. sub_entry1: { pendingAction: null, }, }, }, + // entry2 { key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`, onyxMethod: 'merge', @@ -1767,6 +1772,8 @@ describe('Onyx', () => { sub_entry2: { someNestedObject: { anotherNestedObject: { + // Removing "anotherNestedKey" property in this update. + // Any subsequent changes to this property should be ignored. anotherNestedKey: null, }, }, @@ -1780,8 +1787,11 @@ describe('Onyx', () => { sub_entry2: { someNestedObject: { anotherNestedObject: { + // This change should be ignored because we previously changed "anotherNestedKey" to null. anotherNestedKey: 'anotherNestedValueChanged2', }, + // Removing "anotherNestedObject2" property in this update. + // Any subsequent changes to this property should be ignored. anotherNestedObject2: null, }, }, @@ -1793,6 +1803,7 @@ describe('Onyx', () => { value: { sub_entry2: { someNestedObject: { + // This change should be ignored because we previously changed "anotherNestedObject2" to null. anotherNestedObject2: { anotherNestedKey2: 'anotherNestedValue2', }, @@ -1820,6 +1831,120 @@ describe('Onyx', () => { }); describe('merge', () => { + it('should prioritize null merges in nested properties when batching merges', async () => { + let result: unknown; + connection = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_UPDATE, + waitForCollectionCallback: true, + callback: (value) => { + result = value; + }, + }); + + await Onyx.multiSet({ + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: { + sub_entry1: { + id: 'sub_entry1', + someKey: 'someValue', + }, + }, + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`]: { + sub_entry2: { + id: 'sub_entry2', + someKey: 'someValue', + someNestedObject: { + someNestedKey: 'someNestedValue', + anotherNestedObject: { + anotherNestedKey: 'anotherNestedValue', + }, + }, + }, + }, + }); + + // entry1 + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, { + // Removing "sub_entry1" property in this merge. + // Any subsequent changes to this property should be ignored. + sub_entry1: null, + }); + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, { + // This change should be ignored because we previously changed "sub_entry1" to null. + sub_entry1: { + pendingAction: null, + }, + }); + + // entry2 + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`, { + sub_entry2: { + someKey: 'someValueChanged', + }, + }); + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`, { + sub_entry2: { + someNestedObject: { + anotherNestedObject: { + anotherNestedKey: 'anotherNestedValueChanged', + }, + anotherNestedObject2: { + anotherNestedKey2: 'anotherNestedValue2', + }, + }, + }, + }); + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`, { + sub_entry2: { + someNestedObject: { + anotherNestedObject: { + // Removing "anotherNestedKey" property in this merge. + // Any subsequent changes to this property should be ignored. + anotherNestedKey: null, + }, + }, + }, + }); + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`, { + sub_entry2: { + someNestedObject: { + anotherNestedObject: { + // This change should be ignored because we previously changed "anotherNestedKey" to null. + anotherNestedKey: 'anotherNestedValueChanged2', + }, + // Removing "anotherNestedObject2" property in this merge. + // Any subsequent changes to this property should be ignored. + anotherNestedObject2: null, + }, + }, + }); + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`, { + sub_entry2: { + someNestedObject: { + // This change should be ignored because we previously changed "anotherNestedObject2" to null. + anotherNestedObject2: { + anotherNestedKey2: 'anotherNestedValue2', + }, + }, + }, + }); + + await waitForPromisesToResolve(); + + expect(result).toEqual({ + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: {}, + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`]: { + sub_entry2: { + id: 'sub_entry2', + someKey: 'someValueChanged', + someNestedObject: { + someNestedKey: 'someNestedValue', + anotherNestedObject: {}, + }, + }, + }, + }); + }); + it('should remove a deeply nested null when merging an existing key', () => { let result: unknown; From 3b9400e1f6c242a12fb550dc068196718be4551a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 3 Mar 2025 16:44:41 +0000 Subject: [PATCH 8/9] Add more tests --- tests/unit/onyxTest.ts | 79 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 6a0d8b4fc..d2e0f78fd 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -1686,6 +1686,50 @@ describe('Onyx', () => { }); }); + it('should replace the old value after a null merge in the top-level object when batching updates', async () => { + let result: unknown; + connection = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_UPDATE, + waitForCollectionCallback: true, + callback: (value) => { + result = value; + }, + }); + + await Onyx.multiSet({ + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: { + id: 'entry1', + someKey: 'someValue', + }, + }); + + const queuedUpdates: OnyxUpdate[] = [ + { + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, + onyxMethod: 'merge', + // Removing the entire object in this update. + // Any subsequent changes to this key should completely replace the old value. + value: null, + }, + { + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, + onyxMethod: 'merge', + // This change should completely replace `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1` old value. + value: { + someKey: 'someValueChanged', + }, + }, + ]; + + await Onyx.update(queuedUpdates); + + expect(result).toEqual({ + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: { + someKey: 'someValueChanged', + }, + }); + }); + it('should prioritize null merges in nested properties when batching updates', async () => { let result: unknown; connection = Onyx.connect({ @@ -1831,6 +1875,41 @@ describe('Onyx', () => { }); describe('merge', () => { + it('should replace the old value after a null merge in the top-level object when batching merges', async () => { + let result: unknown; + connection = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_UPDATE, + waitForCollectionCallback: true, + callback: (value) => { + result = value; + }, + }); + + await Onyx.multiSet({ + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: { + id: 'entry1', + someKey: 'someValue', + }, + }); + + // Removing the entire object in this merge. + // Any subsequent changes to this key should completely replace the old value. + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, null); + + // This change should completely replace `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1` old value. + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, { + someKey: 'someValueChanged', + }); + + await waitForPromisesToResolve(); + + expect(result).toEqual({ + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: { + someKey: 'someValueChanged', + }, + }); + }); + it('should prioritize null merges in nested properties when batching merges', async () => { let result: unknown; connection = Onyx.connect({ From d8033dfb737200e79aa93f4e75a0116abf2add45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 3 Mar 2025 19:04:31 +0000 Subject: [PATCH 9/9] Improve test names --- tests/unit/onyxTest.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index d2e0f78fd..8dcee71a9 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -1730,7 +1730,7 @@ describe('Onyx', () => { }); }); - it('should prioritize null merges in nested properties when batching updates', async () => { + it('should ignore subsequent changes after a null merge in a nested property when batching updates', async () => { let result: unknown; connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_UPDATE, @@ -1778,7 +1778,7 @@ describe('Onyx', () => { value: { // This change should be ignored because we previously changed "sub_entry1" to null. sub_entry1: { - pendingAction: null, + newKey: 'newValue', }, }, }, @@ -1910,7 +1910,7 @@ describe('Onyx', () => { }); }); - it('should prioritize null merges in nested properties when batching merges', async () => { + it('should ignore subsequent changes after a null merge in a nested property when batching merges', async () => { let result: unknown; connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_UPDATE,