From 26362027cb782a0daf1e3dd58fb941fcc894c57f Mon Sep 17 00:00:00 2001 From: Sam Willis Date: Sun, 6 Jul 2025 13:53:13 +0100 Subject: [PATCH 1/3] ensure that all transactions are given an id --- .changeset/open-foxes-say.md | 5 +++++ packages/db/src/transactions.ts | 20 +++++--------------- 2 files changed, 10 insertions(+), 15 deletions(-) create mode 100644 .changeset/open-foxes-say.md diff --git a/.changeset/open-foxes-say.md b/.changeset/open-foxes-say.md new file mode 100644 index 00000000..a1f248e5 --- /dev/null +++ b/.changeset/open-foxes-say.md @@ -0,0 +1,5 @@ +--- +"@tanstack/db": patch +--- + +Ensure that all transactions are given an id, fixes a potential bug with direct mutations diff --git a/packages/db/src/transactions.ts b/packages/db/src/transactions.ts index 5b5d0992..024c4a19 100644 --- a/packages/db/src/transactions.ts +++ b/packages/db/src/transactions.ts @@ -15,21 +15,8 @@ let transactionStack: Array> = [] export function createTransaction< TData extends object = Record, >(config: TransactionConfig): Transaction { - 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({ - ...config, - id: transactionId, - }) - + const newTransaction = new Transaction(config) transactions.push(newTransaction) - return newTransaction } @@ -74,7 +61,10 @@ export class Transaction< } constructor(config: TransactionConfig) { - 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 = [] From d65a4c257f4bdb2efe6ba3bd63f26647fbd5255b Mon Sep 17 00:00:00 2001 From: Sam Willis Date: Sun, 6 Jul 2025 15:01:36 +0100 Subject: [PATCH 2/3] add a sequence number to transactions for sorting --- .changeset/khaki-cougars-give.md | 5 +++++ packages/db/src/collection.ts | 4 ++-- packages/db/src/transactions.ts | 19 +++++++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 .changeset/khaki-cougars-give.md diff --git a/.changeset/khaki-cougars-give.md b/.changeset/khaki-cougars-give.md new file mode 100644 index 00000000..5aa49b8c --- /dev/null +++ b/.changeset/khaki-cougars-give.md @@ -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 diff --git a/packages/db/src/collection.ts b/packages/db/src/collection.ts index c2b54371..59e2ad56 100644 --- a/packages/db/src/collection.ts +++ b/packages/db/src/collection.ts @@ -277,8 +277,8 @@ export class CollectionImpl< throw new Error(`Collection requires a sync config`) } - this.transactions = new SortedMap>( - (a, b) => a.createdAt.getTime() - b.createdAt.getTime() + this.transactions = new SortedMap>((a, b) => + a.compareCreatedAt(b) ) this.config = config diff --git a/packages/db/src/transactions.ts b/packages/db/src/transactions.ts index 024c4a19..8e570b12 100644 --- a/packages/db/src/transactions.ts +++ b/packages/db/src/transactions.ts @@ -12,6 +12,8 @@ import type { const transactions: Array> = [] let transactionStack: Array> = [] +let sequenceNumber = 0 + export function createTransaction< TData extends object = Record, >(config: TransactionConfig): Transaction { @@ -54,6 +56,7 @@ export class Transaction< public isPersisted: Deferred> public autoCommit: boolean public createdAt: Date + public sequenceNumber: number public metadata: Record public error?: { message: string @@ -71,6 +74,7 @@ export class Transaction< this.isPersisted = createDeferred>() this.autoCommit = config.autoCommit ?? true this.createdAt = new Date() + this.sequenceNumber = sequenceNumber++ this.metadata = config.metadata ?? {} } @@ -200,4 +204,19 @@ 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): number { + const createdAtComparison = + this.createdAt.getTime() - other.createdAt.getTime() + if (createdAtComparison !== 0) { + return createdAtComparison + } + return this.sequenceNumber - other.sequenceNumber + } } From ef13a14b2ae85c6938522113a845a347b76a81f8 Mon Sep 17 00:00:00 2001 From: Sam Willis Date: Sun, 6 Jul 2025 16:28:02 +0100 Subject: [PATCH 3/3] use createTransaction in direct mutations, and only export the Transaction type --- packages/db/src/collection.ts | 11 ++++++----- packages/db/src/transactions.ts | 4 +++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/db/src/collection.ts b/packages/db/src/collection.ts index 59e2ad56..fc449c3d 100644 --- a/packages/db/src/collection.ts +++ b/packages/db/src/collection.ts @@ -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, @@ -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({ + const directOpTransaction = createTransaction({ mutationFn: async (params) => { // Call the onInsert handler with the transaction return this.config.onInsert!(params) @@ -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() @@ -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({ + const directOpTransaction = createTransaction({ mutationFn: async (params) => { // Call the onUpdate handler with the transaction return this.config.onUpdate!(params) @@ -1559,7 +1560,7 @@ export class CollectionImpl< } // Create a new transaction with a mutation function that calls the onDelete handler - const directOpTransaction = new Transaction({ + const directOpTransaction = createTransaction({ autoCommit: true, mutationFn: async (params) => { // Call the onDelete handler with the transaction diff --git a/packages/db/src/transactions.ts b/packages/db/src/transactions.ts index 8e570b12..ce544a47 100644 --- a/packages/db/src/transactions.ts +++ b/packages/db/src/transactions.ts @@ -45,7 +45,7 @@ function removeFromPendingList(tx: Transaction) { } } -export class Transaction< +class Transaction< T extends object = Record, TOperation extends OperationType = OperationType, > { @@ -220,3 +220,5 @@ export class Transaction< return this.sequenceNumber - other.sequenceNumber } } + +export type { Transaction }