Skip to content

Commit 26296ee

Browse files
TMisiukiewiczTMisiukiewicz
committed
Revert "Merge pull request Expensify#693 from callstack-internal/feat/preserve-references-of-unchanged-collection-items"
This reverts commit 0076604, reversing changes made to 7c85dfa.
1 parent 52146de commit 26296ee

File tree

2 files changed

+4
-154
lines changed

2 files changed

+4
-154
lines changed

‎lib/OnyxUtils.ts‎

Lines changed: 4 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -566,60 +566,6 @@ function tryGetCachedValue<TKey extends OnyxKey>(key: TKey): OnyxValue<OnyxKey>
566566
return val;
567567
}
568568

569-
/**
570-
* Utility function to preserve object references for unchanged items in collection operations.
571-
* Compares new values with cached values using deep equality and preserves references when data is identical.
572-
* @returns The preserved collection with unchanged references maintained
573-
*/
574-
function preserveCollectionReferences(keyValuePairs: StorageKeyValuePair[]): OnyxInputKeyValueMapping {
575-
const preservedCollection: OnyxInputKeyValueMapping = {};
576-
577-
keyValuePairs.forEach(([key, value]) => {
578-
const cachedValue = cache.get(key, false);
579-
580-
// If no cached value exists, we need to add the new value (skip expensive deep equality check)
581-
// Use deep equality check to preserve references for unchanged items
582-
if (cachedValue !== undefined && deepEqual(value, cachedValue)) {
583-
// Keep the existing reference
584-
preservedCollection[key] = cachedValue;
585-
} else {
586-
// Update cache only for changed items
587-
cache.set(key, value);
588-
preservedCollection[key] = value;
589-
}
590-
});
591-
592-
return preservedCollection;
593-
}
594-
595-
/**
596-
* Utility function for merge operations that preserves references after cache merge has been performed.
597-
* Compares merged values with original cached values and preserves references when data is unchanged.
598-
* @returns The preserved collection with unchanged references maintained
599-
*/
600-
function preserveCollectionReferencesAfterMerge(
601-
collection: Record<string, OnyxValue<OnyxKey>>,
602-
originalCachedValues: Record<string, OnyxValue<OnyxKey>>,
603-
): Record<string, OnyxValue<OnyxKey>> {
604-
const preservedCollection: Record<string, OnyxValue<OnyxKey>> = {};
605-
606-
Object.keys(collection).forEach((key) => {
607-
const newMergedValue = cache.get(key, false);
608-
const originalValue = originalCachedValues[key];
609-
610-
// Use deep equality check to preserve references for unchanged items
611-
if (originalValue !== undefined && deepEqual(newMergedValue, originalValue)) {
612-
// Keep the existing reference and update cache
613-
preservedCollection[key] = originalValue;
614-
cache.set(key, originalValue);
615-
} else {
616-
preservedCollection[key] = newMergedValue;
617-
}
618-
});
619-
620-
return preservedCollection;
621-
}
622-
623569
function getCachedCollection<TKey extends CollectionKeyBase>(collectionKey: TKey, collectionMemberKeys?: string[]): NonNullable<OnyxCollection<KeyValueMapping[TKey]>> {
624570
// Use optimized collection data retrieval when cache is populated
625571
const collectionData = cache.getCollectionData(collectionKey);
@@ -1634,21 +1580,10 @@ function mergeCollectionWithPatches<TKey extends CollectionKeyBase>(
16341580
const finalMergedCollection = {...existingKeyCollection, ...newCollection};
16351581

16361582
// Prefill cache if necessary by calling get() on any existing keys and then merge original data to cache
1637-
// and update all subscribers with reference preservation for unchanged items
1583+
// and update all subscribers
16381584
const promiseUpdate = previousCollectionPromise.then((previousCollection) => {
1639-
// Capture the original cached values before merging
1640-
const originalCachedValues: Record<string, OnyxValue<OnyxKey>> = {};
1641-
Object.keys(finalMergedCollection).forEach((key) => {
1642-
originalCachedValues[key] = cache.get(key, false);
1643-
});
1644-
1645-
// Then merge all the data into cache as normal
16461585
cache.merge(finalMergedCollection);
1647-
1648-
// Finally, preserve references for items that didn't actually change
1649-
const preservedCollection = preserveCollectionReferencesAfterMerge(finalMergedCollection, originalCachedValues);
1650-
1651-
return scheduleNotifyCollectionSubscribers(collectionKey, preservedCollection, previousCollection);
1586+
return scheduleNotifyCollectionSubscribers(collectionKey, finalMergedCollection, previousCollection);
16521587
});
16531588

16541589
return Promise.all(promises)
@@ -1712,10 +1647,9 @@ function partialSetCollection<TKey extends CollectionKeyBase>({collectionKey, co
17121647
const previousCollection = getCachedCollection(collectionKey, existingKeys);
17131648
const keyValuePairs = prepareKeyValuePairsForStorage(mutableCollection, true, undefined, true);
17141649

1715-
// Preserve references for unchanged items in partialSetCollection
1716-
const preservedCollection = preserveCollectionReferences(keyValuePairs);
1650+
keyValuePairs.forEach(([key, value]) => cache.set(key, value));
17171651

1718-
const updatePromise = scheduleNotifyCollectionSubscribers(collectionKey, preservedCollection, previousCollection);
1652+
const updatePromise = scheduleNotifyCollectionSubscribers(collectionKey, mutableCollection, previousCollection);
17191653

17201654
return Storage.multiSet(keyValuePairs)
17211655
.catch((error) => retryOperation(error, partialSetCollection, {collectionKey, collection}, retryAttempt))
@@ -1797,8 +1731,6 @@ const OnyxUtils = {
17971731
updateSnapshots,
17981732
mergeCollectionWithPatches,
17991733
partialSetCollection,
1800-
preserveCollectionReferences,
1801-
preserveCollectionReferencesAfterMerge,
18021734
logKeyChanged,
18031735
logKeyRemoved,
18041736
setWithRetry,

‎tests/unit/onyxTest.ts‎

Lines changed: 0 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -2562,59 +2562,6 @@ describe('Onyx', () => {
25622562
});
25632563
});
25642564
});
2565-
2566-
it('should preserve object references for unchanged items when merging collections', async () => {
2567-
const item1 = {id: 1, name: 'Item 1'};
2568-
const item2 = {id: 2, name: 'Item 2'};
2569-
2570-
// Set initial data using separate multiSet calls like existing tests
2571-
await Onyx.multiSet({[`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]: item1});
2572-
await Onyx.multiSet({[`${ONYX_KEYS.COLLECTION.TEST_KEY}2`]: item2});
2573-
2574-
// Get references to the cached objects
2575-
const cachedItem1 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`);
2576-
const cachedItem2 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`);
2577-
2578-
// Merge collection with same data for item1 and changed data for item2
2579-
await Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {
2580-
[`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]: {id: 1, name: 'Item 1'}, // Same data
2581-
[`${ONYX_KEYS.COLLECTION.TEST_KEY}2`]: {id: 2, name: 'Item 2 Updated'}, // Changed data
2582-
} as GenericCollection);
2583-
2584-
// Check that unchanged item keeps its reference
2585-
const newCachedItem1 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`);
2586-
const newCachedItem2 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`);
2587-
2588-
expect(newCachedItem1).toBe(cachedItem1); // Same reference
2589-
expect(newCachedItem2).not.toBe(cachedItem2); // Different reference
2590-
expect(newCachedItem2).toEqual({id: 2, name: 'Item 2 Updated'}); // Correct data
2591-
});
2592-
2593-
it('should preserve all references when no data changes', async () => {
2594-
const item1 = {id: 1, name: 'Item 1', nested: {value: 'test'}};
2595-
const item2 = {id: 2, name: 'Item 2', nested: {value: 'test2'}};
2596-
2597-
// Set initial data using separate calls
2598-
await Onyx.multiSet({[`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]: item1});
2599-
await Onyx.multiSet({[`${ONYX_KEYS.COLLECTION.TEST_KEY}2`]: item2});
2600-
2601-
// Get references to the cached objects
2602-
const cachedItem1 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`);
2603-
const cachedItem2 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`);
2604-
2605-
// Merge collection with identical data
2606-
await Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {
2607-
[`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]: {id: 1, name: 'Item 1', nested: {value: 'test'}},
2608-
[`${ONYX_KEYS.COLLECTION.TEST_KEY}2`]: {id: 2, name: 'Item 2', nested: {value: 'test2'}},
2609-
} as GenericCollection);
2610-
2611-
// Both items should keep their references since data is identical
2612-
const newCachedItem1 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`);
2613-
const newCachedItem2 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`);
2614-
2615-
expect(newCachedItem1).toBe(cachedItem1); // Same reference
2616-
expect(newCachedItem2).toBe(cachedItem2); // Same reference
2617-
});
26182565
});
26192566

26202567
describe('set', () => {
@@ -2757,35 +2704,6 @@ describe('Onyx', () => {
27572704
[routeB]: {name: 'Route B'},
27582705
});
27592706
});
2760-
2761-
it('should preserve object references for unchanged items when setting collections', async () => {
2762-
const item1 = {id: 1, name: 'Item 1'};
2763-
const item2 = {id: 2, name: 'Item 2'};
2764-
2765-
// Set initial data
2766-
await Onyx.setCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {
2767-
[`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]: item1,
2768-
[`${ONYX_KEYS.COLLECTION.TEST_KEY}2`]: item2,
2769-
} as GenericCollection);
2770-
2771-
// Get references to the cached objects
2772-
const cachedItem1 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`);
2773-
const cachedItem2 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`);
2774-
2775-
// Set collection with same data for item1 and changed data for item2
2776-
await Onyx.setCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {
2777-
[`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]: {id: 1, name: 'Item 1'}, // Same data
2778-
[`${ONYX_KEYS.COLLECTION.TEST_KEY}2`]: {id: 2, name: 'Item 2 Updated'}, // Changed data
2779-
} as GenericCollection);
2780-
2781-
// Check that unchanged item keeps its reference
2782-
const newCachedItem1 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`);
2783-
const newCachedItem2 = cache.get(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`);
2784-
2785-
expect(newCachedItem1).toBe(cachedItem1); // Same reference
2786-
expect(newCachedItem2).not.toBe(cachedItem2); // Different reference
2787-
expect(newCachedItem2).toEqual({id: 2, name: 'Item 2 Updated'}); // Correct data
2788-
});
27892707
});
27902708

27912709
describe('skippable collection member ids', () => {

0 commit comments

Comments
 (0)