Skip to content

Conversation

@KyleAMathews
Copy link
Collaborator

Fixes #706

Previously we were validating calls to write* calls against the combined sync+optimistic view — which failed if you'd already made some types of optimistic changes. This PR fixes validation to only look at the synced store as that's where write* calls are going to.

…andler

This test reproduces issue #706 where calling writeDelete() inside an
onDelete handler causes unexpected behavior.

The Root Cause:
When collection.delete() is called, it creates a transaction and calls
commit() before calling recomputeOptimisticState(). Because commit() is
async but starts executing immediately, the onDelete handler runs BEFORE
the optimistic delete is applied to the collection state.

Timeline:
1. collection.delete('1') is called
2. Transaction is created with autoCommit: true
3. commit() is called (async, but starts immediately)
4. Handler runs inside commit() - optimisticDeletes is empty!
5. commit() completes
6. recomputeOptimisticState() is finally called - too late

Expected Behavior:
- optimisticDeletes.has('1') should be TRUE when handler runs
- writeDelete('1') should throw DeleteOperationItemNotFoundError

Actual Behavior (BUG):
- optimisticDeletes.has('1') is FALSE when handler runs
- writeDelete('1') succeeds instead of throwing
- This causes state inconsistencies and silent failures

The test will fail until this timing issue is fixed.

Related: packages/db/src/collection/mutations.ts lines 529-537

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
…teDelete

This test reproduces issue #706 where calling writeDelete() inside an
onDelete handler causes the deleted item to reappear.

The Complete Root Cause:
========================

When collection.delete() is called with an onDelete handler that uses writeDelete():

1. Transaction is created and commit() starts (mutations.ts:531)
2. Transaction NOT yet added to state.transactions (line 533 runs after)
3. onDelete handler runs while transaction.state = 'persisting'
4. Handler calls writeDelete('1')
5. writeDelete checks for persisting transactions in state.transactions
6. Finds NONE (transaction not added yet), commits synced delete immediately
7. Item removed from syncedData ✓
8. Handler completes
9. wrappedOnDelete automatically calls refetch() (query.ts:681)
10. Refetch fetches data from server
11. Server still has item (transaction delete hasn't executed yet)
12. Refetch OVERWRITES syncedData with server data ✗
13. Item reappears!

The Two-Part Bug:
================

Part 1: Transaction added to state.transactions AFTER commit() starts
- In mutations.ts:529-537, commit() is called on line 531
- Transaction added to state.transactions on line 533 (too late)
- Handler runs before transaction is in the map
- This allows writeDelete to commit immediately

Part 2: Automatic refetch undoes the synced write
- In query.ts:674-686, wrappedOnDelete automatically refetches
- Unless handler returns { refetch: false }
- Refetch restores server data, overwriting synced changes
- This is the reason the item reappears

Test Demonstrates:
- writeDelete succeeds (no error)
- Synced transaction committed immediately (persisting transactions: 0)
- queryFn called twice (initial + refetch)
- Final state: item still present (BUG!)

Expected: Item should stay deleted after writeDelete
Actual: Automatic refetch restores it

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Corrected understanding of the bug based on issue details:
- User returns { refetch: false } so automatic refetch is not the cause
- User also deletes on backend, so refetch would work anyway

The Real Bug:
=============

The issue is that errors thrown by writeDelete() inside onDelete handlers
are silently swallowed by .catch(() => undefined) in mutations.ts:531

When optimistic delete IS applied before handler runs:
1. collection.delete('1') creates optimistic delete
2. collection.has('1') returns false
3. onDelete handler runs
4. Handler calls writeDelete('1')
5. writeDelete validates: !collection.has('1') → throws DeleteOperationItemNotFoundError
6. Error propagates, commit() rejects
7. .catch(() => undefined) SILENTLY SWALLOWS error
8. User sees: execution stops, no error message, item flickers and reappears

The test demonstrates calling writeDelete in onDelete with refetch: false
(the exact pattern from the issue). The .catch(() => undefined) is the
root cause that prevents users from seeing errors.

Note: Due to timing (transaction not in state.transactions when handler runs),
this test hits the scenario where writeDelete succeeds. The bug manifests
when the optimistic delete IS applied, causing writeDelete to throw.

Related code: mutations.ts:531

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
…ned view

The bug: When calling writeDelete() inside an onDelete handler, it would throw
DeleteOperationItemNotFoundError because it checked the combined view (synced + optimistic)
which already had the item optimistically deleted.

The fix: Change manual-sync.ts to check only the synced store, not the combined view.

Changes in packages/query-db-collection/src/manual-sync.ts:
- Line 116: Changed from ctx.collection.has(op.key) to ctx.collection._state.syncedData.has(op.key)
- Line 120: Same change for delete validation
- Line 155: Changed from ctx.collection.get(op.key) to ctx.collection._state.syncedData.get(op.key)
- Line 173: Same change for delete operation
- Line 182: Changed ctx.collection.has(op.key) to ctx.collection._state.syncedData.has(op.key) for upsert

Why this fixes the issue:
- writeDelete operates on the synced store, not the optimistic state
- Validation should match the store being modified
- This allows write operations to work correctly even when items are optimistically modified
- Now handlers can safely call writeDelete/writeUpdate regardless of optimistic state

Test updated:
- Renamed test to reflect it now verifies the fix works
- Test passes: writeDelete succeeds, handler completes, item deleted successfully
- No errors thrown, execution continues as expected

Fixes #706

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@changeset-bot
Copy link

changeset-bot bot commented Oct 22, 2025

🦋 Changeset detected

Latest commit: c3f3a51

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@tanstack/query-db-collection Patch
@tanstack/db-example-react-todo Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 22, 2025

More templates

@tanstack/angular-db

npm i https://pkg.pr.new/@tanstack/angular-db@708

@tanstack/db

npm i https://pkg.pr.new/@tanstack/db@708

@tanstack/db-ivm

npm i https://pkg.pr.new/@tanstack/db-ivm@708

@tanstack/electric-db-collection

npm i https://pkg.pr.new/@tanstack/electric-db-collection@708

@tanstack/query-db-collection

npm i https://pkg.pr.new/@tanstack/query-db-collection@708

@tanstack/react-db

npm i https://pkg.pr.new/@tanstack/react-db@708

@tanstack/rxdb-db-collection

npm i https://pkg.pr.new/@tanstack/rxdb-db-collection@708

@tanstack/solid-db

npm i https://pkg.pr.new/@tanstack/solid-db@708

@tanstack/svelte-db

npm i https://pkg.pr.new/@tanstack/svelte-db@708

@tanstack/trailbase-db-collection

npm i https://pkg.pr.new/@tanstack/trailbase-db-collection@708

@tanstack/vue-db

npm i https://pkg.pr.new/@tanstack/vue-db@708

commit: c3f3a51

- Removed console.log and debugging output from test
- Removed lengthy comment explanations
- Simplified test to be concise and focused
- Added changeset describing the fix

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions
Copy link
Contributor

github-actions bot commented Oct 22, 2025

Size Change: 0 B

Total Size: 84.3 kB

ℹ️ View Unchanged
Filename Size
./packages/db/dist/esm/collection/change-events.js 1.63 kB
./packages/db/dist/esm/collection/changes.js 1.01 kB
./packages/db/dist/esm/collection/events.js 413 B
./packages/db/dist/esm/collection/index.js 3.23 kB
./packages/db/dist/esm/collection/indexes.js 1.16 kB
./packages/db/dist/esm/collection/lifecycle.js 1.8 kB
./packages/db/dist/esm/collection/mutations.js 2.52 kB
./packages/db/dist/esm/collection/state.js 3.79 kB
./packages/db/dist/esm/collection/subscription.js 2.2 kB
./packages/db/dist/esm/collection/sync.js 2.2 kB
./packages/db/dist/esm/deferred.js 230 B
./packages/db/dist/esm/errors.js 3.48 kB
./packages/db/dist/esm/event-emitter.js 798 B
./packages/db/dist/esm/index.js 1.62 kB
./packages/db/dist/esm/indexes/auto-index.js 794 B
./packages/db/dist/esm/indexes/base-index.js 835 B
./packages/db/dist/esm/indexes/btree-index.js 2 kB
./packages/db/dist/esm/indexes/lazy-index.js 1.21 kB
./packages/db/dist/esm/indexes/reverse-index.js 577 B
./packages/db/dist/esm/local-only.js 967 B
./packages/db/dist/esm/local-storage.js 2.4 kB
./packages/db/dist/esm/optimistic-action.js 294 B
./packages/db/dist/esm/proxy.js 3.86 kB
./packages/db/dist/esm/query/builder/functions.js 615 B
./packages/db/dist/esm/query/builder/index.js 4.04 kB
./packages/db/dist/esm/query/builder/ref-proxy.js 938 B
./packages/db/dist/esm/query/compiler/evaluators.js 1.55 kB
./packages/db/dist/esm/query/compiler/expressions.js 760 B
./packages/db/dist/esm/query/compiler/group-by.js 2.04 kB
./packages/db/dist/esm/query/compiler/index.js 2.21 kB
./packages/db/dist/esm/query/compiler/joins.js 2.65 kB
./packages/db/dist/esm/query/compiler/order-by.js 1.43 kB
./packages/db/dist/esm/query/compiler/select.js 1.28 kB
./packages/db/dist/esm/query/ir.js 785 B
./packages/db/dist/esm/query/live-query-collection.js 404 B
./packages/db/dist/esm/query/live/collection-config-builder.js 5.54 kB
./packages/db/dist/esm/query/live/collection-registry.js 233 B
./packages/db/dist/esm/query/live/collection-subscriber.js 2.11 kB
./packages/db/dist/esm/query/optimizer.js 3.26 kB
./packages/db/dist/esm/scheduler.js 1.29 kB
./packages/db/dist/esm/SortedMap.js 1.24 kB
./packages/db/dist/esm/transactions.js 3.05 kB
./packages/db/dist/esm/utils.js 1.01 kB
./packages/db/dist/esm/utils/browser-polyfills.js 365 B
./packages/db/dist/esm/utils/btree.js 6.01 kB
./packages/db/dist/esm/utils/comparison.js 754 B
./packages/db/dist/esm/utils/index-optimization.js 1.73 kB

compressed-size-action::db-package-size

@github-actions
Copy link
Contributor

Size Change: 0 B

Total Size: 2.89 kB

ℹ️ View Unchanged
Filename Size
./packages/react-db/dist/esm/index.js 168 B
./packages/react-db/dist/esm/useLiveInfiniteQuery.js 1.41 kB
./packages/react-db/dist/esm/useLiveQuery.js 1.31 kB

compressed-size-action::react-db-package-size

@samwillis
Copy link
Collaborator

samwillis commented Oct 23, 2025

Good spot. Note that this will reject updates/deletes to inserted items until they have been confirmed by the server, that's likely a safe place to be, but does conflict with persisted offline mutations.

Nope, misunderstood it, it's the manure sync methods, this looks good to me

@KyleAMathews KyleAMathews merged commit 5950583 into main Oct 23, 2025
6 checks passed
@KyleAMathews KyleAMathews deleted the claude/reproduce-issue-011CUNK1MsVUpVKUz1B9vzPD branch October 23, 2025 13:35
@github-actions github-actions bot mentioned this pull request Oct 23, 2025
@github-actions
Copy link
Contributor

🎉 This PR has been released!

Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

writeDelete silently failing

3 participants