Skip to content

Commit daff9f3

Browse files
mountinymountiny
authored
Merge pull request #712 from callstack-internal/revert-707
Revert Merge pull request #707
2 parents bf438af + 526baad commit daff9f3

File tree

2 files changed

+6
-268
lines changed

2 files changed

+6
-268
lines changed

‎lib/OnyxUtils.ts‎

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

569-
/**
570-
* Marks items that existed in previousCollection but not in preservedCollection as null.
571-
* This ensures subscribers are properly notified about item removals.
572-
* @param preservedCollection - The collection to mark removed items in (mutated in place)
573-
* @param previousCollection - The previous collection state to compare against
574-
*/
575-
function markRemovedItemsAsNull(preservedCollection: OnyxInputKeyValueMapping, previousCollection: OnyxCollection<KeyValueMapping[OnyxKey]>) {
576-
if (!previousCollection) {
577-
return preservedCollection;
578-
}
579-
580-
const mutablePreservedCollection = {...preservedCollection};
581-
Object.keys(previousCollection).forEach((key) => {
582-
if (key in preservedCollection) {
583-
return;
584-
}
585-
586-
mutablePreservedCollection[key] = null;
587-
});
588-
589-
return mutablePreservedCollection;
590-
}
591-
592-
/**
593-
* Utility function to preserve object references for unchanged items in collection operations.
594-
* Compares new values with cached values using deep equality and preserves references when data is identical.
595-
* @param keyValuePairs - Array of key-value pairs to process
596-
* @param previousCollection - Optional previous collection state. If provided, removed items will be included as null
597-
* @returns The preserved collection with unchanged references maintained and removed items marked as null
598-
*/
599-
function preserveCollectionReferences(keyValuePairs: StorageKeyValuePair[], previousCollection?: OnyxCollection<KeyValueMapping[OnyxKey]>): OnyxInputKeyValueMapping {
600-
const preservedCollection: OnyxInputKeyValueMapping = {};
601-
602-
keyValuePairs.forEach(([key, value]) => {
603-
const cachedValue = cache.get(key, false);
604-
605-
// If no cached value exists, we need to add the new value (skip expensive deep equality check)
606-
// Use deep equality check to preserve references for unchanged items
607-
if (cachedValue !== undefined && deepEqual(value, cachedValue)) {
608-
// Keep the existing reference
609-
preservedCollection[key] = cachedValue;
610-
} else {
611-
// Update cache only for changed items
612-
cache.set(key, value);
613-
preservedCollection[key] = value;
614-
}
615-
});
616-
617-
if (previousCollection) {
618-
return markRemovedItemsAsNull(preservedCollection, previousCollection);
619-
}
620-
621-
return preservedCollection;
622-
}
623-
624-
/**
625-
* Utility function for merge operations that preserves references after cache merge has been performed.
626-
* Compares merged values with original cached values and preserves references when data is unchanged.
627-
* @param collection - Collection of merged data
628-
* @param originalCachedValues - Original cached values before merge
629-
* @param previousCollection - Optional previous collection state. If provided, removed items will be included as null
630-
* @returns The preserved collection with unchanged references maintained and removed items marked as null
631-
*/
632-
function preserveCollectionReferencesAfterMerge(
633-
collection: Record<string, OnyxValue<OnyxKey>>,
634-
originalCachedValues: Record<string, OnyxValue<OnyxKey>>,
635-
previousCollection?: Record<string, OnyxValue<OnyxKey>>,
636-
): Record<string, OnyxValue<OnyxKey>> {
637-
const preservedCollection: Record<string, OnyxValue<OnyxKey>> = {};
638-
639-
Object.keys(collection).forEach((key) => {
640-
const newMergedValue = cache.get(key, false);
641-
const originalValue = originalCachedValues[key];
642-
643-
// Use deep equality check to preserve references for unchanged items
644-
if (originalValue !== undefined && deepEqual(newMergedValue, originalValue)) {
645-
// Keep the existing reference and update cache
646-
preservedCollection[key] = originalValue;
647-
cache.set(key, originalValue);
648-
} else {
649-
preservedCollection[key] = newMergedValue;
650-
}
651-
});
652-
653-
if (previousCollection) {
654-
return markRemovedItemsAsNull(preservedCollection, previousCollection);
655-
}
656-
657-
return preservedCollection;
658-
}
659-
660569
function getCachedCollection<TKey extends CollectionKeyBase>(collectionKey: TKey, collectionMemberKeys?: string[]): NonNullable<OnyxCollection<KeyValueMapping[TKey]>> {
661570
// Use optimized collection data retrieval when cache is populated
662571
const collectionData = cache.getCollectionData(collectionKey);
@@ -1544,10 +1453,9 @@ function setCollectionWithRetry<TKey extends CollectionKeyBase>({collectionKey,
15441453
const keyValuePairs = OnyxUtils.prepareKeyValuePairsForStorage(mutableCollection, true, undefined, true);
15451454
const previousCollection = OnyxUtils.getCachedCollection(collectionKey);
15461455

1547-
// Preserve references for unchanged items and include removed items as null in setCollection
1548-
const preservedCollection = preserveCollectionReferences(keyValuePairs, previousCollection);
1456+
keyValuePairs.forEach(([key, value]) => cache.set(key, value));
15491457

1550-
const updatePromise = OnyxUtils.scheduleNotifyCollectionSubscribers(collectionKey, preservedCollection, previousCollection);
1458+
const updatePromise = OnyxUtils.scheduleNotifyCollectionSubscribers(collectionKey, mutableCollection, previousCollection);
15511459

15521460
return Storage.multiSet(keyValuePairs)
15531461
.catch((error) => OnyxUtils.retryOperation(error, setCollectionWithRetry, {collectionKey, collection}, retryAttempt))
@@ -1608,9 +1516,6 @@ function mergeCollectionWithPatches<TKey extends CollectionKeyBase>(
16081516

16091517
return getAllKeys()
16101518
.then((persistedKeys) => {
1611-
// Capture keys that will be removed (before calling remove())
1612-
const keysToRemove = resultCollectionKeys.filter((key) => resultCollection[key] === null && persistedKeys.has(key));
1613-
16141519
// Split to keys that exist in storage and keys that don't
16151520
const keys = resultCollectionKeys.filter((key) => {
16161521
if (resultCollection[key] === null) {
@@ -1622,8 +1527,6 @@ function mergeCollectionWithPatches<TKey extends CollectionKeyBase>(
16221527

16231528
const existingKeys = keys.filter((key) => persistedKeys.has(key));
16241529

1625-
// Get previous values for both existing keys and keys that will be removed
1626-
const allAffectedKeys = [...existingKeys, ...keysToRemove];
16271530
const cachedCollectionForExistingKeys = getCachedCollection(collectionKey, existingKeys);
16281531

16291532
const existingKeyCollection = existingKeys.reduce((obj: OnyxInputKeyValueMapping, key) => {
@@ -1660,8 +1563,7 @@ function mergeCollectionWithPatches<TKey extends CollectionKeyBase>(
16601563

16611564
// We need to get the previously existing values so we can compare the new ones
16621565
// against them, to avoid unnecessary subscriber updates.
1663-
// Include keys that will be removed so subscribers are notified about removals
1664-
const previousCollectionPromise = Promise.all(allAffectedKeys.map((key) => get(key).then((value) => [key, value]))).then(Object.fromEntries);
1566+
const previousCollectionPromise = Promise.all(existingKeys.map((key) => get(key).then((value) => [key, value]))).then(Object.fromEntries);
16651567

16661568
// New keys will be added via multiSet while existing keys will be updated using multiMerge
16671569
// This is because setting a key that doesn't exist yet with multiMerge will throw errors
@@ -1679,20 +1581,8 @@ function mergeCollectionWithPatches<TKey extends CollectionKeyBase>(
16791581
// Prefill cache if necessary by calling get() on any existing keys and then merge original data to cache
16801582
// and update all subscribers
16811583
const promiseUpdate = previousCollectionPromise.then((previousCollection) => {
1682-
// Capture the original cached values before merging
1683-
const originalCachedValues: Record<string, OnyxValue<OnyxKey>> = {};
1684-
Object.keys(finalMergedCollection).forEach((key) => {
1685-
originalCachedValues[key] = cache.get(key, false);
1686-
});
1687-
1688-
// Then merge all the data into cache as normal
1689-
16901584
cache.merge(finalMergedCollection);
1691-
1692-
// Finally, preserve references for items that didn't actually change and include removed items as null
1693-
const preservedCollection = preserveCollectionReferencesAfterMerge(finalMergedCollection, originalCachedValues, previousCollection);
1694-
1695-
return scheduleNotifyCollectionSubscribers(collectionKey, preservedCollection, previousCollection);
1585+
return scheduleNotifyCollectionSubscribers(collectionKey, finalMergedCollection, previousCollection);
16961586
});
16971587

16981588
return Promise.all(promises)
@@ -1756,10 +1646,9 @@ function partialSetCollection<TKey extends CollectionKeyBase>({collectionKey, co
17561646
const previousCollection = getCachedCollection(collectionKey, existingKeys);
17571647
const keyValuePairs = prepareKeyValuePairsForStorage(mutableCollection, true, undefined, true);
17581648

1759-
// Preserve references for unchanged items and include removed items as null in partialSetCollection
1760-
const preservedCollection = preserveCollectionReferences(keyValuePairs, previousCollection);
1649+
keyValuePairs.forEach(([key, value]) => cache.set(key, value));
17611650

1762-
const updatePromise = scheduleNotifyCollectionSubscribers(collectionKey, preservedCollection, previousCollection);
1651+
const updatePromise = scheduleNotifyCollectionSubscribers(collectionKey, mutableCollection, previousCollection);
17631652

17641653
return Storage.multiSet(keyValuePairs)
17651654
.catch((error) => retryOperation(error, partialSetCollection, {collectionKey, collection}, retryAttempt))

‎tests/unit/onyxUtilsTest.ts‎

Lines changed: 0 additions & 151 deletions
Original file line numberDiff line numberDiff line change
@@ -228,157 +228,6 @@ describe('OnyxUtils', () => {
228228

229229
await Onyx.disconnect(connection);
230230
});
231-
232-
it('should notify subscribers when removing collection items with null values', async () => {
233-
let collectionResult: OnyxCollection<unknown>;
234-
let callbackCount = 0;
235-
const routeA = `${ONYXKEYS.COLLECTION.ROUTES}A`;
236-
const routeB = `${ONYXKEYS.COLLECTION.ROUTES}B`;
237-
const routeC = `${ONYXKEYS.COLLECTION.ROUTES}C`;
238-
239-
// Test with waitForCollectionCallback: true
240-
const connection = Onyx.connect({
241-
key: ONYXKEYS.COLLECTION.ROUTES,
242-
initWithStoredValues: false,
243-
callback: (value) => {
244-
collectionResult = value;
245-
callbackCount++;
246-
},
247-
waitForCollectionCallback: true,
248-
});
249-
250-
// Set initial collection state
251-
await Onyx.setCollection(ONYXKEYS.COLLECTION.ROUTES, {
252-
[routeA]: {name: 'Route A'},
253-
[routeB]: {name: 'Route B'},
254-
[routeC]: {name: 'Route C'},
255-
} as GenericCollection);
256-
257-
callbackCount = 0; // Reset counter after initial set
258-
259-
// Remove items by setting them to null
260-
await OnyxUtils.partialSetCollection({
261-
collectionKey: ONYXKEYS.COLLECTION.ROUTES,
262-
collection: {
263-
[routeA]: null,
264-
[routeB]: null,
265-
} as GenericCollection,
266-
});
267-
268-
// Should be notified about the removal
269-
expect(callbackCount).toBeGreaterThan(0);
270-
expect(collectionResult).toEqual({
271-
[routeC]: {name: 'Route C'},
272-
});
273-
274-
await Onyx.disconnect(connection);
275-
});
276-
277-
it('should notify individual key subscribers when removing collection items with null values', async () => {
278-
let routeAValue: unknown;
279-
let routeBValue: unknown;
280-
let routeCValue: unknown;
281-
const routeA = `${ONYXKEYS.COLLECTION.ROUTES}A`;
282-
const routeB = `${ONYXKEYS.COLLECTION.ROUTES}B`;
283-
const routeC = `${ONYXKEYS.COLLECTION.ROUTES}C`;
284-
285-
// Test with waitForCollectionCallback: false (individual callbacks)
286-
const connectionA = Onyx.connect({
287-
key: routeA,
288-
initWithStoredValues: false,
289-
callback: (value) => {
290-
routeAValue = value;
291-
},
292-
});
293-
294-
const connectionB = Onyx.connect({
295-
key: routeB,
296-
initWithStoredValues: false,
297-
callback: (value) => {
298-
routeBValue = value;
299-
},
300-
});
301-
302-
const connectionC = Onyx.connect({
303-
key: routeC,
304-
initWithStoredValues: false,
305-
callback: (value) => {
306-
routeCValue = value;
307-
},
308-
});
309-
310-
// Set initial collection state
311-
await Onyx.setCollection(ONYXKEYS.COLLECTION.ROUTES, {
312-
[routeA]: {name: 'Route A'},
313-
[routeB]: {name: 'Route B'},
314-
[routeC]: {name: 'Route C'},
315-
} as GenericCollection);
316-
317-
// Remove items by setting them to null
318-
await OnyxUtils.partialSetCollection({
319-
collectionKey: ONYXKEYS.COLLECTION.ROUTES,
320-
collection: {
321-
[routeA]: null,
322-
[routeB]: null,
323-
} as GenericCollection,
324-
});
325-
326-
// Individual subscribers should be notified about removals
327-
expect(routeAValue).toBeUndefined();
328-
expect(routeBValue).toBeUndefined();
329-
expect(routeCValue).toEqual({name: 'Route C'});
330-
331-
await Onyx.disconnect(connectionA);
332-
await Onyx.disconnect(connectionB);
333-
await Onyx.disconnect(connectionC);
334-
});
335-
336-
it('should notify collection subscribers without waitForCollectionCallback when removing items', async () => {
337-
const callbackResults: Array<{value: unknown; key: string}> = [];
338-
const routeA = `${ONYXKEYS.COLLECTION.ROUTES}A`;
339-
const routeB = `${ONYXKEYS.COLLECTION.ROUTES}B`;
340-
const routeC = `${ONYXKEYS.COLLECTION.ROUTES}C`;
341-
342-
// Test without waitForCollectionCallback (gets called for each item)
343-
const connection = Onyx.connect({
344-
key: ONYXKEYS.COLLECTION.ROUTES,
345-
initWithStoredValues: false,
346-
callback: (value, key) => {
347-
callbackResults.push({value, key});
348-
},
349-
waitForCollectionCallback: false,
350-
});
351-
352-
// Set initial collection state
353-
await Onyx.setCollection(ONYXKEYS.COLLECTION.ROUTES, {
354-
[routeA]: {name: 'Route A'},
355-
[routeB]: {name: 'Route B'},
356-
[routeC]: {name: 'Route C'},
357-
} as GenericCollection);
358-
359-
callbackResults.length = 0; // Clear after initial set
360-
361-
// Remove items by setting them to null
362-
await OnyxUtils.partialSetCollection({
363-
collectionKey: ONYXKEYS.COLLECTION.ROUTES,
364-
collection: {
365-
[routeA]: null,
366-
[routeB]: null,
367-
} as GenericCollection,
368-
});
369-
370-
// Should be notified about removals (value will be undefined for removed keys)
371-
const removalCallbacks = callbackResults.filter((result) => result.value === undefined);
372-
373-
// We expect at least routeA or routeB to have undefined callback
374-
// Both should ideally be notified, but the exact behavior depends on iteration order
375-
expect(removalCallbacks.length).toBeGreaterThanOrEqual(1);
376-
const hasRouteARemoval = removalCallbacks.some((result) => result.key === routeA);
377-
const hasRouteBRemoval = removalCallbacks.some((result) => result.key === routeB);
378-
expect(hasRouteARemoval || hasRouteBRemoval).toBe(true);
379-
380-
await Onyx.disconnect(connection);
381-
});
382231
});
383232

384233
describe('keysChanged', () => {

0 commit comments

Comments
 (0)