Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mergeCollection: discrepancy between cache & storage behaviours #514

Open
paultsimura opened this issue Mar 19, 2024 · 1 comment
Open

Comments

@paultsimura
Copy link
Contributor

paultsimura commented Mar 19, 2024

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);
  1. Perform the following mergeCollection operation:
Onyx.mergeCollection('transactions_', {
    transactions_test: {
        pendingFields: {
            waypoints: null
        },
    },
});
  1. Verify the collection item subscriber was called with the following value:
{
    "pendingFields": {
        "waypoints": null
    },
}
  1. 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
Copy link
Contributor Author

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:

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant