Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix minor issues after introducing incremental JSON_PATCH merge #284

Merged
merged 16 commits into from
Jul 27, 2023
10 changes: 6 additions & 4 deletions lib/Onyx.js
Original file line number Diff line number Diff line change
Expand Up @@ -900,7 +900,7 @@ function set(key, value) {
// This approach prioritizes fast UI changes without waiting for data to be stored in device storage.
broadcastUpdate(key, value, hasChanged, 'set');

// If the value has not changed then we don't need to write it to storage
// If the value has not changed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead.
if (!hasChanged) {
return Promise.resolve();
}
Expand Down Expand Up @@ -961,7 +961,8 @@ function applyMerge(existingValue, changes) {
if (_.isObject(existingValue) || _.every(changes, _.isObject)) {
// Object values are merged one after the other
// lodash adds a small overhead so we don't use it here
chrispader marked this conversation as resolved.
Show resolved Hide resolved
return _.reduce(changes, (modifiedData, change) => ({...fastMerge(modifiedData, change)}),
// eslint-disable-next-line prefer-object-spread, rulesdir/prefer-underscore-method
return _.reduce(changes, (modifiedData, change) => Object.assign({}, fastMerge(modifiedData, change)),
existingValue || {});
}

Expand Down Expand Up @@ -1012,7 +1013,8 @@ function merge(key, changes) {
// After that we merge the batched changes with the existing value
let modifiedData = applyMerge(existingValue, [batchedChanges]);

// For objects, we want to remove all top level keys that are null
// For objects, the key for null values needs to be removed from the object to ensure the value will get removed from storage completely.
// On native, SQLite will remove keys top-level keys that are null. To be consistent, we remove them on web too.
chrispader marked this conversation as resolved.
Show resolved Hide resolved
if (!_.isArray(modifiedData) && _.isObject(modifiedData)) {
modifiedData = _.omit(modifiedData, value => _.isNull(value));
}
Expand All @@ -1022,7 +1024,7 @@ function merge(key, changes) {
// This approach prioritizes fast UI changes without waiting for data to be stored in device storage.
broadcastUpdate(key, modifiedData, hasChanged, 'merge');

// If the value has not changed then we don't need to write it to storage
// If the value has not changed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead.
if (!hasChanged) {
return Promise.resolve();
}
Expand Down
Loading