Skip to content

updateSnapshots - remove picking only existing props #638

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.

Sign up for GitHub

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

Merged
iwiznia merged 2 commits into from
May 28, 2025
Merged

updateSnapshots - remove picking only existing props #638

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a2ba002
remove picking only existing props
FitseTLT May 26, 2025
dfe80ea
added test
FitseTLT May 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions lib/OnyxUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import {deepEqual} from 'fast-equals';
import lodashClone from 'lodash/clone';
import type {ValueOf} from 'type-fest';
import lodashPick from 'lodash/pick';
import DevTools from './DevTools';
import * as Logger from './Logger';
import type Onyx from './Onyx';
Expand Down Expand Up @@ -1417,9 +1416,8 @@ function updateSnapshots(data: OnyxUpdate[], mergeFn: typeof Onyx.merge): Array<
}

const oldValue = updatedData[key] || {};
const newValue = lodashPick(value, Object.keys(snapshotData[key]));

updatedData = {...updatedData, [key]: Object.assign(oldValue, newValue)};
updatedData = {...updatedData, [key]: Object.assign(oldValue, value)};
});

// Skip the update if there's no data to be merged
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/onyxTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1448,7 +1448,7 @@ describe('Onyx', () => {
const snapshot1 = `${ONYX_KEYS.COLLECTION.SNAPSHOT}1`;

const initialValue = {name: 'Fluffy'};
const finalValue = {name: 'Kitty'};
const finalValue = {name: 'Kitty', nickName: 'Fitse'};
Copy link
Copy Markdown
Contributor

@iwiznia iwiznia May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite get it, how is this change exercising the changed code?

Copy link
Copy Markdown
Member

@parasharrajat parasharrajat May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Earlier, if we couldn't add new keys(change the Object structure) to the existing values in snapshots in Onyx. But now we can do that. Here in this test, a new key is now added which confirms that change work and allows adding new keys to the existing items in snapshots.

@iwiznia

Copy link
Copy Markdown
Contributor Author

@FitseTLT FitseTLT May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nickName is a new prop that didn't exist in intialValue. Evidence: if u test it on main it will fail.

Copy link
Copy Markdown
Contributor

@iwiznia iwiznia May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah got it! Thanks for clarifying


await Onyx.set(cat, initialValue);
await Onyx.set(snapshot1, {data: {[cat]: initialValue}});
Expand Down