feat(offline-transactions): confirm writes off the serial drain path (confirmWrite hook)#1603
Conversation
Add an opt-in `OfflineConfig.confirmWrite` hook that holds a just-committed write's optimistic state through the post-commit confirmation window, off the serial drain path, so waiting for an async sync stream (e.g. Electric's awaitTxId) no longer throttles drain throughput and no longer flickers rows. - Factor the hold create/release primitive into executor/OptimisticHold.ts and reuse it in restoreOptimisticState. - Add maxConfirmationHolds cap + getActiveConfirmationHoldCount() diagnostic; release holds on clear()/dispose(). - Thread the mutationFn result through to the completion promise and the hook (previously awaited and discarded). - Fully opt-in: with no confirmWrite, behavior is unchanged. Closes TanStack#1602 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds an opt-in ChangesconfirmWrite hook and optimistic confirmation lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/offline-transactions/src/types.ts (1)
103-103: ⚡ Quick winUse
unknownforConfirmWriteContext.metadatainstead ofany.Line 103 introduces a new public API field typed as
Record<string, any>, which drops type safety at the boundary and propagates unchecked values downstream.Suggested patch
- metadata?: Record<string, any> + metadata?: Record<string, unknown>As per coding guidelines, "Avoid using
anytypes; useunknowninstead when the type is truly unknown."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/offline-transactions/src/types.ts` at line 103, Replace the `any` type with `unknown` in the ConfirmWriteContext.metadata field definition. Change the metadata field from Record<string, any> to Record<string, unknown> to maintain type safety at the public API boundary and align with coding guidelines that avoid using any types.Source: Coding guidelines
packages/offline-transactions/tests/confirm-write.test.ts (1)
149-171: ⚡ Quick winAdd a regression assertion that
confirmWritestill executes when hold creation is skipped.This test currently validates only the hold count/overlay behavior. It should also assert that the hook is invoked under
maxConfirmationHolds: 0, so a contract regression (hook suppressed) is caught.Suggested patch
it(`skips the hold past maxConfirmationHolds (O(n^2) safety valve)`, async () => { const gate = deferred() + let confirmCalls = 0 const config: Partial<OfflineConfig> = { - confirmWrite: () => gate.promise, + confirmWrite: () => { + confirmCalls += 1 + return gate.promise + }, maxConfirmationHolds: 0, } @@ expect(env.executor.getActiveConfirmationHoldCount()).toBe(0) expect(env.collection.get(`item-1`)).toBeUndefined() + expect(confirmCalls).toBe(1)As per coding guidelines, "
**/*.test.{ts,tsx,js}: Always add unit tests that reproduce a bug before fixing it to ensure the bug is fixed and prevent regression."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/offline-transactions/tests/confirm-write.test.ts` around lines 149 - 171, The test currently only validates that no hold is created when maxConfirmationHolds is 0, but does not verify that the confirmWrite hook is still invoked. Add a spy or flag to track whether the confirmWrite callback executes, then add an assertion before gate.resolve() to verify that confirmWrite was indeed called despite the hold being skipped. This ensures the hook contract is maintained even when hold creation is bypassed due to the maxConfirmationHolds cap.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/offline-transactions/src/executor/TransactionExecutor.ts`:
- Around line 200-219: The confirmWrite callback is being silently skipped due
to early returns at lines 202 and 213 when holds are capped or hold creation
fails, which violates the post-commit hook contract. Restructure the logic in
the method body to ensure confirmWrite is always invoked, regardless of hold
availability. Additionally, the maxConfirmationHolds assignment does not
normalize NaN or negative config values, which can unexpectedly bypass or
disable the cap. Add validation to ensure maxConfirmationHolds is a positive
integer, using something like Math.max to ensure proper normalization before the
size check is performed.
---
Nitpick comments:
In `@packages/offline-transactions/src/types.ts`:
- Line 103: Replace the `any` type with `unknown` in the
ConfirmWriteContext.metadata field definition. Change the metadata field from
Record<string, any> to Record<string, unknown> to maintain type safety at the
public API boundary and align with coding guidelines that avoid using any types.
In `@packages/offline-transactions/tests/confirm-write.test.ts`:
- Around line 149-171: The test currently only validates that no hold is created
when maxConfirmationHolds is 0, but does not verify that the confirmWrite hook
is still invoked. Add a spy or flag to track whether the confirmWrite callback
executes, then add an assertion before gate.resolve() to verify that
confirmWrite was indeed called despite the hold being skipped. This ensures the
hook contract is maintained even when hold creation is bypassed due to the
maxConfirmationHolds cap.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a618729c-330c-4d17-ba1b-7fa34d594a50
📒 Files selected for processing (7)
.changeset/confirm-write-hook.mdpackages/offline-transactions/src/OfflineExecutor.tspackages/offline-transactions/src/executor/OptimisticHold.tspackages/offline-transactions/src/executor/TransactionExecutor.tspackages/offline-transactions/src/types.tspackages/offline-transactions/tests/confirm-write.test.tspackages/offline-transactions/tests/offline-e2e.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/offline-transactions/src/executor/OptimisticHold.ts`:
- Around line 90-119: Replace the direct use of private collection internals in
the optimistic transaction setup with a supported public API or hook exposed by
`@tanstack/db`. Update the logic around mutation collections and the cleanup path
in OptimisticHold to call that registration/recompute hook, preserving
deduplication and failure-atomic rollback without accessing _state.transactions
or recomputeOptimisticState().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cd1d025e-8254-4ce3-b1c4-236c66954fec
📒 Files selected for processing (13)
.changeset/confirm-write-hook.mdpackages/offline-transactions/README.mdpackages/offline-transactions/skills/offline/SKILL.mdpackages/offline-transactions/src/OfflineExecutor.tspackages/offline-transactions/src/api/OfflineTransaction.tspackages/offline-transactions/src/executor/ConfirmationManager.tspackages/offline-transactions/src/executor/OptimisticHold.tspackages/offline-transactions/src/executor/TransactionExecutor.tspackages/offline-transactions/src/index.tspackages/offline-transactions/src/react-native/index.tspackages/offline-transactions/src/types.tspackages/offline-transactions/tests/confirm-write.test.tspackages/offline-transactions/tests/optimistic-hold.test.ts
✅ Files skipped from review due to trivial changes (2)
- packages/offline-transactions/src/react-native/index.ts
- packages/offline-transactions/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- .changeset/confirm-write-hook.md
- packages/offline-transactions/src/types.ts
| try { | ||
| transaction.applyMutations(mutations) | ||
|
|
||
| for (const mutation of mutations) { | ||
| // Defensive check for corrupted deserialized data | ||
| // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition | ||
| if (!mutation.collection) { | ||
| continue | ||
| } | ||
| if (touchedCollections.has(mutation.collection)) { | ||
| continue | ||
| } | ||
| // Track before registration so a throwing set/recompute is unwound too. | ||
| touchedCollections.add(mutation.collection) | ||
| mutation.collection._state.transactions.set(transaction.id, transaction) | ||
| // `recomputeOptimisticState(true)` forces the recompute through even when | ||
| // a sync commit is in flight (the "triggered by user action" path), so the | ||
| // overlay always applies. | ||
| mutation.collection._state.recomputeOptimisticState(true) | ||
| } | ||
| } catch (error) { | ||
| // Failure-atomic creation: remove any registrations installed before the | ||
| // throw and complete the synthetic transaction so no global state leaks. | ||
| try { | ||
| release() | ||
| } catch { | ||
| // Preserve the original registration error after best-effort cleanup. | ||
| } | ||
| throw error | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm _state.transactions / recomputeOptimisticState are exposed & used elsewhere in-repo
rg -nP '_state\.(transactions|recomputeOptimisticState)' --type=ts
# Inspect the Collection state definition to confirm these members exist
fd -e ts . packages/db/src --exec rg -nl 'recomputeOptimisticState' {}Repository: TanStack/db
Length of output: 1974
🌐 Web query:
"@tanstack/db" recomputeOptimisticState _state.transactions
💡 Result:
In @tanstack/db, the recomputeOptimisticState method is a core private function within the CollectionStateManager (or CollectionImpl) responsible for recalculating the application's optimistic state based on active transactions, pending sync operations, and local mutations [1][2]. It manages the reconciliation between synced data, optimistic upserts, and optimistic deletes to ensure the UI remains consistent while background syncs are in progress [3][4][5]. Key responsibilities and behaviors of recomputeOptimisticState include: 1. Optimistic State Reconciliation: It clears existing optimistic upserts, deletes, and local changes, then reseeds them based on active/pending transactions [5]. It uses sets like pendingOptimisticDirectUpserts and pendingOptimisticDirectDeletes to maintain the visibility of optimistic changes during the gap between transaction completion and server confirmation [3][4]. 2. User Action Optimization: The method accepts a triggeredByUserAction boolean parameter. When true, it bypasses certain complex event filtering and batching logic to prevent UI blocking, ensuring that actions initiated directly by the user are reflected immediately [1]. 3. Acknowledgment Tracking: It manages the $acknowledged state of rows by tying it to the covering (last-applied) active optimistic mutation [6]. Recent changes ensure that recomputeOptimisticState snapshots previous acknowledgment states before rebuilding, which keeps the $acknowledged status accurate during rollbacks or overwrites [7]. 4. Addressing State Leaks: The logic has undergone various refinements to prevent state "leaks" (where keys persist indefinitely in optimistic sets), particularly when a server confirms a mutation under a different key than the client's optimistic key [3][4]. These fixes often involve using guards (like add-once mechanisms) and tracking transactions that performed immediate sync writes to ensure optimistic state is correctly cleaned up [4]. If you are encountering errors related to this method, ensure your version of @tanstack/db is up to date, as many issues involving state consistency, cleanup, and transaction handling have been addressed in recent releases [4][8].
Citations:
- 1: 3e9a36d
- 2: https://app.unpkg.com/@tanstack/db@0.5.31/files/src/collection/state.ts
- 3: fix(db): pendingOptimisticDirectUpserts leaks after writeInsert in onInsert handler, causing duplication and stale $synced #1440
- 4: fix(db): clean up optimistic state when server returns a different key #1465
- 5: https://app.unpkg.com/@tanstack/db@0.6.7/files/dist/cjs/collection/state.cjs.map
- 6: 447080d
- 7: feat(db): expose an
acknowledgedstate for optimistic mutations #1633 - 8: TypeError: Cannot read properties of undefined (reading 'Symbol(liveQueryInternal)') when calling .update() on a queryCollection #1599
🌐 Web query:
site:github.com/tanstack/db recomputeOptimisticState _state.transactions
💡 Result:
The function recomputeOptimisticState is a method found within TanStack DB, specifically in the context of handling offline transactions [1]. It is used to recalculate the application state following optimistic updates [1][2]. When _state.transactions is modified—for example, when a transaction is deleted or completed—the system calls recomputeOptimisticState to ensure the local state accurately reflects the pending changes and any subsequent operations [1]. This mechanism allows TanStack DB to provide instant user interface feedback by processing writes locally before they are confirmed by the server [2]. By managing the transaction queue and recomputing the resulting state, the library maintains consistency between the server-side source of truth and the client-side optimistic UI [1][2].
Citations:
- 1: https://github.com/TanStack/db/blob/main/packages/offline-transactions/src/OfflineExecutor.ts
- 2: https://github.com/tanstack/db
Avoid depending on @tanstack/db internals here. collection._state.transactions and recomputeOptimisticState() are private implementation details, so an @tanstack/db upgrade can break optimistic overlays without a public API change. Expose a supported hook for transaction registration/recompute instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/offline-transactions/src/executor/OptimisticHold.ts` around lines 90
- 119, Replace the direct use of private collection internals in the optimistic
transaction setup with a supported public API or hook exposed by `@tanstack/db`.
Update the logic around mutation collections and the cleanup path in
OptimisticHold to call that registration/recompute hook, preserving
deduplication and failure-atomic rollback without accessing _state.transactions
or recomputeOptimisticState().
1 participant
Closes #1602
🎯 Changes
Adds an opt-in
OfflineConfig.confirmWritehook to@tanstack/offline-transactionsthat keeps a just-committed write's optimistic state painted across the post-commit confirmation window — running off the serial drain path.Motivation
The executor drains the outbox serially (one
mutationFnat a time), which is what preserves create-then-update / FK ordering. But there's no built-in way to keep a row painted between "the server committed the write" and "the sync stream echoed it back". The only option today is toawaitthat confirmation inside themutationFn— e.g.:Because the drain is serial, that
awaitblocks the next write. With an ElectricSQL shape stream whoseawaitTxIdbudget is ~10s, throughput collapses to ~1 write / 10s and each settled write tends to spawn a duplicate "already-applied" resend. The alternative — dropping theawait— makes rows flicker (disappear then reappear) in the commit→sync gap, because the executor drops the transaction's optimistic overlay the instantmutationFnresolves.What this adds
confirmWriteruns after the write commits and its outbox entry is removed, but off the serial path — it never blocks the nextmutationFn:While the returned promise is pending, the library keeps the committed mutations' optimistic overlay painted via an internal hold transaction — the same primitive
restoreOptimisticStatealready uses to re-show pending writes after a reload — then releases it when the hook settles. The serial chain still serializes the POSTs (ordering preserved); only the confirmation moved off it.Design points:
confirmWritejust releases the overlay early (a possible brief flicker), never data loss. Any timeout / verify-by-state logic lives inside the hook.resolveTransactiondrops the original overlay, so the overlay is owned continuously.maxConfirmationHolds(default1000) caps concurrent holds to avoid O(n²) optimistic-recompute churn on a large, fast drain; beyond the cap the hold is skipped (degrades to today's behavior).getActiveConfirmationHoldCount()exposes the live count.clear()anddispose().Internals: the hold create/release pattern is factored into a new
executor/OptimisticHold.tsand reused byrestoreOptimisticState, so there's a single primitive. The hook is fully opt-in — with noconfirmWriteconfigured, behavior is byte-for-byte unchanged.Drive-by fix
runMutationFnpreviouslyawaited themutationFnand discarded its return value, sowaitForTransactionCompletionalways resolvedundefined. The result is now threaded through to the completion promise and toconfirmWrite(so the hook can read e.g. a server txid). One existing e2e assertion was updated to reflect this.Context
This generalizes a pattern we shipped app-side at Autarc (an external
offlineConfirmationHolds.tsthat reaches intocollection._stateand re-implements the library's private hold/teardown). With this hook, that ~360-line module collapses to the smallconfirmWritecallback above. Happy to bikeshed the name (confirmWrite/awaitSync/confirmTransaction) and the default cap.✅ Checklist
pnpm test. (@tanstack/offline-transactions: 71 passing + 6 new intests/confirm-write.test.ts, 1 pre-existing skip; full suite green.eslint srcandprettier --checkclean.tscintroduces zero new errors vsmain.)🚀 Release Impact
.changeset/confirm-write-hook.md,minor).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
confirmWritepost-commit hook to control when optimistic overlays are released.maxConfirmationHoldsto cap concurrent confirmation overlays.getActiveConfirmationHoldCount()for visibility into active confirmation holds.Tests
confirmWrite, hold limits, clear/dispose behavior, and non-blocking confirmations.Documentation