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/clarify-local-write-status.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@tanstack/db': patch
---

Clarify local write status documentation for `$synced` and `isPersisted.promise`, and add core coverage for queued ambiguous server-key sync while optimistic temp-key inserts are pending.
7 changes: 6 additions & 1 deletion packages/db/src/collection/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,12 @@ export class CollectionStateManager<
}

/**
* Checks if a row has pending optimistic mutations (not yet confirmed by sync).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I find "(not yet confirmed by sync)" clearer than "in the visible projection"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

One of the clarifications this PR is trying to make is that not all "confirmations by the collection" means that the write has been persisted to the backend e.g. powersync just confirms when the write is in the local sqlite. I pushed this commit which hopefully is clearer 4bbc268

* Checks whether this row currently has no pending local optimistic writes.
*
* This is local mutation status, not backend confirmation: `true` means the
* row is not currently affected by an optimistic transaction in this
* collection's visible state.
*
* Used to compute the $synced virtual property.
*/
public isRowSynced(key: TKey): boolean {
Expand Down
12 changes: 12 additions & 0 deletions packages/db/src/transactions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,18 @@ class Transaction<T extends object = Record<string, unknown>> {
public state: TransactionState
public mutationFn: MutationFn<T>
public mutations: Array<PendingMutation<T>>
/**
* Deferred that settles when this transaction settles.
*
* Await `isPersisted.promise`, not `isPersisted` itself. The promise resolves
* when the transaction completes successfully and rejects if the transaction
* fails or is rolled back.
*
* For non-empty commits, the mutation function is the normal settlement
* boundary. This does not inherently prove that a backend has uploaded,
* confirmed, or read back the write unless the mutation function waits for
* that backend observation before returning.
*/
public isPersisted: Deferred<Transaction<T>>
public autoCommit: boolean
public createdAt: Date
Expand Down
15 changes: 10 additions & 5 deletions packages/db/src/virtual-props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export type VirtualOrigin = 'local' | 'remote'
* // Accessing virtual properties on a row
* const user = collection.get('user-1')
* if (user.$synced) {
* console.log('Confirmed by backend')
* console.log('No pending local optimistic writes for this row')
* }
* if (user.$origin === 'local') {
* console.log('Created/modified locally')
Expand All @@ -47,7 +47,7 @@ export type VirtualOrigin = 'local' | 'remote'
* @example
* ```typescript
* // Using virtual properties in queries
* const confirmedOrders = createLiveQueryCollection({
* const ordersWithoutLocalWrites = createLiveQueryCollection({
* query: (q) => q
* .from({ order: orders })
* .where(({ order }) => eq(order.$synced, true))
Expand All @@ -58,10 +58,15 @@ export interface VirtualRowProps<
TKey extends string | number = string | number,
> {
/**
* Whether this row reflects confirmed state from the backend.
* Whether this row currently has no pending local optimistic writes.
*
* - `true`: Row is confirmed by the backend (no pending optimistic mutations)
* - `false`: Row has pending optimistic mutations that haven't been confirmed
* - `true`: No pending local optimistic mutation currently affects this row
* - `false`: One or more pending local optimistic mutations currently affect this row
*
* This is local mutation status. It does not prove that a backend has uploaded,
* confirmed, or read back the row. If you need backend-confirmed status, keep
* your mutation function pending until that backend observation has happened,
* or expose adapter-specific status.
*
* For local-only collections (no sync), this is always `true`.
* For live query collections, this is passed through from the source collection.
Expand Down
7 changes: 6 additions & 1 deletion packages/db/tests/collection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe(`Collection`, () => {
).toThrow(DuplicateKeySyncError)
})

it(`removes optimistic insert when sync confirms with a different server-generated key`, async () => {
it(`keeps ambiguous server-key sync queued while a temp-key optimistic insert is pending`, async () => {
const options = mockSyncCollectionOptionsNoInitialState<{
id: number
text: string
Expand All @@ -102,6 +102,11 @@ describe(`Collection`, () => {
options.utils.commit()

// The sync commit is held while the local insert transaction is persisting.
// Without an explicit temp-key -> server-key mapping, core cannot know
// whether key 24 is this optimistic insert's server echo or an unrelated
// row, so it must not expose both rows at the same time.
expect(tx.isPersisted.isPending()).toBe(true)
expect(collection.has(24)).toBe(false)
expect(getStateEntries(collection)).toEqual([
[4733, { id: 4733, text: `two` }],
])
Expand Down
Loading