mergeCollection: discrepancy between cache & storage behaviours

[paultsimura]

I've put up a draft PR with a unit test to cover this issue, and a potential fix: #515

Test case:

1. Setup the initial value:

const initialValue = {
    pendingFields: {
        waypoints: 'add'
    },
};
Onyx.set('transactions_test', initialValue);

2. Perform the following mergeCollection operation:

Onyx.mergeCollection('transactions_', {
    transactions_test: {
        pendingFields: {
            waypoints: null
        },
    },
});

3. Verify the collection item subscriber was called with the following value:

{
    "pendingFields": {
        "waypoints": null
    },
}

4. Check the data processed in IDBKeyVal.multiMerge.

   Expected:
   The data persisted in the Storage must be the same as the cached data.

   Actual:
   Storage persists the following data:

{
    "pendingFields": {
        "waypoints": "add"
    }
}

This causes the following bug: after the operation is complete, the pendingFields are reset correctly (because the subscriber callback is called with the cached data), but after the page is refreshed, the pendingFields are fetched from the Storage and show waypoints: "add".

[paultsimura]

The issue lies here:

react-native-onyx/lib/Onyx.js

Lines 1531 to 1540 in 4f9cebf

	const keyValuePairsForExistingCollection = prepareKeyValuePairsForStorage(existingKeyCollection);
	const keyValuePairsForNewCollection = prepareKeyValuePairsForStorage(newCollection);
	const promises = [];
	// New keys will be added via multiSet while existing keys will be updated using multiMerge
	// This is because setting a key that doesn't exist yet with multiMerge will throw errors
	if (keyValuePairsForExistingCollection.length > 0) {
	promises.push(Storage.multiMerge(keyValuePairsForExistingCollection));
	}

When calling prepareKeyValuePairsForStorage(existingKeyCollection), we remove the null values, which results in the data for multiMerge being: "pendingFields": {}

However, since the existing data holds a value under pendingFields.waypoints, the newValue, which is a result of fastMerge({waypoints: 'add'}, {}), retains the old value in the Storage:

react-native-onyx/lib/storage/providers/IDBKeyVal.ts

Lines 28 to 30 in 4f9cebf

	const prev = values[index];
	const newValue = utils.fastMerge(prev as Record<string, unknown>, value as Record<string, unknown>);
	return promisifyRequest(store.put(newValue, key));