Expensify / react-native-onyx

Persistent, offline-first state management solution for React Native. Easy to use with minimal config and boilerplate.
MIT License
156 stars 71 forks source link

`mergeCollection`: discrepancy between cache & storage behaviours #514

Open paultsimura opened 6 months ago

paultsimura commented 6 months ago

I've put up a draft PR with a unit test to cover this issue, and a potential fix: https://github.com/Expensify/react-native-onyx/pull/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 commented 6 months ago

The issue lies here:

https://github.com/Expensify/react-native-onyx/blob/4f9cebf817dc1a294e4be152a99af6df1c504f32/lib/Onyx.js#L1531-L1540

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:

https://github.com/Expensify/react-native-onyx/blob/4f9cebf817dc1a294e4be152a99af6df1c504f32/lib/storage/providers/IDBKeyVal.ts#L28-L30