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/khaki-cougars-give.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@tanstack/db": patch
---

add a sequence number to transactions to when sorting we can ensure that those created in the same ms are sorted in the correct order
5 changes: 5 additions & 0 deletions .changeset/open-foxes-say.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@tanstack/db": patch
---

Ensure that all transactions are given an id, fixes a potential bug with direct mutations
15 changes: 8 additions & 7 deletions packages/db/src/collection.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { Store } from "@tanstack/store"
import { withArrayChangeTracking, withChangeTracking } from "./proxy"
import { Transaction, getActiveTransaction } from "./transactions"
import { createTransaction, getActiveTransaction } from "./transactions"
import { SortedMap } from "./SortedMap"
import type { Transaction } from "./transactions"
import type {
ChangeListener,
ChangeMessage,
Expand Down Expand Up @@ -277,8 +278,8 @@ export class CollectionImpl<
throw new Error(`Collection requires a sync config`)
}

this.transactions = new SortedMap<string, Transaction<any>>(
(a, b) => a.createdAt.getTime() - b.createdAt.getTime()
this.transactions = new SortedMap<string, Transaction<any>>((a, b) =>
a.compareCreatedAt(b)
)

this.config = config
Expand Down Expand Up @@ -1242,7 +1243,7 @@ export class CollectionImpl<
return ambientTransaction
} else {
// Create a new transaction with a mutation function that calls the onInsert handler
const directOpTransaction = new Transaction<T>({
const directOpTransaction = createTransaction<T>({
mutationFn: async (params) => {
// Call the onInsert handler with the transaction
return this.config.onInsert!(params)
Expand Down Expand Up @@ -1444,7 +1445,7 @@ export class CollectionImpl<

// If no changes were made, return an empty transaction early
if (mutations.length === 0) {
const emptyTransaction = new Transaction({
const emptyTransaction = createTransaction({
mutationFn: async () => {},
})
emptyTransaction.commit()
Expand All @@ -1464,7 +1465,7 @@ export class CollectionImpl<
// No need to check for onUpdate handler here as we've already checked at the beginning

// Create a new transaction with a mutation function that calls the onUpdate handler
const directOpTransaction = new Transaction<T>({
const directOpTransaction = createTransaction<T>({
mutationFn: async (params) => {
// Call the onUpdate handler with the transaction
return this.config.onUpdate!(params)
Expand Down Expand Up @@ -1559,7 +1560,7 @@ export class CollectionImpl<
}

// Create a new transaction with a mutation function that calls the onDelete handler
const directOpTransaction = new Transaction<T>({
const directOpTransaction = createTransaction<T>({
autoCommit: true,
mutationFn: async (params) => {
// Call the onDelete handler with the transaction
Expand Down
43 changes: 27 additions & 16 deletions packages/db/src/transactions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,13 @@ import type {
const transactions: Array<Transaction<any>> = []
let transactionStack: Array<Transaction<any>> = []

let sequenceNumber = 0

export function createTransaction<
TData extends object = Record<string, unknown>,
>(config: TransactionConfig<TData>): Transaction<TData> {
if (typeof config.mutationFn === `undefined`) {
throw `mutationFn is required when creating a transaction`
}

let transactionId = config.id
if (!transactionId) {
transactionId = crypto.randomUUID()
}
const newTransaction = new Transaction<TData>({
...config,
id: transactionId,
})

const newTransaction = new Transaction<TData>(config)
transactions.push(newTransaction)

return newTransaction
}

Expand All @@ -56,7 +45,7 @@ function removeFromPendingList(tx: Transaction<any>) {
}
}

export class Transaction<
class Transaction<
T extends object = Record<string, unknown>,
TOperation extends OperationType = OperationType,
> {
Expand All @@ -67,20 +56,25 @@ export class Transaction<
public isPersisted: Deferred<Transaction<T, TOperation>>
public autoCommit: boolean
public createdAt: Date
public sequenceNumber: number
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice!

On new Transaction — we could just only export the type so people can't use it. It was definitely my mistake using new Transaction.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

agreed, I changed it to only export the type, and changed the direct mutations to use createTransaction.

public metadata: Record<string, unknown>
public error?: {
message: string
error: Error
}

constructor(config: TransactionConfig<T>) {
this.id = config.id!
if (typeof config.mutationFn === `undefined`) {
throw `mutationFn is required when creating a transaction`
}
this.id = config.id ?? crypto.randomUUID()
this.mutationFn = config.mutationFn
this.state = `pending`
this.mutations = []
this.isPersisted = createDeferred<Transaction<T, TOperation>>()
this.autoCommit = config.autoCommit ?? true
this.createdAt = new Date()
this.sequenceNumber = sequenceNumber++
this.metadata = config.metadata ?? {}
}

Expand Down Expand Up @@ -210,4 +204,21 @@ export class Transaction<

return this
}

/**
* Compare two transactions by their createdAt time and sequence number in order
* to sort them in the order they were created.
* @param other - The other transaction to compare to
* @returns -1 if this transaction was created before the other, 1 if it was created after, 0 if they were created at the same time
*/
compareCreatedAt(other: Transaction<any>): number {
const createdAtComparison =
this.createdAt.getTime() - other.createdAt.getTime()
if (createdAtComparison !== 0) {
return createdAtComparison
}
return this.sequenceNumber - other.sequenceNumber
}
}

export type { Transaction }