Implement JSON_PATCH() in Onyx.merge() in react-native-onyx(native)

[chrispader]

Details

JSON_PATCH is an algorithm in SQLite we can use on iOS and Android to merge delta changes natively (in the storage provider) instead of manually "pre-merging" in the "Onyx layer .

The original goal of this PR was to utilise JSON_PATCH on native platforms, whilst still retaining the current merge logic on web.

(By "pre-merging" i mean using the current implementation of applyMerge which uses a mergeQueue to batch and merge delta changes, after which data will be written to storage with Storage.setItem instead of Storage.mergeItem)

Related Issues

Expensify/App#13972

Automated Tests

Linked PRs

[marcaaron]

Just leaving a couple comments. I think it would be good to lay out this approach somewhere and get more feedback.

@tgolen or @kidroca might also be able to help validate the correct approach.

The existing code (merge queue stuff) is far from perfect and could likely be improved - but something about the notifyAfterWrite() direction is making me uneasy and I would appreciate some more eyes on this problem.

[kidroca]

Left some notes

---

I have a question:

• We need to perform the merge in order to update memory cache
• Then why do we move the merge logic to storage providers

If we need to extract the merge logic we can extract it to a lib and import the storage provider there (this would import the Storage provider we're currently using)

[marcaaron]

I think maybe this one is on hold pending the AsyncStorage removal?

[marcaaron]

Still holding

[chrispader]

@marcaaron @kidroca change requests resolved 👍

[marcaaron]

Can we fix the lint errors?

[chrispader]

Can we fix the lint errors?

Done, though i'm not sure if this eslint rule should trigger here. Basically it gave errors because the withOnyx HOC calls in the tests don't have propTypes, although the component that it wraps has those defined...

[marcaaron]

Basically it gave errors because the withOnyx HOC calls in the tests don't have propTypes, although the component that it wraps has those defined...

I think it's ok to disable it in this case.

[chrispader]

Basically it gave errors because the withOnyx HOC calls in the tests don't have propTypes, although the component that it wraps has those defined...

I think it's ok to disable it in this case.

done 👍

[marcaaron]

Can we clean up the diff? There are a bunch of what look like new changes, but are actually recent changes to main.

[chrispader]

@marcaaron just updated the diff and replied to your questions :)

[marcaaron]

LGTM 👍

[marcaaron]

@chrispader Let's raise a PR in App to bump the version of Onyx there and do any final testing? Thanks so much for your patience and continued effort on this one. And thank you @kidroca for the additional thoughts and review 🙇

[chrispader]

Some issues that arose from this PR are fixed in this PR