multiMerge will *overwrite* key on iOS for '{}'

[irisjae]

What happened?

According to documentation, the function of multiMerge is Multiple merging of existing and new values in a batch. Assumes that values are stringified JSON. Once completed, invokes callback with errors (if any).

One expects that the empty object {} is the identity of the operation of merging objects, and that merging the JSON-ified object "{}" into any value that is already an object will produce no effect.

This is the case on Android and not the case on iOS (tested with 1.19.3). The following snippet reproduces this, producing '{ "x": 1 }' for key "a" on Android, and producing '{}' on iOS.

let { default: { multiMerge, getAllKeys, multiGet } } = require ('@react-native-async-storage/async-storage')

Promise .resolve ()
.then (_ => multiMerge ([ [ 'a', '{ "x": 1 }' ] ] ))
.then (_ => multiMerge ([ [ 'a', '{}' ] ] ))
.then (_ => getAllKeys ())
.then (keys => multiGet (keys))
.then (entries => Object .fromEntries (entries))
.then (result => console .log (result))

The bug is caused by this line

async-storage/packages/default-storage-backend/ios/RNCAsyncStorage.mm

Line 791 in 9d9bb1a

if (RCTMergeRecursive(mergedVal, RCTJSONParse(entry[1], &jsonError))) {

Presumably, the line is intended to overwrite the previous value if the provided input is not an object (i.e. not mergeable). What is actually specified is overwriting of the previous value if the provided input has no keys, which unfortunately produces this bug. The corresponding implementation in Android

async-storage/packages/default-storage-backend/android/src/main/java/com/reactnativecommunity/asyncstorage/next/StorageSupplier.kt

Line 61 in 9d9bb1a

JSONObject(oldEntry.value).mergeWith(JSONObject(newEntry.value)).toString()

properly (it seems) merges the values instead, and handles the case correctly.

Version

1.19.3

What platforms are you seeing this issue on?

• Android
• iOS
• macOS
• Windows
• web

System Information

.

Steps to Reproduce

Provided above

[github-actions]

This issue has been marked as stale due to inactivity. Please respond or otherwise resolve the issue within 7 days or it will be closed.

[irisjae]

I've worked around this, but this is not stale; I've attached a test case and the relevant code that appears to cause the issue.

This issue potentially causes data loss.

[irisjae]

I don't know about the maintainers of this project, but to me, unexpected data loss issues are not very encouraging signs. I'm happy to provide a PR if maintainers show interest.

[krizzu]

@irisjae Thanks for the details explanation, I'll try to find some time later next week to look into that 🙏

[AsyncStorageBot]

🎉 This issue has been resolved in version 1.22.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀