[Improvement] Add a shallow merge option to Onyx.merge

[thienlnam]

Problem

The default behavior of Oynx.merge is a deep merge which will object keys and array elements. This is fine but for some scenarios, we want to overwrite the object entirely while.

For example, to merge this data

{
  A: {email: 'test@gmail.com', description: 'this is a description'}
}

into this existing onyx key

{
  A: {email: 'A@gmail.com', description: '', test: 'test', stuff: 'stuff'}
  B: {email: 'testB@gmail.com', description: 'this is a description'}
  otherKey: bla
  ...  
}

Instead of getting a new value of
A: {email: 'test@gmail.com', description: 'this is a description', test: 'test', stuff: 'stuff'}
we want to get this saved in the onyx key

{
  A: {email: 'test@gmail.com', description: 'this is a description'}
  B: {email: 'testB@gmail.com', description: 'this is a description'}
  otherKey: bla
  ...  
}

Solution

Create an option for shallow merging which will merge the values but will overwrite objects
Also could look into doing something like Onyx.removeProperty('keyName', ['A'])

Workarounds

Right now, the workaround is to first merge the key with null, and then do another merge with the new object as the Onyx.merge calls are batched and should

cc @marcaaron

[marcaaron]

Just adding some more context here. A specific example of this workaround looks like this:

// Given {errors: {someError: true}}
Onyx.merge(key, {errors: null});
Onyx.merge(key, {errors: {someOtherError: true}}); // -> {errors: {someOtherError: true}}

Since otherwise a typical deep merge would give us {errors: {someError: true, someOtherError: true}}

Shallow merge could work, but I think the most flexible one is to add a callback to Onyx.set(). This also would solve the issue where array values do not get concatenated in a deep merge. But the current workaround for that has us relying on local data e.g. here in Policy.js.

So instead we'd end up with

Onyx.set((previous) => ({
    ...previous,
    errors: {someOtherError: true},
}))

And

Onyx.set(key, (previousPolicy) => ({
    ...previousPolicy,
    employeeList: _.without(previousPolicy.employeeList, ...members),
}));

The only thing blocking me here is that a set() is more likely to cause a race condition than a merge() if called consecutively. So, I want to:

• See if we are actually doing that and add logs ✅
• Make a set() always cancel any queued merge() that happen before them in the same call
• Implement the set() with callback