From dc5a1b3b1a74da83a7b900cabacf42a3a66dc93f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Nov 2025 15:01:17 +0000 Subject: [PATCH 1/7] refactor: migrate paced mutations to @tanstack/pacer-lite Replace @tanstack/pacer with @tanstack/pacer-lite for paced mutations implementation. The lite version provides the same core functionality with minimal overhead and no external dependencies, making it more suitable for library use. Changes: - Update package.json to use @tanstack/pacer-lite ^0.1.0 - Replace Debouncer with LiteDebouncer in debounceStrategy - Replace Throttler with LiteThrottler in throttleStrategy - Replace AsyncQueuer with LiteQueuer in queueStrategy - Implement manual promise chaining in queueStrategy to maintain async serialization behavior All tests pass and the build completes successfully. --- package.json | 1 + packages/db/package.json | 2 +- .../db/src/strategies/debounceStrategy.ts | 4 +-- packages/db/src/strategies/queueStrategy.ts | 27 ++++++++++------ .../db/src/strategies/throttleStrategy.ts | 4 +-- pnpm-lock.yaml | 31 ++++++------------- 6 files changed, 34 insertions(+), 35 deletions(-) diff --git a/package.json b/package.json index 367afe0f9..93d2ba5d6 100644 --- a/package.json +++ b/package.json @@ -99,6 +99,7 @@ } }, "dependencies": { + "@tanstack/pacer-lite": "^0.1.0", "tsx": "^4.20.6" } } diff --git a/packages/db/package.json b/packages/db/package.json index 68781818c..ac7a236e8 100644 --- a/packages/db/package.json +++ b/packages/db/package.json @@ -5,7 +5,7 @@ "dependencies": { "@standard-schema/spec": "^1.0.0", "@tanstack/db-ivm": "workspace:*", - "@tanstack/pacer": "^0.16.3" + "@tanstack/pacer-lite": "^0.1.0" }, "devDependencies": { "@vitest/coverage-istanbul": "^3.2.4", diff --git a/packages/db/src/strategies/debounceStrategy.ts b/packages/db/src/strategies/debounceStrategy.ts index 8f0cd6cc1..b73b6a386 100644 --- a/packages/db/src/strategies/debounceStrategy.ts +++ b/packages/db/src/strategies/debounceStrategy.ts @@ -1,4 +1,4 @@ -import { Debouncer } from "@tanstack/pacer/debouncer" +import { LiteDebouncer } from "@tanstack/pacer-lite/lite-debouncer" import type { DebounceStrategy, DebounceStrategyOptions } from "./types" import type { Transaction } from "../transactions" @@ -28,7 +28,7 @@ import type { Transaction } from "../transactions" export function debounceStrategy( options: DebounceStrategyOptions ): DebounceStrategy { - const debouncer = new Debouncer( + const debouncer = new LiteDebouncer( (callback: () => Transaction) => callback(), options ) diff --git a/packages/db/src/strategies/queueStrategy.ts b/packages/db/src/strategies/queueStrategy.ts index 10c8669db..7f9887333 100644 --- a/packages/db/src/strategies/queueStrategy.ts +++ b/packages/db/src/strategies/queueStrategy.ts @@ -1,4 +1,4 @@ -import { AsyncQueuer } from "@tanstack/pacer/async-queuer" +import { LiteQueuer } from "@tanstack/pacer-lite/lite-queuer" import type { QueueStrategy, QueueStrategyOptions } from "./types" import type { Transaction } from "../transactions" @@ -44,16 +44,25 @@ import type { Transaction } from "../transactions" * ``` */ export function queueStrategy(options?: QueueStrategyOptions): QueueStrategy { - const queuer = new AsyncQueuer<() => Transaction>( - async (fn) => { - const transaction = fn() - // Wait for the transaction to be persisted before processing next item - // Note: fn() already calls commit(), we just wait for it to complete - await transaction.isPersisted.promise + // Track the current promise chain to ensure serialization + let processingChain = Promise.resolve() + + const queuer = new LiteQueuer<() => Transaction>( + (fn) => { + // Chain each transaction processing to ensure serialization + processingChain = processingChain + .then(async () => { + const transaction = fn() + // Wait for the transaction to be persisted before processing next item + await transaction.isPersisted.promise + }) + .catch(() => { + // Errors are handled via transaction.isPersisted.promise + // This catch prevents unhandled promise rejections + }) }, { - concurrency: 1, // Process one at a time to ensure serialization - wait: options?.wait, + wait: options?.wait ?? 0, maxSize: options?.maxSize, addItemsTo: options?.addItemsTo ?? `back`, // Default FIFO: add to back getItemsFrom: options?.getItemsFrom ?? `front`, // Default FIFO: get from front diff --git a/packages/db/src/strategies/throttleStrategy.ts b/packages/db/src/strategies/throttleStrategy.ts index 2a274fd37..e8f3f543d 100644 --- a/packages/db/src/strategies/throttleStrategy.ts +++ b/packages/db/src/strategies/throttleStrategy.ts @@ -1,4 +1,4 @@ -import { Throttler } from "@tanstack/pacer/throttler" +import { LiteThrottler } from "@tanstack/pacer-lite/lite-throttler" import type { ThrottleStrategy, ThrottleStrategyOptions } from "./types" import type { Transaction } from "../transactions" @@ -48,7 +48,7 @@ import type { Transaction } from "../transactions" export function throttleStrategy( options: ThrottleStrategyOptions ): ThrottleStrategy { - const throttler = new Throttler( + const throttler = new LiteThrottler( (callback: () => Transaction) => callback(), options ) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3b1c50ea4..959c60fc0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -25,6 +25,9 @@ importers: .: dependencies: + '@tanstack/pacer-lite': + specifier: ^0.1.0 + version: 0.1.0 tsx: specifier: ^4.20.6 version: 4.20.6 @@ -603,7 +606,7 @@ importers: version: 0.44.7(@opentelemetry/api@1.9.0)(@types/pg@8.15.6)(gel@2.1.1)(kysely@0.28.5)(pg@8.16.3)(postgres@3.4.7) drizzle-zod: specifier: ^0.8.3 - version: 0.8.3(drizzle-orm@0.44.7(@opentelemetry/api@1.9.0)(@types/pg@8.15.6)(gel@2.1.1)(kysely@0.28.5)(pg@8.16.3)(postgres@3.4.7))(zod@3.25.76) + version: 0.8.3(drizzle-orm@0.44.7(@opentelemetry/api@1.9.0)(@types/pg@8.15.6)(gel@2.1.1)(kysely@0.28.5)(pg@8.16.3)(postgres@3.4.7))(zod@4.1.11) express: specifier: ^4.21.2 version: 4.21.2 @@ -717,9 +720,9 @@ importers: '@tanstack/db-ivm': specifier: workspace:* version: link:../db-ivm - '@tanstack/pacer': - specifier: ^0.16.3 - version: 0.16.3 + '@tanstack/pacer-lite': + specifier: ^0.1.0 + version: 0.1.0 typescript: specifier: '>=4.7' version: 5.9.3 @@ -3644,10 +3647,6 @@ packages: resolution: {integrity: sha512-7Wwfw6wBv2Kc+OBNIJQzBSJ6q7GABtwVT+VOQ/7/Gl7z8z1rtEYUZrxUrNvbbrHY+J5/WNZNZjJjTWDf8nTUBw==} engines: {node: '>=18'} - '@tanstack/devtools-event-client@0.3.5': - resolution: {integrity: sha512-RL1f5ZlfZMpghrCIdzl6mLOFLTuhqmPNblZgBaeKfdtk5rfbjykurv+VfYydOFXj0vxVIoA2d/zT7xfD7Ph8fw==} - engines: {node: '>=18'} - '@tanstack/directive-functions-plugin@1.134.5': resolution: {integrity: sha512-J3oawV8uBRBbPoLgMdyHt+LxzTNuWRKNJJuCLWsm/yq6v0IQSvIVCgfD2+liIiSnDPxGZ8ExduPXy8IzS70eXw==} engines: {node: '>=12'} @@ -3662,8 +3661,8 @@ packages: resolution: {integrity: sha512-B7+x7eP2FFvi3fgd3rNH9o/Eixt+pp0zCIdGhnQbAJjFrlwIKGjGnwyJjhWJ5fMQlGks/E2LdDTqEV4W9Plx7g==} engines: {node: '>=12'} - '@tanstack/pacer@0.16.3': - resolution: {integrity: sha512-hJGPODkjuUEncwHsFacLY6W5E7lmEU2FMf4Mh0kuxqUx3UsuneQX6ctRpoHBLlgdb7sqDieIaslQnivG3OAZ+A==} + '@tanstack/pacer-lite@0.1.0': + resolution: {integrity: sha512-a5A0PI0H4npUy7u3VOjOhdynXnRBna+mDvpt8ghDCVzS3Tgn8DlGzHlRqS2rKJP8ZcLuVO2qxlIIblhcoaiv8Q==} engines: {node: '>=18'} '@tanstack/publish-config@0.2.1': @@ -11633,8 +11632,6 @@ snapshots: - typescript - vite - '@tanstack/devtools-event-client@0.3.5': {} - '@tanstack/directive-functions-plugin@1.134.5(vite@6.4.1(@types/node@24.7.0)(jiti@2.6.1)(lightningcss@1.30.2)(sass@1.90.0)(terser@5.44.0)(tsx@4.20.6)(yaml@2.8.1))': dependencies: '@babel/code-frame': 7.27.1 @@ -11681,10 +11678,7 @@ snapshots: '@tanstack/history@1.133.28': {} - '@tanstack/pacer@0.16.3': - dependencies: - '@tanstack/devtools-event-client': 0.3.5 - '@tanstack/store': 0.8.0 + '@tanstack/pacer-lite@0.1.0': {} '@tanstack/publish-config@0.2.1': dependencies: @@ -13710,11 +13704,6 @@ snapshots: pg: 8.16.3 postgres: 3.4.7 - drizzle-zod@0.8.3(drizzle-orm@0.44.7(@opentelemetry/api@1.9.0)(@types/pg@8.15.6)(gel@2.1.1)(kysely@0.28.5)(pg@8.16.3)(postgres@3.4.7))(zod@3.25.76): - dependencies: - drizzle-orm: 0.44.7(@opentelemetry/api@1.9.0)(@types/pg@8.15.6)(gel@2.1.1)(kysely@0.28.5)(pg@8.16.3)(postgres@3.4.7) - zod: 3.25.76 - drizzle-zod@0.8.3(drizzle-orm@0.44.7(@opentelemetry/api@1.9.0)(@types/pg@8.15.6)(gel@2.1.1)(kysely@0.28.5)(pg@8.16.3)(postgres@3.4.7))(zod@4.1.11): dependencies: drizzle-orm: 0.44.7(@opentelemetry/api@1.9.0)(@types/pg@8.15.6)(gel@2.1.1)(kysely@0.28.5)(pg@8.16.3)(postgres@3.4.7) From b14e8dbaff44d98235f5cab13c9beb0bfab5bc19 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Nov 2025 15:13:49 +0000 Subject: [PATCH 2/7] chore: add changeset for pacer-lite migration --- .changeset/migrate-to-pacer-lite.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/migrate-to-pacer-lite.md diff --git a/.changeset/migrate-to-pacer-lite.md b/.changeset/migrate-to-pacer-lite.md new file mode 100644 index 000000000..a4d9674c5 --- /dev/null +++ b/.changeset/migrate-to-pacer-lite.md @@ -0,0 +1,5 @@ +--- +"@tanstack/db": patch +--- + +Migrated paced mutations implementation from `@tanstack/pacer` to `@tanstack/pacer-lite`. The lite version provides the same core functionality with minimal overhead and no external dependencies, making it more suitable for library use. This is an internal implementation change with no impact on the public API - all paced mutation strategies (debounce, throttle, queue) continue to work exactly as before. From a61ab945b64dfe68502ad1756f4f8a772910491c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Nov 2025 15:16:39 +0000 Subject: [PATCH 3/7] chore: remove @tanstack/pacer-lite from root package.json The dependency is only needed in @tanstack/db package. --- package.json | 1 - pnpm-lock.yaml | 3 --- 2 files changed, 4 deletions(-) diff --git a/package.json b/package.json index 93d2ba5d6..367afe0f9 100644 --- a/package.json +++ b/package.json @@ -99,7 +99,6 @@ } }, "dependencies": { - "@tanstack/pacer-lite": "^0.1.0", "tsx": "^4.20.6" } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 959c60fc0..11c98d87e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -25,9 +25,6 @@ importers: .: dependencies: - '@tanstack/pacer-lite': - specifier: ^0.1.0 - version: 0.1.0 tsx: specifier: ^4.20.6 version: 4.20.6 From 556e9206d01d87e0912ada4702a4eba3a715de82 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Nov 2025 15:35:22 +0000 Subject: [PATCH 4/7] test: add core unit tests for paced mutations Add comprehensive unit tests for createPacedMutations and all three strategies (debounce, throttle, queue) that test the core functionality directly without React dependencies. Tests cover: - Debounce strategy with timer reset and leading edge execution - Throttle strategy with leading/trailing edge behavior - Queue strategy with sequential processing and serialization - Error handling for failed transactions - Transaction batching and merging behavior These tests verify the pacer-lite migration maintains correct behavior. --- packages/db/tests/paced-mutations.test.ts | 625 ++++++++++++++++++++++ 1 file changed, 625 insertions(+) create mode 100644 packages/db/tests/paced-mutations.test.ts diff --git a/packages/db/tests/paced-mutations.test.ts b/packages/db/tests/paced-mutations.test.ts new file mode 100644 index 000000000..8d5381096 --- /dev/null +++ b/packages/db/tests/paced-mutations.test.ts @@ -0,0 +1,625 @@ +import { describe, expect, it, vi } from "vitest" +import { createCollection } from "../src/collection" +import { createPacedMutations } from "../src/paced-mutations" +import { + debounceStrategy, + queueStrategy, + throttleStrategy, +} from "../src/strategies" +import { mockSyncCollectionOptionsNoInitialState } from "./utils" + +type Item = { + id: number + value: number +} + +describe(`createPacedMutations`, () => { + describe(`with debounce strategy`, () => { + it(`should batch multiple rapid mutations into a single transaction`, async () => { + const mutationFn = vi.fn(async () => {}) + + const collection = createCollection( + mockSyncCollectionOptionsNoInitialState({ + id: `test`, + getKey: (item) => item.id, + }) + ) + + // Setup collection + const preloadPromise = collection.preload() + collection.utils.begin() + collection.utils.commit() + collection.utils.markReady() + await preloadPromise + + let insertCount = 0 + const mutate = createPacedMutations<{ id: number; value: number }>({ + onMutate: (item) => { + if (insertCount === 0) { + collection.insert(item) + insertCount++ + } else { + collection.update(item.id, (draft) => { + draft.value = item.value + }) + } + }, + mutationFn, + strategy: debounceStrategy({ wait: 50 }), + }) + + // Trigger three rapid mutations (all within 50ms debounce window) + const tx1 = mutate({ id: 1, value: 1 }) + const tx2 = mutate({ id: 1, value: 2 }) + const tx3 = mutate({ id: 1, value: 3 }) + + // All three calls should return the SAME transaction object + expect(tx1).toBe(tx2) + expect(tx2).toBe(tx3) + + // Mutations get auto-merged (insert + updates on same key = single insert with final value) + expect(tx1.mutations).toHaveLength(1) + expect(tx1.mutations[0]).toMatchObject({ + type: `insert`, + changes: { id: 1, value: 3 }, // Final merged value + }) + + // mutationFn should NOT have been called yet (still debouncing) + expect(mutationFn).not.toHaveBeenCalled() + + // Wait for debounce period + await new Promise((resolve) => setTimeout(resolve, 100)) + + // Now mutationFn should have been called ONCE with the merged mutation + expect(mutationFn).toHaveBeenCalledTimes(1) + expect(mutationFn).toHaveBeenCalledWith({ + transaction: expect.objectContaining({ + mutations: [ + expect.objectContaining({ + type: `insert`, + changes: { id: 1, value: 3 }, + }), + ], + }), + }) + + // Transaction should be completed + expect(tx1.state).toBe(`completed`) + }) + + it(`should reset debounce timer on each new mutation`, async () => { + const mutationFn = vi.fn(async () => {}) + + const collection = createCollection( + mockSyncCollectionOptionsNoInitialState({ + id: `test`, + getKey: (item) => item.id, + }) + ) + + const preloadPromise = collection.preload() + collection.utils.begin() + collection.utils.commit() + collection.utils.markReady() + await preloadPromise + + let insertCount = 0 + const mutate = createPacedMutations<{ id: number; value: number }>({ + onMutate: (item) => { + if (insertCount === 0) { + collection.insert(item) + insertCount++ + } else { + collection.update(item.id, (draft) => { + draft.value = item.value + }) + } + }, + mutationFn, + strategy: debounceStrategy({ wait: 50 }), + }) + + // First mutation at t=0 + mutate({ id: 1, value: 1 }) + + // Wait 40ms (still within 50ms debounce window) + await new Promise((resolve) => setTimeout(resolve, 40)) + + // mutationFn should NOT have been called yet + expect(mutationFn).not.toHaveBeenCalled() + + // Second mutation at t=40 (resets the timer) + mutate({ id: 1, value: 2 }) + + // Wait another 40ms (t=80, but only 40ms since last mutation) + await new Promise((resolve) => setTimeout(resolve, 40)) + + // mutationFn still should NOT have been called (timer was reset) + expect(mutationFn).not.toHaveBeenCalled() + + // Wait another 20ms (t=100, now 60ms since last mutation, past the 50ms debounce) + await new Promise((resolve) => setTimeout(resolve, 20)) + + // NOW mutationFn should have been called + expect(mutationFn).toHaveBeenCalledTimes(1) + expect(mutationFn.mock.calls[0][0].transaction.mutations).toHaveLength(1) + }) + + it(`should execute on leading edge when leading: true`, async () => { + const mutationFn = vi.fn(async () => {}) + + const collection = createCollection( + mockSyncCollectionOptionsNoInitialState({ + id: `test`, + getKey: (item) => item.id, + }) + ) + + const preloadPromise = collection.preload() + collection.utils.begin() + collection.utils.commit() + collection.utils.markReady() + await preloadPromise + + const mutate = createPacedMutations({ + onMutate: (item) => { + collection.insert(item) + }, + mutationFn, + strategy: debounceStrategy({ wait: 50, leading: true }), + }) + + // First mutation should execute immediately with leading: true + const tx1 = mutate({ id: 1, value: 1 }) + + // Small delay for immediate execution + await new Promise((resolve) => setTimeout(resolve, 10)) + + // Should have been called immediately (leading edge) + expect(mutationFn).toHaveBeenCalledTimes(1) + expect(tx1.state).toBe(`completed`) + }) + }) + + describe(`with throttle strategy`, () => { + it(`should throttle mutations with leading and trailing execution`, async () => { + const mutationFn = vi.fn(async () => {}) + + const collection = createCollection( + mockSyncCollectionOptionsNoInitialState({ + id: `test`, + getKey: (item) => item.id, + }) + ) + + // Setup collection + const preloadPromise = collection.preload() + collection.utils.begin() + collection.utils.commit() + collection.utils.markReady() + await preloadPromise + + const mutate = createPacedMutations({ + onMutate: (item) => { + collection.insert(item) + }, + mutationFn, + strategy: throttleStrategy({ + wait: 100, + leading: true, + trailing: true, + }), + }) + + // First mutation at t=0 (should execute immediately due to leading: true) + const tx1 = mutate({ id: 1, value: 1 }) + + // Leading edge should execute immediately + await new Promise((resolve) => setTimeout(resolve, 10)) + expect(mutationFn).toHaveBeenCalledTimes(1) + expect(tx1.state).toBe(`completed`) + + // Second mutation at t=20 (during throttle period, should batch) + const tx2 = mutate({ id: 2, value: 2 }) + + // Third mutation at t=30 (during throttle period, should batch with second) + await new Promise((resolve) => setTimeout(resolve, 10)) + const tx3 = mutate({ id: 3, value: 3 }) + + // tx2 and tx3 should be the same transaction (batched) + expect(tx2).toBe(tx3) + + // Still only 1 call (waiting for throttle period to end) + expect(mutationFn).toHaveBeenCalledTimes(1) + + // Wait for throttle period to complete (100ms from first mutation) + await new Promise((resolve) => setTimeout(resolve, 110)) + + // Trailing edge should have executed + expect(mutationFn).toHaveBeenCalledTimes(2) + expect(tx2.state).toBe(`completed`) + expect(tx3.state).toBe(`completed`) + + // Verify the batched transaction has 2 inserts + expect(tx2.mutations).toHaveLength(2) + }) + + it(`should respect leading: false option`, async () => { + const mutationFn = vi.fn(async () => {}) + + const collection = createCollection( + mockSyncCollectionOptionsNoInitialState({ + id: `test`, + getKey: (item) => item.id, + }) + ) + + const preloadPromise = collection.preload() + collection.utils.begin() + collection.utils.commit() + collection.utils.markReady() + await preloadPromise + + const mutate = createPacedMutations({ + onMutate: (item) => { + collection.insert(item) + }, + mutationFn, + strategy: throttleStrategy({ + wait: 50, + leading: false, + trailing: true, + }), + }) + + // First mutation should NOT execute immediately with leading: false + const tx1 = mutate({ id: 1, value: 1 }) + + // Wait for throttle period to complete + await new Promise((resolve) => setTimeout(resolve, 70)) + + // Now trailing edge should have executed + expect(mutationFn).toHaveBeenCalledTimes(1) + await tx1.isPersisted.promise + expect(tx1.state).toBe(`completed`) + }) + }) + + describe(`with queue strategy`, () => { + it(`should process mutations sequentially`, async () => { + const mutationFn = vi.fn(async () => { + // Quick execution + await new Promise((resolve) => setTimeout(resolve, 5)) + }) + + const collection = createCollection( + mockSyncCollectionOptionsNoInitialState({ + id: `test`, + getKey: (item) => item.id, + }) + ) + + // Setup collection + const preloadPromise = collection.preload() + collection.utils.begin() + collection.utils.commit() + collection.utils.markReady() + await preloadPromise + + const mutate = createPacedMutations({ + onMutate: (item) => { + collection.insert(item) + }, + mutationFn, + strategy: queueStrategy({ wait: 10 }), + }) + + // Trigger rapid mutations - queue creates separate transactions + const tx1 = mutate({ id: 1, value: 1 }) + const tx2 = mutate({ id: 2, value: 2 }) + const tx3 = mutate({ id: 3, value: 3 }) + + // Each should be a different transaction + expect(tx1).not.toBe(tx2) + expect(tx2).not.toBe(tx3) + + // Queue starts processing immediately + await new Promise((resolve) => setTimeout(resolve, 5)) + expect(mutationFn).toHaveBeenCalledTimes(1) + + // Wait for first transaction to complete + await tx1.isPersisted.promise + expect(tx1.state).toBe(`completed`) + + // Each mutation should be in its own transaction + expect(tx1.mutations).toHaveLength(1) + expect(tx1.mutations[0]).toMatchObject({ + type: `insert`, + changes: { id: 1, value: 1 }, + }) + }) + + it(`should ensure serialization - wait for each transaction to complete`, async () => { + const executionOrder: Array = [] + let currentlyExecuting = false + + const mutationFn = vi.fn(async ({ transaction }) => { + // Verify no concurrent execution + expect(currentlyExecuting).toBe(false) + currentlyExecuting = true + + const id = transaction.mutations[0].changes.id + executionOrder.push(id) + + // Simulate async work + await new Promise((resolve) => setTimeout(resolve, 20)) + + currentlyExecuting = false + }) + + const collection = createCollection( + mockSyncCollectionOptionsNoInitialState({ + id: `test`, + getKey: (item) => item.id, + }) + ) + + const preloadPromise = collection.preload() + collection.utils.begin() + collection.utils.commit() + collection.utils.markReady() + await preloadPromise + + const mutate = createPacedMutations({ + onMutate: (item) => { + collection.insert(item) + }, + mutationFn, + strategy: queueStrategy({ wait: 5 }), + }) + + // Trigger rapid mutations + const tx1 = mutate({ id: 1, value: 1 }) + const tx2 = mutate({ id: 2, value: 2 }) + const tx3 = mutate({ id: 3, value: 3 }) + + // Wait for all to complete + await Promise.all([ + tx1.isPersisted.promise, + tx2.isPersisted.promise, + tx3.isPersisted.promise, + ]) + + // Should have executed in order + expect(executionOrder).toEqual([1, 2, 3]) + + // All should be completed + expect(tx1.state).toBe(`completed`) + expect(tx2.state).toBe(`completed`) + expect(tx3.state).toBe(`completed`) + }) + + it(`should process each mutation in its own transaction`, async () => { + const mutationFn = vi.fn(async () => { + await new Promise((resolve) => setTimeout(resolve, 5)) + }) + + const collection = createCollection( + mockSyncCollectionOptionsNoInitialState({ + id: `test`, + getKey: (item) => item.id, + }) + ) + + const preloadPromise = collection.preload() + collection.utils.begin() + collection.utils.commit() + collection.utils.markReady() + await preloadPromise + + const mutate = createPacedMutations({ + onMutate: (item) => { + collection.insert(item) + }, + mutationFn, + strategy: queueStrategy({ wait: 5 }), + }) + + const tx1 = mutate({ id: 1, value: 1 }) + const tx2 = mutate({ id: 2, value: 2 }) + const tx3 = mutate({ id: 3, value: 3 }) + + // Each should be a separate transaction + expect(tx1).not.toBe(tx2) + expect(tx2).not.toBe(tx3) + expect(tx1).not.toBe(tx3) + + await Promise.all([ + tx1.isPersisted.promise, + tx2.isPersisted.promise, + tx3.isPersisted.promise, + ]) + + // All mutations should have been executed + expect(mutationFn).toHaveBeenCalledTimes(3) + + // Each transaction should have exactly one mutation + expect(tx1.mutations).toHaveLength(1) + expect(tx2.mutations).toHaveLength(1) + expect(tx3.mutations).toHaveLength(1) + }) + }) + + describe(`error handling`, () => { + it(`should handle mutationFn errors and set transaction to failed state`, async () => { + const error = new Error(`Mutation failed`) + const mutationFn = vi.fn(async () => { + throw error + }) + + const collection = createCollection( + mockSyncCollectionOptionsNoInitialState({ + id: `test`, + getKey: (item) => item.id, + }) + ) + + const preloadPromise = collection.preload() + collection.utils.begin() + collection.utils.commit() + collection.utils.markReady() + await preloadPromise + + const mutate = createPacedMutations({ + onMutate: (item) => { + collection.insert(item) + }, + mutationFn, + strategy: debounceStrategy({ wait: 10 }), + }) + + const tx = mutate({ id: 1, value: 1 }) + + // Wait for debounce + await new Promise((resolve) => setTimeout(resolve, 30)) + + // Transaction should be in failed state + expect(tx.state).toBe(`failed`) + await expect(tx.isPersisted.promise).rejects.toThrow(`Mutation failed`) + }) + + it(`should continue processing queue after an error`, async () => { + const mutationFn = vi + .fn() + .mockRejectedValueOnce(new Error(`First failed`)) + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(undefined) + + const collection = createCollection( + mockSyncCollectionOptionsNoInitialState({ + id: `test`, + getKey: (item) => item.id, + }) + ) + + const preloadPromise = collection.preload() + collection.utils.begin() + collection.utils.commit() + collection.utils.markReady() + await preloadPromise + + const mutate = createPacedMutations({ + onMutate: (item) => { + collection.insert(item) + }, + mutationFn, + strategy: queueStrategy({ wait: 10 }), + }) + + const tx1 = mutate({ id: 1, value: 1 }) + const tx2 = mutate({ id: 2, value: 2 }) + const tx3 = mutate({ id: 3, value: 3 }) + + // Wait for all to settle + await Promise.allSettled([ + tx1.isPersisted.promise, + tx2.isPersisted.promise, + tx3.isPersisted.promise, + ]) + + // First should fail, rest should succeed + expect(tx1.state).toBe(`failed`) + expect(tx2.state).toBe(`completed`) + expect(tx3.state).toBe(`completed`) + + // All three should have been attempted + expect(mutationFn).toHaveBeenCalledTimes(3) + }) + }) + + describe(`transaction batching`, () => { + it(`should merge mutations on the same key`, async () => { + const mutationFn = vi.fn(async () => {}) + + const collection = createCollection( + mockSyncCollectionOptionsNoInitialState({ + id: `test`, + getKey: (item) => item.id, + }) + ) + + const preloadPromise = collection.preload() + collection.utils.begin() + collection.utils.commit() + collection.utils.markReady() + await preloadPromise + + let insertCount = 0 + const mutate = createPacedMutations({ + onMutate: (item) => { + if (insertCount === 0) { + collection.insert(item) + insertCount++ + } else { + collection.update(item.id, (draft) => { + draft.value = item.value + }) + } + }, + mutationFn, + strategy: debounceStrategy({ wait: 50 }), + }) + + // Insert then update same key - should merge to single insert + mutate({ id: 1, value: 1 }) + mutate({ id: 1, value: 2 }) + mutate({ id: 1, value: 3 }) + + await new Promise((resolve) => setTimeout(resolve, 100)) + + expect(mutationFn).toHaveBeenCalledTimes(1) + const call = mutationFn.mock.calls[0][0] + expect(call.transaction.mutations).toHaveLength(1) + expect(call.transaction.mutations[0]).toMatchObject({ + type: `insert`, + changes: { id: 1, value: 3 }, // Final value + }) + }) + + it(`should batch mutations on different keys`, async () => { + const mutationFn = vi.fn(async () => {}) + + const collection = createCollection( + mockSyncCollectionOptionsNoInitialState({ + id: `test`, + getKey: (item) => item.id, + }) + ) + + const preloadPromise = collection.preload() + collection.utils.begin() + collection.utils.commit() + collection.utils.markReady() + await preloadPromise + + const mutate = createPacedMutations({ + onMutate: (item) => { + collection.insert(item) + }, + mutationFn, + strategy: debounceStrategy({ wait: 50 }), + }) + + // Multiple inserts on different keys + mutate({ id: 1, value: 1 }) + mutate({ id: 2, value: 2 }) + mutate({ id: 3, value: 3 }) + + await new Promise((resolve) => setTimeout(resolve, 100)) + + expect(mutationFn).toHaveBeenCalledTimes(1) + const call = mutationFn.mock.calls[0][0] + expect(call.transaction.mutations).toHaveLength(3) + }) + }) +}) From aecb09757873884b518d877acf8113eed034fd48 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Nov 2025 15:52:14 +0000 Subject: [PATCH 5/7] fix: resolve TypeScript type errors in paced mutations tests Use type assertions to handle vitest mock.calls type inference issues. --- packages/db/tests/paced-mutations.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/db/tests/paced-mutations.test.ts b/packages/db/tests/paced-mutations.test.ts index 8d5381096..6ed19d303 100644 --- a/packages/db/tests/paced-mutations.test.ts +++ b/packages/db/tests/paced-mutations.test.ts @@ -142,7 +142,8 @@ describe(`createPacedMutations`, () => { // NOW mutationFn should have been called expect(mutationFn).toHaveBeenCalledTimes(1) - expect(mutationFn.mock.calls[0][0].transaction.mutations).toHaveLength(1) + const firstCall = (mutationFn.mock.calls as any)[0] + expect(firstCall[0].transaction.mutations).toHaveLength(1) }) it(`should execute on leading edge when leading: true`, async () => { @@ -578,7 +579,7 @@ describe(`createPacedMutations`, () => { await new Promise((resolve) => setTimeout(resolve, 100)) expect(mutationFn).toHaveBeenCalledTimes(1) - const call = mutationFn.mock.calls[0][0] + const call = (mutationFn.mock.calls as any)[0][0] expect(call.transaction.mutations).toHaveLength(1) expect(call.transaction.mutations[0]).toMatchObject({ type: `insert`, @@ -618,7 +619,7 @@ describe(`createPacedMutations`, () => { await new Promise((resolve) => setTimeout(resolve, 100)) expect(mutationFn).toHaveBeenCalledTimes(1) - const call = mutationFn.mock.calls[0][0] + const call = (mutationFn.mock.calls as any)[0][0] expect(call.transaction.mutations).toHaveLength(3) }) }) From 91f025105eb7e6607b9f62427a12404fda2428c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Nov 2025 15:59:58 +0000 Subject: [PATCH 6/7] refactor: simplify React paced mutations tests Remove redundant strategy behavior tests from React package since those are now comprehensively tested in the core package. Kept: - Simple smoke tests for each strategy (verify React hook wrapper works) - All memoization tests (React-specific behavior) Removed: - Detailed debounce timer reset tests (now in core) - Detailed throttle leading/trailing tests (now in core) - Queue serialization tests (now in core) - Transaction batching tests (now in core) Reduced test file from 472 to 249 lines with better separation of concerns. --- .../react-db/tests/usePacedMutations.test.tsx | 467 +++++------------- 1 file changed, 123 insertions(+), 344 deletions(-) diff --git a/packages/react-db/tests/usePacedMutations.test.tsx b/packages/react-db/tests/usePacedMutations.test.tsx index 61d1643e4..8ce10b1d9 100644 --- a/packages/react-db/tests/usePacedMutations.test.tsx +++ b/packages/react-db/tests/usePacedMutations.test.tsx @@ -14,8 +14,8 @@ type Item = { value: number } -describe(`usePacedMutations with debounce strategy`, () => { - it(`should batch multiple rapid mutations into a single transaction`, async () => { +describe(`usePacedMutations React hook`, () => { + it(`should work with debounce strategy (smoke test)`, async () => { const mutationFn = vi.fn(async () => {}) const collection = createCollection( @@ -25,81 +25,36 @@ describe(`usePacedMutations with debounce strategy`, () => { }) ) - // Setup collection const preloadPromise = collection.preload() collection.utils.begin() collection.utils.commit() collection.utils.markReady() await preloadPromise - let insertCount = 0 const { result } = renderHook(() => - usePacedMutations<{ id: number; value: number }>({ + usePacedMutations({ onMutate: (item) => { - if (insertCount === 0) { - collection.insert(item) - insertCount++ - } else { - collection.update(item.id, (draft) => { - draft.value = item.value - }) - } + collection.insert(item) }, mutationFn, strategy: debounceStrategy({ wait: 50 }), }) ) - let tx1, tx2, tx3 - - // Trigger three rapid mutations (all within 50ms debounce window) - act(() => { - tx1 = result.current({ id: 1, value: 1 }) - }) - - act(() => { - tx2 = result.current({ id: 1, value: 2 }) - }) - + let tx act(() => { - tx3 = result.current({ id: 1, value: 3 }) + tx = result.current({ id: 1, value: 1 }) }) - // All three calls should return the SAME transaction object - expect(tx1).toBe(tx2) - expect(tx2).toBe(tx3) - - // Mutations get auto-merged (insert + updates on same key = single insert with final value) - expect(tx1!.mutations).toHaveLength(1) - expect(tx1!.mutations[0]).toMatchObject({ - type: `insert`, - changes: { id: 1, value: 3 }, // Final merged value - }) - - // mutationFn should NOT have been called yet (still debouncing) expect(mutationFn).not.toHaveBeenCalled() - // Wait for debounce period await new Promise((resolve) => setTimeout(resolve, 100)) - // Now mutationFn should have been called ONCE with the merged mutation expect(mutationFn).toHaveBeenCalledTimes(1) - expect(mutationFn).toHaveBeenCalledWith({ - transaction: expect.objectContaining({ - mutations: [ - expect.objectContaining({ - type: `insert`, - changes: { id: 1, value: 3 }, - }), - ], - }), - }) - - // Transaction should be completed - expect(tx1!.state).toBe(`completed`) + expect(tx!.state).toBe(`completed`) }) - it(`should reset debounce timer on each new mutation`, async () => { + it(`should work with throttle strategy (smoke test)`, async () => { const mutationFn = vi.fn(async () => {}) const collection = createCollection( @@ -115,62 +70,28 @@ describe(`usePacedMutations with debounce strategy`, () => { collection.utils.markReady() await preloadPromise - let insertCount = 0 const { result } = renderHook(() => - usePacedMutations<{ id: number; value: number }>({ + usePacedMutations({ onMutate: (item) => { - if (insertCount === 0) { - collection.insert(item) - insertCount++ - } else { - collection.update(item.id, (draft) => { - draft.value = item.value - }) - } + collection.insert(item) }, mutationFn, - strategy: debounceStrategy({ wait: 50 }), + strategy: throttleStrategy({ wait: 50, leading: true }), }) ) - // First mutation at t=0 + let tx act(() => { - result.current({ id: 1, value: 1 }) + tx = result.current({ id: 1, value: 1 }) }) - // Wait 40ms (still within 50ms debounce window) - await new Promise((resolve) => setTimeout(resolve, 40)) - - // mutationFn should NOT have been called yet - expect(mutationFn).not.toHaveBeenCalled() - - // Second mutation at t=40 (resets the timer) - act(() => { - result.current({ id: 1, value: 2 }) - }) - - // Wait another 40ms (t=80, but only 40ms since last mutation) - await new Promise((resolve) => setTimeout(resolve, 40)) - - // mutationFn still should NOT have been called (timer was reset) - expect(mutationFn).not.toHaveBeenCalled() - - // Wait another 20ms (t=100, now 60ms since last mutation, past the 50ms debounce) - await new Promise((resolve) => setTimeout(resolve, 20)) - - // NOW mutationFn should have been called + await new Promise((resolve) => setTimeout(resolve, 10)) expect(mutationFn).toHaveBeenCalledTimes(1) - const firstCall = mutationFn.mock.calls[0] as unknown as [ - { transaction: { mutations: Array } }, - ] - expect(firstCall[0].transaction.mutations).toHaveLength(1) // Merged to 1 mutation + expect(tx!.state).toBe(`completed`) }) -}) -describe(`usePacedMutations with queue strategy`, () => { - it(`should accumulate mutations then process sequentially`, async () => { + it(`should work with queue strategy (smoke test)`, async () => { const mutationFn = vi.fn(async () => { - // Quick execution await new Promise((resolve) => setTimeout(resolve, 5)) }) @@ -181,7 +102,6 @@ describe(`usePacedMutations with queue strategy`, () => { }) ) - // Setup collection const preloadPromise = collection.preload() collection.utils.begin() collection.utils.commit() @@ -198,275 +118,134 @@ describe(`usePacedMutations with queue strategy`, () => { }) ) - let tx1 - - // Trigger rapid mutations - queue creates separate transactions + let tx act(() => { - tx1 = result.current({ id: 1, value: 1 }) - }) - act(() => { - result.current({ id: 2, value: 2 }) - }) - act(() => { - result.current({ id: 3, value: 3 }) + tx = result.current({ id: 1, value: 1 }) }) - // Queue starts processing immediately - await new Promise((resolve) => setTimeout(resolve, 5)) + await tx!.isPersisted.promise expect(mutationFn).toHaveBeenCalledTimes(1) - - // Wait for transaction to complete - await tx1!.isPersisted.promise - expect(tx1!.state).toBe(`completed`) - - // Each mutation should be in its own transaction - expect(tx1!.mutations).toHaveLength(1) + expect(tx!.state).toBe(`completed`) }) -}) -describe(`usePacedMutations with throttle strategy`, () => { - it(`should throttle mutations with leading and trailing execution`, async () => { - const mutationFn = vi.fn(async () => {}) + describe(`memoization`, () => { + it(`should not recreate instance when strategy object changes but values are same`, () => { + const mutationFn = vi.fn(async () => {}) + const onMutate = vi.fn(() => {}) + + // Simulate a custom hook that creates strategy inline on each render + const useCustomHook = (wait: number) => { + return usePacedMutations({ + onMutate, + mutationFn, + // Strategy is created inline on every render - new object reference each time + strategy: debounceStrategy({ wait }), + }) + } + + const { result, rerender } = renderHook( + ({ wait }) => useCustomHook(wait), + { + initialProps: { wait: 3000 }, + } + ) + + const firstMutate = result.current + + // Rerender with same wait value - strategy object will be different reference + rerender({ wait: 3000 }) + const secondMutate = result.current + + // mutate function should be stable (same reference) + expect(secondMutate).toBe(firstMutate) + + // Rerender with different wait value - should create new instance + rerender({ wait: 5000 }) + const thirdMutate = result.current + + // mutate function should be different now + expect(thirdMutate).not.toBe(firstMutate) + }) + + it(`should not recreate instance when wrapped in custom hook with inline strategy`, () => { + const mutationFn = vi.fn(async () => {}) + const onMutate = vi.fn(() => {}) + + // Simulate the exact user scenario: custom hook wrapping usePacedMutations + const useDebouncedTransaction = (opts?: { + wait?: number + trailing?: boolean + leading?: boolean + }) => { + return usePacedMutations({ + onMutate, + mutationFn, + strategy: debounceStrategy({ + wait: opts?.wait ?? 3000, + trailing: opts?.trailing ?? true, + leading: opts?.leading ?? false, + }), + }) + } - const collection = createCollection( - mockSyncCollectionOptionsNoInitialState({ - id: `test`, - getKey: (item) => item.id, - }) - ) + const { result, rerender } = renderHook(() => useDebouncedTransaction()) - // Setup collection - const preloadPromise = collection.preload() - collection.utils.begin() - collection.utils.commit() - collection.utils.markReady() - await preloadPromise + const firstMutate = result.current - const { result } = renderHook(() => - usePacedMutations({ - onMutate: (item) => { - collection.insert(item) - }, - mutationFn, - strategy: throttleStrategy({ - wait: 100, - leading: true, - trailing: true, - }), - }) - ) + // Multiple rerenders with no options - should not recreate instance + rerender() + expect(result.current).toBe(firstMutate) - let tx1, tx2, tx3 + rerender() + expect(result.current).toBe(firstMutate) - // First mutation at t=0 (should execute immediately due to leading: true) - act(() => { - tx1 = result.current({ id: 1, value: 1 }) - }) + rerender() + expect(result.current).toBe(firstMutate) - // Leading edge should execute immediately - await new Promise((resolve) => setTimeout(resolve, 10)) - expect(mutationFn).toHaveBeenCalledTimes(1) - expect(tx1!.state).toBe(`completed`) - - // Second mutation at t=20 (during throttle period, should batch) - act(() => { - tx2 = result.current({ id: 2, value: 2 }) - }) - - // Third mutation at t=30 (during throttle period, should batch with second) - await new Promise((resolve) => setTimeout(resolve, 10)) - act(() => { - tx3 = result.current({ id: 3, value: 3 }) + // All should still be the same mutate function + expect(result.current).toBe(firstMutate) }) - // tx2 and tx3 should be the same transaction (batched) - expect(tx2).toBe(tx3) - - // Still only 1 call (waiting for throttle period to end) - expect(mutationFn).toHaveBeenCalledTimes(1) - - // Wait for throttle period to complete (100ms from first mutation) - await new Promise((resolve) => setTimeout(resolve, 110)) - - // Trailing edge should have executed - expect(mutationFn).toHaveBeenCalledTimes(2) - expect(tx2!.state).toBe(`completed`) - expect(tx3!.state).toBe(`completed`) + it(`should recreate instance when strategy options actually change`, () => { + const mutationFn = vi.fn(async () => {}) + const onMutate = vi.fn(() => {}) - // Verify the batched transaction has 2 inserts - expect(tx2!.mutations).toHaveLength(2) - }) - - it(`should respect trailing: true with leading: false option`, async () => { - const mutationFn = vi.fn(async () => {}) - - const collection = createCollection( - mockSyncCollectionOptionsNoInitialState({ - id: `test`, - getKey: (item) => item.id, - }) - ) + const useDebouncedTransaction = (opts?: { + wait?: number + trailing?: boolean + leading?: boolean + }) => { + return usePacedMutations({ + onMutate, + mutationFn, + strategy: debounceStrategy({ + wait: opts?.wait ?? 3000, + trailing: opts?.trailing ?? true, + leading: opts?.leading ?? false, + }), + }) + } - const preloadPromise = collection.preload() - collection.utils.begin() - collection.utils.commit() - collection.utils.markReady() - await preloadPromise + const { result, rerender } = renderHook( + ({ opts }) => useDebouncedTransaction(opts), + { initialProps: { opts: { wait: 3000 } } } + ) - const { result } = renderHook(() => - usePacedMutations({ - onMutate: (item) => { - collection.insert(item) - }, - mutationFn, - strategy: throttleStrategy({ - wait: 50, - leading: false, - trailing: true, - }), - }) - ) + const firstMutate = result.current - let tx1 + // Rerender with different wait value + rerender({ opts: { wait: 5000 } }) + const secondMutate = result.current - // First mutation should NOT execute immediately with leading: false - act(() => { - tx1 = result.current({ id: 1, value: 1 }) - }) + // Should be different instance since wait changed + expect(secondMutate).not.toBe(firstMutate) - // Should not have been called yet - await new Promise((resolve) => setTimeout(resolve, 10)) - expect(mutationFn).not.toHaveBeenCalled() + // Rerender with same wait value again + rerender({ opts: { wait: 5000 } }) + const thirdMutate = result.current - // Add another mutation during throttle period to ensure trailing fires - act(() => { - result.current({ id: 2, value: 2 }) + // Should be same as second since value didn't change + expect(thirdMutate).toBe(secondMutate) }) - - // Wait for throttle period to complete - await new Promise((resolve) => setTimeout(resolve, 70)) - - // Now trailing edge should have executed - expect(mutationFn).toHaveBeenCalledTimes(1) - await tx1!.isPersisted.promise - expect(tx1!.state).toBe(`completed`) - }) -}) - -describe(`usePacedMutations memoization`, () => { - it(`should not recreate instance when strategy object changes but values are same`, () => { - const mutationFn = vi.fn(async () => {}) - const onMutate = vi.fn(() => {}) - - // Simulate a custom hook that creates strategy inline on each render - const useCustomHook = (wait: number) => { - return usePacedMutations({ - onMutate, - mutationFn, - // Strategy is created inline on every render - new object reference each time - strategy: debounceStrategy({ wait }), - }) - } - - const { result, rerender } = renderHook(({ wait }) => useCustomHook(wait), { - initialProps: { wait: 3000 }, - }) - - const firstMutate = result.current - - // Rerender with same wait value - strategy object will be different reference - rerender({ wait: 3000 }) - const secondMutate = result.current - - // mutate function should be stable (same reference) - expect(secondMutate).toBe(firstMutate) - - // Rerender with different wait value - should create new instance - rerender({ wait: 5000 }) - const thirdMutate = result.current - - // mutate function should be different now - expect(thirdMutate).not.toBe(firstMutate) - }) - - it(`should not recreate instance when wrapped in custom hook with inline strategy`, () => { - const mutationFn = vi.fn(async () => {}) - const onMutate = vi.fn(() => {}) - - // Simulate the exact user scenario: custom hook wrapping usePacedMutations - const useDebouncedTransaction = (opts?: { - wait?: number - trailing?: boolean - leading?: boolean - }) => { - return usePacedMutations({ - onMutate, - mutationFn, - strategy: debounceStrategy({ - wait: opts?.wait ?? 3000, - trailing: opts?.trailing ?? true, - leading: opts?.leading ?? false, - }), - }) - } - - const { result, rerender } = renderHook(() => useDebouncedTransaction()) - - const firstMutate = result.current - - // Multiple rerenders with no options - should not recreate instance - rerender() - expect(result.current).toBe(firstMutate) - - rerender() - expect(result.current).toBe(firstMutate) - - rerender() - expect(result.current).toBe(firstMutate) - - // All should still be the same mutate function - expect(result.current).toBe(firstMutate) - }) - - it(`should recreate instance when strategy options actually change`, () => { - const mutationFn = vi.fn(async () => {}) - const onMutate = vi.fn(() => {}) - - const useDebouncedTransaction = (opts?: { - wait?: number - trailing?: boolean - leading?: boolean - }) => { - return usePacedMutations({ - onMutate, - mutationFn, - strategy: debounceStrategy({ - wait: opts?.wait ?? 3000, - trailing: opts?.trailing ?? true, - leading: opts?.leading ?? false, - }), - }) - } - - const { result, rerender } = renderHook( - ({ opts }) => useDebouncedTransaction(opts), - { initialProps: { opts: { wait: 3000 } } } - ) - - const firstMutate = result.current - - // Rerender with different wait value - rerender({ opts: { wait: 5000 } }) - const secondMutate = result.current - - // Should be different instance since wait changed - expect(secondMutate).not.toBe(firstMutate) - - // Rerender with same wait value again - rerender({ opts: { wait: 5000 } }) - const thirdMutate = result.current - - // Should be same as second since value didn't change - expect(thirdMutate).toBe(secondMutate) }) }) From 2a8667629b919e64321256e7335e68b5117bb47f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Nov 2025 16:33:05 +0000 Subject: [PATCH 7/7] docs: address code review feedback for paced mutations - Add explanatory comments for manual promise chaining in queueStrategy explaining why LiteQueuer lacks async queue primitives and how we compensate with manual serialization - Enhance error handling documentation clarifying that errors are surfaced via transaction.isPersisted and don't break the chain - Add test coverage for queue with zero/no wait option - Extract createReadyCollection helper to reduce test duplication --- packages/db/src/strategies/queueStrategy.ts | 12 +- packages/db/tests/paced-mutations.test.ts | 266 +++++++++----------- 2 files changed, 127 insertions(+), 151 deletions(-) diff --git a/packages/db/src/strategies/queueStrategy.ts b/packages/db/src/strategies/queueStrategy.ts index 7f9887333..1f1270a4d 100644 --- a/packages/db/src/strategies/queueStrategy.ts +++ b/packages/db/src/strategies/queueStrategy.ts @@ -44,12 +44,15 @@ import type { Transaction } from "../transactions" * ``` */ export function queueStrategy(options?: QueueStrategyOptions): QueueStrategy { - // Track the current promise chain to ensure serialization + // Manual promise chaining to ensure async serialization + // LiteQueuer (unlike AsyncQueuer from @tanstack/pacer) lacks built-in async queue + // primitives and concurrency control. We compensate by manually chaining promises + // to ensure each transaction completes before the next one starts. let processingChain = Promise.resolve() const queuer = new LiteQueuer<() => Transaction>( (fn) => { - // Chain each transaction processing to ensure serialization + // Chain each transaction to the previous one's completion processingChain = processingChain .then(async () => { const transaction = fn() @@ -57,8 +60,9 @@ export function queueStrategy(options?: QueueStrategyOptions): QueueStrategy { await transaction.isPersisted.promise }) .catch(() => { - // Errors are handled via transaction.isPersisted.promise - // This catch prevents unhandled promise rejections + // Errors are handled via transaction.isPersisted.promise and surfaced there. + // This catch prevents unhandled promise rejections from breaking the chain, + // ensuring subsequent transactions can still execute even if one fails. }) }, { diff --git a/packages/db/tests/paced-mutations.test.ts b/packages/db/tests/paced-mutations.test.ts index 6ed19d303..1944969d5 100644 --- a/packages/db/tests/paced-mutations.test.ts +++ b/packages/db/tests/paced-mutations.test.ts @@ -13,24 +13,36 @@ type Item = { value: number } +/** + * Helper to create a collection that's ready for testing. + * Handles all the boilerplate setup: preload, begin, commit, markReady. + */ +async function createReadyCollection(opts: { + id: string + getKey: (item: T) => string | number +}) { + const collection = createCollection( + mockSyncCollectionOptionsNoInitialState(opts) + ) + + const preloadPromise = collection.preload() + collection.utils.begin() + collection.utils.commit() + collection.utils.markReady() + await preloadPromise + + return collection +} + describe(`createPacedMutations`, () => { describe(`with debounce strategy`, () => { it(`should batch multiple rapid mutations into a single transaction`, async () => { const mutationFn = vi.fn(async () => {}) - const collection = createCollection( - mockSyncCollectionOptionsNoInitialState({ - id: `test`, - getKey: (item) => item.id, - }) - ) - - // Setup collection - const preloadPromise = collection.preload() - collection.utils.begin() - collection.utils.commit() - collection.utils.markReady() - await preloadPromise + const collection = await createReadyCollection({ + id: `test`, + getKey: (item) => item.id, + }) let insertCount = 0 const mutate = createPacedMutations<{ id: number; value: number }>({ @@ -90,18 +102,10 @@ describe(`createPacedMutations`, () => { it(`should reset debounce timer on each new mutation`, async () => { const mutationFn = vi.fn(async () => {}) - const collection = createCollection( - mockSyncCollectionOptionsNoInitialState({ - id: `test`, - getKey: (item) => item.id, - }) - ) - - const preloadPromise = collection.preload() - collection.utils.begin() - collection.utils.commit() - collection.utils.markReady() - await preloadPromise + const collection = await createReadyCollection({ + id: `test`, + getKey: (item) => item.id, + }) let insertCount = 0 const mutate = createPacedMutations<{ id: number; value: number }>({ @@ -149,18 +153,10 @@ describe(`createPacedMutations`, () => { it(`should execute on leading edge when leading: true`, async () => { const mutationFn = vi.fn(async () => {}) - const collection = createCollection( - mockSyncCollectionOptionsNoInitialState({ - id: `test`, - getKey: (item) => item.id, - }) - ) - - const preloadPromise = collection.preload() - collection.utils.begin() - collection.utils.commit() - collection.utils.markReady() - await preloadPromise + const collection = await createReadyCollection({ + id: `test`, + getKey: (item) => item.id, + }) const mutate = createPacedMutations({ onMutate: (item) => { @@ -186,19 +182,10 @@ describe(`createPacedMutations`, () => { it(`should throttle mutations with leading and trailing execution`, async () => { const mutationFn = vi.fn(async () => {}) - const collection = createCollection( - mockSyncCollectionOptionsNoInitialState({ - id: `test`, - getKey: (item) => item.id, - }) - ) - - // Setup collection - const preloadPromise = collection.preload() - collection.utils.begin() - collection.utils.commit() - collection.utils.markReady() - await preloadPromise + const collection = await createReadyCollection({ + id: `test`, + getKey: (item) => item.id, + }) const mutate = createPacedMutations({ onMutate: (item) => { @@ -248,18 +235,10 @@ describe(`createPacedMutations`, () => { it(`should respect leading: false option`, async () => { const mutationFn = vi.fn(async () => {}) - const collection = createCollection( - mockSyncCollectionOptionsNoInitialState({ - id: `test`, - getKey: (item) => item.id, - }) - ) - - const preloadPromise = collection.preload() - collection.utils.begin() - collection.utils.commit() - collection.utils.markReady() - await preloadPromise + const collection = await createReadyCollection({ + id: `test`, + getKey: (item) => item.id, + }) const mutate = createPacedMutations({ onMutate: (item) => { @@ -293,19 +272,10 @@ describe(`createPacedMutations`, () => { await new Promise((resolve) => setTimeout(resolve, 5)) }) - const collection = createCollection( - mockSyncCollectionOptionsNoInitialState({ - id: `test`, - getKey: (item) => item.id, - }) - ) - - // Setup collection - const preloadPromise = collection.preload() - collection.utils.begin() - collection.utils.commit() - collection.utils.markReady() - await preloadPromise + const collection = await createReadyCollection({ + id: `test`, + getKey: (item) => item.id, + }) const mutate = createPacedMutations({ onMutate: (item) => { @@ -358,18 +328,10 @@ describe(`createPacedMutations`, () => { currentlyExecuting = false }) - const collection = createCollection( - mockSyncCollectionOptionsNoInitialState({ - id: `test`, - getKey: (item) => item.id, - }) - ) - - const preloadPromise = collection.preload() - collection.utils.begin() - collection.utils.commit() - collection.utils.markReady() - await preloadPromise + const collection = await createReadyCollection({ + id: `test`, + getKey: (item) => item.id, + }) const mutate = createPacedMutations({ onMutate: (item) => { @@ -405,18 +367,10 @@ describe(`createPacedMutations`, () => { await new Promise((resolve) => setTimeout(resolve, 5)) }) - const collection = createCollection( - mockSyncCollectionOptionsNoInitialState({ - id: `test`, - getKey: (item) => item.id, - }) - ) - - const preloadPromise = collection.preload() - collection.utils.begin() - collection.utils.commit() - collection.utils.markReady() - await preloadPromise + const collection = await createReadyCollection({ + id: `test`, + getKey: (item) => item.id, + }) const mutate = createPacedMutations({ onMutate: (item) => { @@ -449,6 +403,56 @@ describe(`createPacedMutations`, () => { expect(tx2.mutations).toHaveLength(1) expect(tx3.mutations).toHaveLength(1) }) + + it(`should work with zero or no wait option`, async () => { + const mutationFn = vi.fn(async () => { + await new Promise((resolve) => setTimeout(resolve, 5)) + }) + + const collection = await createReadyCollection({ + id: `test`, + getKey: (item) => item.id, + }) + + // Test with explicit wait: 0 + const mutateExplicitZero = createPacedMutations({ + onMutate: (item) => { + collection.insert(item) + }, + mutationFn, + strategy: queueStrategy({ wait: 0 }), + }) + + const tx1 = mutateExplicitZero({ id: 1, value: 1 }) + const tx2 = mutateExplicitZero({ id: 2, value: 2 }) + + // Should still process sequentially even with zero wait + await Promise.all([tx1.isPersisted.promise, tx2.isPersisted.promise]) + + expect(mutationFn).toHaveBeenCalledTimes(2) + expect(tx1.state).toBe(`completed`) + expect(tx2.state).toBe(`completed`) + + mutationFn.mockClear() + + // Test with no wait option (defaults to 0) + const mutateNoWait = createPacedMutations({ + onMutate: (item) => { + collection.insert(item) + }, + mutationFn, + strategy: queueStrategy(), + }) + + const tx3 = mutateNoWait({ id: 3, value: 3 }) + const tx4 = mutateNoWait({ id: 4, value: 4 }) + + await Promise.all([tx3.isPersisted.promise, tx4.isPersisted.promise]) + + expect(mutationFn).toHaveBeenCalledTimes(2) + expect(tx3.state).toBe(`completed`) + expect(tx4.state).toBe(`completed`) + }) }) describe(`error handling`, () => { @@ -458,18 +462,10 @@ describe(`createPacedMutations`, () => { throw error }) - const collection = createCollection( - mockSyncCollectionOptionsNoInitialState({ - id: `test`, - getKey: (item) => item.id, - }) - ) - - const preloadPromise = collection.preload() - collection.utils.begin() - collection.utils.commit() - collection.utils.markReady() - await preloadPromise + const collection = await createReadyCollection({ + id: `test`, + getKey: (item) => item.id, + }) const mutate = createPacedMutations({ onMutate: (item) => { @@ -496,18 +492,10 @@ describe(`createPacedMutations`, () => { .mockResolvedValueOnce(undefined) .mockResolvedValueOnce(undefined) - const collection = createCollection( - mockSyncCollectionOptionsNoInitialState({ - id: `test`, - getKey: (item) => item.id, - }) - ) - - const preloadPromise = collection.preload() - collection.utils.begin() - collection.utils.commit() - collection.utils.markReady() - await preloadPromise + const collection = await createReadyCollection({ + id: `test`, + getKey: (item) => item.id, + }) const mutate = createPacedMutations({ onMutate: (item) => { @@ -542,18 +530,10 @@ describe(`createPacedMutations`, () => { it(`should merge mutations on the same key`, async () => { const mutationFn = vi.fn(async () => {}) - const collection = createCollection( - mockSyncCollectionOptionsNoInitialState({ - id: `test`, - getKey: (item) => item.id, - }) - ) - - const preloadPromise = collection.preload() - collection.utils.begin() - collection.utils.commit() - collection.utils.markReady() - await preloadPromise + const collection = await createReadyCollection({ + id: `test`, + getKey: (item) => item.id, + }) let insertCount = 0 const mutate = createPacedMutations({ @@ -590,18 +570,10 @@ describe(`createPacedMutations`, () => { it(`should batch mutations on different keys`, async () => { const mutationFn = vi.fn(async () => {}) - const collection = createCollection( - mockSyncCollectionOptionsNoInitialState({ - id: `test`, - getKey: (item) => item.id, - }) - ) - - const preloadPromise = collection.preload() - collection.utils.begin() - collection.utils.commit() - collection.utils.markReady() - await preloadPromise + const collection = await createReadyCollection({ + id: `test`, + getKey: (item) => item.id, + }) const mutate = createPacedMutations({ onMutate: (item) => {