Skip to content
New issue

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

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

Already on GitHub? # to your account

feat(persist): Deep merge records in batchUpdate #3386

Merged
merged 13 commits into from
Jan 31, 2025

Conversation

nalanj
Copy link
Contributor

@nalanj nalanj commented Jan 28, 2025

Currently batchUpdate only shallow merges. This switches it to deep merge, as request by some customers.

I want to get @bastienbeurier's take on docs strategy here, but code is ready for review.

See https://linear.app/nango/issue/NAN-1982/partial-records-update

How I tested it

  • Wrote tests
  • Wrote a sync that dumped a lot of objects into the store and then used batchUpdate in a subsequent run to merge them.

@nalanj nalanj self-assigned this Jan 29, 2025
@nalanj nalanj requested a review from a team January 29, 2025 18:11
@@ -59,7 +59,7 @@ describe('Supervisor', () => {
void supervisor2.start();

await vi.waitUntil(() => tickSpy1.mock.calls.length > 0, {
timeout: 5000
timeout: 10000
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had this one fail on me in dev again, so I went ahead and bumped it up a good bit more.

@nalanj nalanj requested review from TBonnin and a team January 29, 2025 20:10
@nalanj nalanj marked this pull request as ready for review January 29, 2025 20:11
Copy link
Collaborator

@bodinsamuel bodinsamuel left a comment

Choose a reason for hiding this comment

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

Looks good and simple 👍🏻
I'm not sure it's really clear for an end user, there is no way to optin or optout, this might break some expectation (or be unnoticed)

@nalanj nalanj merged commit 17e4ece into master Jan 31, 2025
17 checks passed
@nalanj nalanj deleted the alan/update-records-deep-merge branch January 31, 2025 15:45
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants