From 26296eed960996808d21c4a7f5d6ed04cc3e96d3 Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Thu, 6 Nov 2025 14:14:36 +0100 Subject: [PATCH 1/3] Revert "Merge pull request #693 from callstack-internal/feat/preserve-references-of-unchanged-collection-items" This reverts commit 00766044dc1ee491709d8628f1776d4cc9b62038, reversing changes made to 7c85dfaa01e71afb222348b49071884a8e8b2602. --- lib/OnyxUtils.ts | 76 +++------------------------------------ tests/unit/onyxTest.ts | 82 ------------------------------------------ 2 files changed, 4 insertions(+), 154 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 155e0d67f..85e6ada12 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -566,60 +566,6 @@ function tryGetCachedValue(key: TKey): OnyxValue return val; } -/** - * Utility function to preserve object references for unchanged items in collection operations. - * Compares new values with cached values using deep equality and preserves references when data is identical. - * @returns The preserved collection with unchanged references maintained - */ -function preserveCollectionReferences(keyValuePairs: StorageKeyValuePair[]): OnyxInputKeyValueMapping { - const preservedCollection: OnyxInputKeyValueMapping = {}; - - keyValuePairs.forEach(([key, value]) => { - const cachedValue = cache.get(key, false); - - // If no cached value exists, we need to add the new value (skip expensive deep equality check) - // Use deep equality check to preserve references for unchanged items - if (cachedValue !== undefined && deepEqual(value, cachedValue)) { - // Keep the existing reference - preservedCollection[key] = cachedValue; - } else { - // Update cache only for changed items - cache.set(key, value); - preservedCollection[key] = value; - } - }); - - return preservedCollection; -} - -/** - * Utility function for merge operations that preserves references after cache merge has been performed. - * Compares merged values with original cached values and preserves references when data is unchanged. - * @returns The preserved collection with unchanged references maintained - */ -function preserveCollectionReferencesAfterMerge( - collection: Record>, - originalCachedValues: Record>, -): Record> { - const preservedCollection: Record> = {}; - - Object.keys(collection).forEach((key) => { - const newMergedValue = cache.get(key, false); - const originalValue = originalCachedValues[key]; - - // Use deep equality check to preserve references for unchanged items - if (originalValue !== undefined && deepEqual(newMergedValue, originalValue)) { - // Keep the existing reference and update cache - preservedCollection[key] = originalValue; - cache.set(key, originalValue); - } else { - preservedCollection[key] = newMergedValue; - } - }); - - return preservedCollection; -} - function getCachedCollection(collectionKey: TKey, collectionMemberKeys?: string[]): NonNullable> { // Use optimized collection data retrieval when cache is populated const collectionData = cache.getCollectionData(collectionKey); @@ -1634,21 +1580,10 @@ function mergeCollectionWithPatches( 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 with reference preservation for unchanged items + // and update all subscribers const promiseUpdate = previousCollectionPromise.then((previousCollection) => { - // Capture the original cached values before merging - const originalCachedValues: Record> = {}; - Object.keys(finalMergedCollection).forEach((key) => { - originalCachedValues[key] = cache.get(key, false); - }); - - // Then merge all the data into cache as normal cache.merge(finalMergedCollection); - - // Finally, preserve references for items that didn't actually change - const preservedCollection = preserveCollectionReferencesAfterMerge(finalMergedCollection, originalCachedValues); - - return scheduleNotifyCollectionSubscribers(collectionKey, preservedCollection, previousCollection); + return scheduleNotifyCollectionSubscribers(collectionKey, finalMergedCollection, previousCollection); }); return Promise.all(promises) @@ -1712,10 +1647,9 @@ function partialSetCollection({collectionKey, co const previousCollection = getCachedCollection(collectionKey, existingKeys); const keyValuePairs = prepareKeyValuePairsForStorage(mutableCollection, true, undefined, true); - // Preserve references for unchanged items in partialSetCollection - const preservedCollection = preserveCollectionReferences(keyValuePairs); + keyValuePairs.forEach(([key, value]) => cache.set(key, value)); - const updatePromise = scheduleNotifyCollectionSubscribers(collectionKey, preservedCollection, previousCollection); + const updatePromise = scheduleNotifyCollectionSubscribers(collectionKey, mutableCollection, previousCollection); return Storage.multiSet(keyValuePairs) .catch((error) => retryOperation(error, partialSetCollection, {collectionKey, collection}, retryAttempt)) @@ -1797,8 +1731,6 @@ const OnyxUtils = { updateSnapshots, mergeCollectionWithPatches, partialSetCollection, - preserveCollectionReferences, - preserveCollectionReferencesAfterMerge, logKeyChanged, logKeyRemoved, setWithRetry, diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 1d2034be2..066f4513f 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -2562,59 +2562,6 @@ describe('Onyx', () => { }); }); }); - - it('should preserve object references for unchanged items when merging collections', async () => { - const item1 = {id: 1, name: 'Item 1'}; - const item2 = {id: 2, name: 'Item 2'}; - - // Set initial data using separate multiSet calls like existing tests - await Onyx.multiSet({[`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]: item1}); - await Onyx.multiSet({[`${ONYX_KEYS.COLLECTION.TEST_KEY}2`]: item2}); - - // Get references to the cached objects - const cachedItem1 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`); - const cachedItem2 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`); - - // Merge collection with same data for item1 and changed data for item2 - await Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { - [`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]: {id: 1, name: 'Item 1'}, // Same data - [`${ONYX_KEYS.COLLECTION.TEST_KEY}2`]: {id: 2, name: 'Item 2 Updated'}, // Changed data - } as GenericCollection); - - // Check that unchanged item keeps its reference - const newCachedItem1 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`); - const newCachedItem2 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`); - - expect(newCachedItem1).toBe(cachedItem1); // Same reference - expect(newCachedItem2).not.toBe(cachedItem2); // Different reference - expect(newCachedItem2).toEqual({id: 2, name: 'Item 2 Updated'}); // Correct data - }); - - it('should preserve all references when no data changes', async () => { - const item1 = {id: 1, name: 'Item 1', nested: {value: 'test'}}; - const item2 = {id: 2, name: 'Item 2', nested: {value: 'test2'}}; - - // Set initial data using separate calls - await Onyx.multiSet({[`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]: item1}); - await Onyx.multiSet({[`${ONYX_KEYS.COLLECTION.TEST_KEY}2`]: item2}); - - // Get references to the cached objects - const cachedItem1 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`); - const cachedItem2 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`); - - // Merge collection with identical data - await Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { - [`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]: {id: 1, name: 'Item 1', nested: {value: 'test'}}, - [`${ONYX_KEYS.COLLECTION.TEST_KEY}2`]: {id: 2, name: 'Item 2', nested: {value: 'test2'}}, - } as GenericCollection); - - // Both items should keep their references since data is identical - const newCachedItem1 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`); - const newCachedItem2 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`); - - expect(newCachedItem1).toBe(cachedItem1); // Same reference - expect(newCachedItem2).toBe(cachedItem2); // Same reference - }); }); describe('set', () => { @@ -2757,35 +2704,6 @@ describe('Onyx', () => { [routeB]: {name: 'Route B'}, }); }); - - it('should preserve object references for unchanged items when setting collections', async () => { - const item1 = {id: 1, name: 'Item 1'}; - const item2 = {id: 2, name: 'Item 2'}; - - // Set initial data - await Onyx.setCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { - [`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]: item1, - [`${ONYX_KEYS.COLLECTION.TEST_KEY}2`]: item2, - } as GenericCollection); - - // Get references to the cached objects - const cachedItem1 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`); - const cachedItem2 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`); - - // Set collection with same data for item1 and changed data for item2 - await Onyx.setCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { - [`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]: {id: 1, name: 'Item 1'}, // Same data - [`${ONYX_KEYS.COLLECTION.TEST_KEY}2`]: {id: 2, name: 'Item 2 Updated'}, // Changed data - } as GenericCollection); - - // Check that unchanged item keeps its reference - const newCachedItem1 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`); - const newCachedItem2 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`); - - expect(newCachedItem1).toBe(cachedItem1); // Same reference - expect(newCachedItem2).not.toBe(cachedItem2); // Different reference - expect(newCachedItem2).toEqual({id: 2, name: 'Item 2 Updated'}); // Correct data - }); }); describe('skippable collection member ids', () => { From 6b1a6befb7c2eadf3fe4476b5c5a18155587b7ac Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Thu, 6 Nov 2025 14:18:44 +0100 Subject: [PATCH 2/3] remove remaining preserveCollectionReferences call --- lib/OnyxUtils.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 85e6ada12..2521c7a40 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1453,10 +1453,7 @@ function setCollectionWithRetry({collectionKey, const keyValuePairs = OnyxUtils.prepareKeyValuePairsForStorage(mutableCollection, true, undefined, true); const previousCollection = OnyxUtils.getCachedCollection(collectionKey); - // Preserve references for unchanged items in setCollection - const preservedCollection = OnyxUtils.preserveCollectionReferences(keyValuePairs); - - const updatePromise = OnyxUtils.scheduleNotifyCollectionSubscribers(collectionKey, preservedCollection, previousCollection); + const updatePromise = OnyxUtils.scheduleNotifyCollectionSubscribers(collectionKey, mutableCollection, previousCollection); return Storage.multiSet(keyValuePairs) .catch((error) => OnyxUtils.retryOperation(error, setCollectionWithRetry, {collectionKey, collection}, retryAttempt)) From 180e658a068432aedd36d64bbe119e7becde8a25 Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Thu, 6 Nov 2025 14:32:49 +0100 Subject: [PATCH 3/3] fix tests --- lib/OnyxUtils.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 2521c7a40..c3d66c3f7 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1453,6 +1453,8 @@ function setCollectionWithRetry({collectionKey, const keyValuePairs = OnyxUtils.prepareKeyValuePairsForStorage(mutableCollection, true, undefined, true); const previousCollection = OnyxUtils.getCachedCollection(collectionKey); + keyValuePairs.forEach(([key, value]) => cache.set(key, value)); + const updatePromise = OnyxUtils.scheduleNotifyCollectionSubscribers(collectionKey, mutableCollection, previousCollection); return Storage.multiSet(keyValuePairs)