feat: $pendingOperation virtual prop#1431
Open
marbemac wants to merge 8 commits into
Open
Conversation
… type
Add a new virtual property $pendingOperation ('insert' | 'update' | 'delete' | null)
to every collection row. This tells consumers what type of optimistic mutation is
pending, enabling draft/review UIs that show git-style change indicators.
- Subscription-layer delete-to-update conversion for opted-in queries - Auto-detection of $pendingOperation in where clauses - Initial snapshot includes pending-delete items - Fix missing $pendingOperation in multi-group GROUP BY path - Tighten string|null to PendingOperationType in group-by compiler - Fix ?? vs !== undefined inconsistency for nullable $pendingOperation - Add GROUP BY tests for $pendingOperation aggregation
… memory cleanup - Fix stale $pendingOperation after rollback of optimistic delete by tracking converted delete values and converting rollback inserts to updates - Fix isRowSynced to check pendingOptimistic* maps for consistency with getPendingOperation (prevents contradictory $synced: true + $pendingOperation: 'delete') - Fix lazy source (join/subquery) pending deletes by merging child sourceWhereClauses into parent during query compilation - Fix convertedDeleteValues memory leak: clean up on sync-confirmed delete, truncate, and unsubscribe - Add computePendingOperation callback to enrichRowWithVirtualProps
Export expressionReferencesPendingOperation from collection-subscriber and use it in effect.ts buildSubscriptionOptions to auto-detect $pendingOperation references and set includePendingDeletes on the subscription.
Signed-off-by: Marc MacLeod <marbemac+gh@gmail.com>
Contributor
Author
|
One thing I considered but left out because wasn't sure if ya'll would be ok w a new API method. A .where(({ task }) =>
and(
eq(task.projectId, projectId),
or(isNull(task.$pendingOperation), not(isNull(task.$pendingOperation))),
),
)User could do: .where(({ task }) => eq(task.projectId, projectId))
.includePendingDeletes()OR alternative, just a simple exported util: .where(({ task }) => includeDeletedItems(eq(task.projectId, projectId)))If that's something you guys would consider, I can try and add it - don't think it'd be much code. |
More templates
@tanstack/angular-db
@tanstack/browser-db-sqlite-persistence
@tanstack/capacitor-db-sqlite-persistence
@tanstack/cloudflare-durable-objects-db-sqlite-persistence
@tanstack/db
@tanstack/db-ivm
@tanstack/db-sqlite-persistence-core
@tanstack/electric-db-collection
@tanstack/electron-db-sqlite-persistence
@tanstack/expo-db-sqlite-persistence
@tanstack/node-db-sqlite-persistence
@tanstack/offline-transactions
@tanstack/powersync-db-collection
@tanstack/query-db-collection
@tanstack/react-db
@tanstack/react-native-db-sqlite-persistence
@tanstack/rxdb-db-collection
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/tauri-db-sqlite-persistence
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
…-virtual-prop # Conflicts: # packages/db/src/query/compiler/group-by.ts
Contributor
📝 WalkthroughWalkthroughIntroduces a Changes$pendingOperation virtual property feature
Import ordering cosmetic changes
Sequence Diagram(s)sequenceDiagram
participant LiveQuery
participant CollectionSubscriber
participant CollectionSubscription
participant CollectionStateManager
participant expressionReferencesPendingOperation
LiveQuery->>CollectionSubscriber: subscribe(whereExpression)
CollectionSubscriber->>expressionReferencesPendingOperation: whereExpression
expressionReferencesPendingOperation-->>CollectionSubscriber: true (references $pendingOperation)
CollectionSubscriber->>CollectionSubscription: subscribeChanges({includePendingDeletes: true})
Note over CollectionSubscription: optimistic delete fires
CollectionSubscription->>CollectionStateManager: getPendingOperation(key) → 'delete'
CollectionSubscription->>CollectionSubscription: convertPendingDeletes — delete→update($pendingOperation:'delete')
CollectionSubscription-->>LiveQuery: update event (row visible with $pendingOperation:'delete')
Note over CollectionSubscription: sync confirms delete
CollectionSubscription-->>LiveQuery: delete event (row removed)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/db/src/collection/subscription.ts (1)
87-87: ⚡ Quick winReplace
anyinconvertedDeleteValueswith a concrete row value type.Using
anyhere weakens type safety in the rollback conversion path.♻️ Suggested typing cleanup
+type ConvertedDeleteValue = Record<string, unknown> @@ - private convertedDeleteValues = new Map<string | number, any>() + private convertedDeleteValues = new Map<string | number, ConvertedDeleteValue>()As per coding guidelines, "
**/*.{ts,tsx}: Avoid usinganytypes; useunknowninstead when the type is truly unknown, and provide proper type annotations for return values".🤖 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/db/src/collection/subscription.ts` at line 87, In the convertedDeleteValues Map declaration, replace the any type parameter used for the Map's values with a concrete row value type that represents the actual data being stored in the rollback conversion path. If the specific type cannot be determined, use unknown instead of any per the coding guidelines. Ensure proper type annotations are provided for this Map to strengthen type safety throughout the subscription class.Source: Coding guidelines
packages/db/tests/pending-operation.test.ts (1)
954-963: 🏗️ Heavy liftReduce
anyusage in test query/effect wiring to preserve type safety.These blocks rely on
anyfor query builders, row aliases, and event payloads, which weakens compile-time checks in exactly the contracts this suite is validating. Introduce local typed aliases/helpers (for query row shapes and event values) and useunknown+ narrowing only where unavoidable.As per coding guidelines, “Avoid using
anytypes; useunknowninstead when the type is truly unknown, and provide proper type annotations for return values.”Also applies to: 989-998, 1092-1102, 1188-1204, 1287-1297
🤖 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/db/tests/pending-operation.test.ts` around lines 954 - 963, Replace excessive `any` type annotations with properly typed aliases and narrowed types throughout the test file to improve type safety. Introduce local typed aliases for query row shapes (such as the task item structure used in the groupBy and select callbacks) and replace all `any` parameter annotations with either the specific typed aliases or `unknown` with appropriate type narrowing. This change applies to multiple locations in packages/db/tests/pending-operation.test.ts: the query builder callback at lines 954-963, the effect wiring blocks at lines 989-998, 1092-1102, 1188-1204, and 1287-1297. For each location, identify what data structure is being queried or processed and create a corresponding type interface, then use that type instead of `any` in the callback parameters and destructuring patterns.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/db/src/collection/subscription.ts`:
- Around line 424-446: The appendPendingDeleteItems method is adding all
pending-delete items to the changes array without verifying they match the
active whereExpression filter. This causes rows that fail the query predicate to
be incorrectly emitted in filtered queries, particularly those with predicates
on $pendingOperation. Before pushing the enriched value to the changes array,
check if the enriched value passes the active whereExpression filter (similar to
how other code paths in the class validate items against the query predicate),
and only add it if it matches. Apply this same filtering logic at the other two
locations mentioned in the comment where pending deletes are appended.
In `@packages/db/tests/pending-operation.test.ts`:
- Line 11: The waitForChanges function uses a hardcoded 10ms setTimeout delay,
making tests timing-sensitive and flaky. Replace this fixed-delay approach with
a deterministic mechanism that waits for actual observable state transitions,
such as awaiting a specific collection/query event, a promise that resolves when
data changes are detected, or a polling helper that checks for completion of
pending operations rather than relying on arbitrary time delays.
---
Nitpick comments:
In `@packages/db/src/collection/subscription.ts`:
- Line 87: In the convertedDeleteValues Map declaration, replace the any type
parameter used for the Map's values with a concrete row value type that
represents the actual data being stored in the rollback conversion path. If the
specific type cannot be determined, use unknown instead of any per the coding
guidelines. Ensure proper type annotations are provided for this Map to
strengthen type safety throughout the subscription class.
In `@packages/db/tests/pending-operation.test.ts`:
- Around line 954-963: Replace excessive `any` type annotations with properly
typed aliases and narrowed types throughout the test file to improve type
safety. Introduce local typed aliases for query row shapes (such as the task
item structure used in the groupBy and select callbacks) and replace all `any`
parameter annotations with either the specific typed aliases or `unknown` with
appropriate type narrowing. This change applies to multiple locations in
packages/db/tests/pending-operation.test.ts: the query builder callback at lines
954-963, the effect wiring blocks at lines 989-998, 1092-1102, 1188-1204, and
1287-1297. For each location, identify what data structure is being queried or
processed and create a corresponding type interface, then use that type instead
of `any` in the callback parameters and destructuring patterns.
🪄 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: be4fed18-8548-4356-b955-6e6bc39d01f5
📒 Files selected for processing (19)
.changeset/pending-operation-virtual-prop.mdpackages/db-sqlite-persistence-core/tests/persisted.test.tspackages/db/src/collection/state.tspackages/db/src/collection/subscription.tspackages/db/src/index.tspackages/db/src/query/compiler/group-by.tspackages/db/src/query/effect.tspackages/db/src/query/live/collection-subscriber.tspackages/db/src/types.tspackages/db/src/virtual-props.tspackages/db/tests/collection-subscribe-changes.test.tspackages/db/tests/pending-operation.test.tspackages/db/tests/utils.tspackages/electric-db-collection/e2e/electric.e2e.test.tspackages/electric-db-collection/tests/electric-live-query.test.tspackages/query-db-collection/e2e/offline-refresh.e2e.test.tspackages/query-db-collection/e2e/query.e2e.test.tspackages/react-db/tests/useLiveInfiniteQuery.test.tsxpackages/trailbase-db-collection/e2e/trailbase.e2e.test.ts
Comment on lines
+424
to
+446
| private appendPendingDeleteItems( | ||
| changes: Array<ChangeMessage<any, any>>, | ||
| ): void { | ||
| const state = this.collection._state | ||
| const pendingDeleteKeys = new Set([ | ||
| ...state.optimisticDeletes, | ||
| ...state.pendingOptimisticDeletes, | ||
| ]) | ||
| for (const key of pendingDeleteKeys) { | ||
| if (this.sentKeys.has(key)) continue | ||
| const syncedValue = state.syncedData.get(key) | ||
| if (syncedValue !== undefined) { | ||
| // enrichWithVirtualProps computes $pendingOperation via getPendingOperation(key), | ||
| // which returns 'delete' for keys in optimisticDeletes/pendingOptimisticDeletes. | ||
| const enrichedValue = state.enrichWithVirtualProps(syncedValue, key) | ||
| changes.push({ | ||
| type: `insert` as const, | ||
| key, | ||
| value: enrichedValue, | ||
| }) | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Pending-delete append path bypasses active whereExpression filtering.
appendPendingDeleteItems adds all pending-delete keys from state, but these rows are not re-checked against the query predicate before being emitted. In filtered queries that reference $pendingOperation plus other predicates, this can leak rows that should not match.
🛠️ Proposed fix
- private appendPendingDeleteItems(
- changes: Array<ChangeMessage<any, any>>,
- ): void {
+ private appendPendingDeleteItems(
+ changes: Array<ChangeMessage<any, any>>,
+ whereExpression?: BasicExpression<boolean>,
+ ): void {
const state = this.collection._state
+ const whereFilter = whereExpression
+ ? createFilterFunctionFromExpression(whereExpression)
+ : undefined
const pendingDeleteKeys = new Set([
...state.optimisticDeletes,
...state.pendingOptimisticDeletes,
@@
const syncedValue = state.syncedData.get(key)
if (syncedValue !== undefined) {
const enrichedValue = state.enrichWithVirtualProps(syncedValue, key)
+ if (whereFilter && !whereFilter(enrichedValue)) {
+ continue
+ }
changes.push({
type: `insert` as const,
key,
value: enrichedValue,
})
}
}
}- this.appendPendingDeleteItems(snapshot)
+ this.appendPendingDeleteItems(snapshot, stateOpts.where)- this.appendPendingDeleteItems(changes)
+ this.appendPendingDeleteItems(changes, where)Also applies to: 513-518, 666-670
🤖 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/db/src/collection/subscription.ts` around lines 424 - 446, The
appendPendingDeleteItems method is adding all pending-delete items to the
changes array without verifying they match the active whereExpression filter.
This causes rows that fail the query predicate to be incorrectly emitted in
filtered queries, particularly those with predicates on $pendingOperation.
Before pushing the enriched value to the changes array, check if the enriched
value passes the active whereExpression filter (similar to how other code paths
in the class validate items against the query predicate), and only add it if it
matches. Apply this same filtering logic at the other two locations mentioned in
the comment where pending deletes are appended.
| import { mockSyncCollectionOptions, stripVirtualProps } from './utils' | ||
| import type { ChangeMessage } from '../src/types' | ||
|
|
||
| const waitForChanges = () => new Promise((resolve) => setTimeout(resolve, 10)) |
Contributor
There was a problem hiding this comment.
Replace fixed-delay waiting with a deterministic async signal.
Line 11 uses a hardcoded setTimeout(..., 10), which can make CI timing-sensitive and flaky under load. Prefer awaiting an explicit collection/query event or a reusable polling helper tied to observable state transitions.
As per coding guidelines, “Test corner cases including ... async race conditions,” fixed sleeps are brittle for race-sensitive test synchronization.
🤖 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/db/tests/pending-operation.test.ts` at line 11, The waitForChanges
function uses a hardcoded 10ms setTimeout delay, making tests timing-sensitive
and flaky. Replace this fixed-delay approach with a deterministic mechanism that
waits for actual observable state transitions, such as awaiting a specific
collection/query event, a promise that resolves when data changes are detected,
or a polling helper that checks for completion of pending operations rather than
relying on arbitrary time delays.
Source: Coding guidelines
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Changes
Adds a new
$pendingOperationvirtual property to collection rows ('insert' | 'update' | 'delete' | null). This tells you what type of optimistic mutation is pending for each row, which is useful for building draft/review UIs where you want to show git-style change indicators, amongst other things.Note, large-ish diff but most of it is new tests 😅.
The key feature: items deleted in a pending transaction can now stay visible in query results. By default nothing changes — deleted items still vanish from queries. But if your query references
$pendingOperationin a.where()clause, the implicit filter is disabled and you can see pending-delete items inline:Works with live queries,
createEffect, joins/subqueries, GROUP BY, ordered/paginated queries, and selective where clauses likenot(isNull($pendingOperation))for "show only pending changes" views.This was discussed with @samwillis beforehand — the approach is to keep deletes as deletes at the collection layer and convert them to updates at the subscription layer when opted in.
I also went ahead and built and linked into our actual app to test out the functionality. It's damn neat! For example, can show indicators inline for as yet uncommitted transaction. Imagine CRM app, and app with data grids w rows that can be updated/removed/etc, or really any productivity app that wants to allow humans and agents to safely make changes, and represent those changes inline in the application without having to change the data layer at all (note
RandAbadges on the files on the left, and the red bordered "install" that has been deleted but not yet "committed").Notable behavioral change
isRowSynced()now also checkspendingOptimisticUpsertsandpendingOptimisticDeletes, so$syncedisfalseduring the completed-but-awaiting-sync window. Previously it could briefly show$synced: truewhile a pending operation was still awaiting sync confirmation.✅ Checklist
pnpm test.🚀 Release Impact
Summary by CodeRabbit
New Features
$pendingOperationvirtual property tracking optimistic mutations (insert, update, delete) on collection rows, enabling pending-delete visibility in live queries, effects, grouped results, and joins when referenced in filter conditions.Tests
$pendingOperationbehavior across mutations, transactions, live queries, and query filtering.