Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/eleven-towns-carry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@tanstack/electric-db-collection": patch
---

fix the handling of an electric must-refetch message so that the truncate is handled in the same transaction as the next up-to-date, ensuring you don't get a momentary empty collection.
17 changes: 13 additions & 4 deletions packages/electric-db-collection/src/electric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -476,17 +476,27 @@ function createElectricSync<T extends Row<unknown>>(

return {
sync: (params: Parameters<SyncConfig<T>[`sync`]>[0]) => {
const { begin, write, commit, markReady, truncate } = params
const { begin, write, commit, markReady, truncate, collection } = params
const stream = new ShapeStream({
...shapeOptions,
signal: abortController.signal,
onError: (errorParams) => {
// Just immediately mark ready if there's an error to avoid blocking
// apps waiting for `.preload()` to finish.
// Note that Electric sends a 409 error on a `must-refetch` message, but the
// ShapeStream handled this and it will not reach this handler, therefor
// this markReady will not be triggers by a `must-refetch`.
markReady()

if (shapeOptions.onError) {
return shapeOptions.onError(errorParams)
} else {
console.error(
`An error occurred while syncing collection: ${collection.id}, \n` +
`it has been marked as ready to avoid blocking apps waiting for '.preload()' to finish. \n` +
`You can provide an 'onError' handler on the shapeOptions to handle this error, and this message will not be logged.`,
errorParams
)
Comment on lines +493 to +499
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@KyleAMathews I have added this logging here so that a user is alerted to the fact the collection has been marked ready on an error. You added the original code to do this and I wanted to check your thinking.

}

return
Expand Down Expand Up @@ -540,9 +550,8 @@ function createElectricSync<T extends Row<unknown>>(

truncate()

// Commit the truncate transaction immediately
commit()
transactionStarted = false
// Reset hasUpToDate so we continue accumulating changes until next up-to-date
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are no longer committing it here, when will it be committed and where?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At the next up-to-date from electric, line 558 just below.

hasUpToDate = false
}
}

Expand Down
6 changes: 4 additions & 2 deletions packages/electric-db-collection/tests/electric.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,10 @@ describe(`Electric Integration`, () => {
},
])

// The collection should be cleared but remain in ready state
expect(collection.state.size).toBe(0)
// The collection should still have old data because truncate is in pending
// transaction. This is the intended behavior of the collection, you should have
// the old data until the next up-to-date message.
expect(collection.state.size).toBe(2)
expect(collection.status).toBe(`ready`)

// Send new data after must-refetch
Expand Down
Loading