react-native-onyx: Onyx.update doesn't apply updates in order with set and merge

Problem

Onyx.update doesn’t apply updates in order when mixing set and merge

If Onyx.update is called with an update to merge an object on a key, and then the next update removes it by calling set with the value as null, then the updates are not applied in order. The Onyx.set update is applied first and the end state of the system is that the key is not removed.

Also, subscribers are notified with each update. In some cases we want to only notify subscribers with the correct end state of the system.

Related to [Tracking] “Replay effect” when sequential actions are taken offline, and user subsequently comes online

Example

I found this to be the root problem of this app issue https://github.com/Expensify/App/issues/15550. Here is the user action and results.

Action Performed:

  1. Turn off network
  2. Create a workspace
  3. Delete the workspace
  4. Turn on network
  5. Notice that the deleted workspace blinks

Expected Result:

The deleted workspace should not be visible

Actual Result:

The deleted workspace blinks

After we go back online and the sequential queue finishes processing, we call Onyx.update with a list of queued updates. We have already optimistically created and deleted the policy, and the list of updates includes creating and deleting the policy from the network responses. We create the policy with a call to merge and delete it with a call to set. To test I added some log lines and ran the following code in the JS console. You can see that the set call is applied first when it should be applied second.

Logs
Onyx.update([
    {
        onyxMethod: 'merge',
        key: 'policy_BEB826F4F75AD818',
        value: {
            id: 'BEB826F4F75AD818',
            name: "Expensifail's Workspace 2",
            role: 'admin',
            type: 'free',
            owner: 'neil+chat@expensifail.com',
            outputCurrency: 'USD',
            employeeList: [
                {
                    email: 'neil+chat@expensifail.com',
                    forwardsTo: '',
                    role: 'admin',
                    submitsTo: 'neil+chat@expensifail.com',
                },
            ],
        },
    },
    {
        onyxMethod: 'set',
        key: 'policy_BEB826F4F75AD818',
        value: null,
    },
]);
web.development.js:1083 [Onyx] top of merge, key = policy_BEB826F4F75AD818 value =  {id: 'BEB826F4F75AD818', name: "Expensifail's Workspace 2", role: 'admin', type: 'free', owner: 'neil+chat@expensifail.com', …}
web.development.js:941 [Onyx] top of set, key = policy_BEB826F4F75AD818 value =  null
web.development.js:891 [Onyx] top of remove
web.development.js:1101 [Onyx] merge method calling set, modifiedData = {id: 'BEB826F4F75AD818', name: "Expensifail's Workspace 2", role: 'admin', type: 'free', owner: 'neil+chat@expensifail.com', …}
web.development.js:941 [Onyx] top of set, key = policy_BEB826F4F75AD818 value =  {id: 'BEB826F4F75AD818', name: "Expensifail's Workspace 2", role: 'admin', type: 'free', owner: 'neil+chat@expensifail.com', …}
web.development.js:962 [Onyx] set method called cache.set key =  policy_BEB826F4F75AD818 value = {id: 'BEB826F4F75AD818', name: "Expensifail's Workspace 2", role: 'admin', type: 'free', owner: 'neil+chat@expensifail.com', …}
PolicyUtils.js:10 [policy_BEB826F4F75AD818] subscriber callback, val =  null
PolicyUtils.js:10 [policy_BEB826F4F75AD818] subscriber callback, val =  {id: 'BEB826F4F75AD818', name: "Expensifail's Workspace 2", role: 'admin', type: 'free', owner: 'neil+chat@expensifail.com', …}
Promise {<pending>}

Why it’s important

Applying all updates from the Networks responses at once fixes the “replay effect” by updating the App with the proper end state as determined by the server without replaying any intermediate and redundant updates. If we applied these updates as soon as each API response resolved, then we would see each optimistic action we took while offline replayed in the UI.

However, because of this bug we end up in the wrong state and so the replay effect bug is not solved.

Solution

Modify Onyx.update so that it always applies all updates in order. Also, as an additional improvement, prevent notifying subscribers until all the updates have been applied.

About this issue

  • Original URL
  • State: closed
  • Created a year ago
  • Comments: 17 (15 by maintainers)

Most upvoted comments

PR merged! Please put up another to update the Onyx version in App.