refactor(db): extract shared live-query adapter helpers (RFC #1623 step 2)#1641
Open
kevin-dp wants to merge 1 commit into
Open
refactor(db): extract shared live-query adapter helpers (RFC #1623 step 2)#1641kevin-dp wants to merge 1 commit into
kevin-dp wants to merge 1 commit into
Conversation
Contributor
📝 WalkthroughWalkthroughShared live-query adapter helper functions (isCollection, isSingleResultCollection, getLiveQueryStatusFlags) are added to ChangesLive-query adapter helper extraction and migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
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: |
Contributor
|
Size Change: +383 B (+0.31%) Total Size: 125 kB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
|
Size Change: 0 B Total Size: 4.26 kB ℹ️ View Unchanged
|
kevin-dp
force-pushed
the
refactor/extract-adapter-helpers
branch
from
92f19fa to
84895f6
Compare
kevin-dp
force-pushed
the
refactor/extract-adapter-helpers
branch
from
84895f6 to
84f4e93
Compare
Add isCollection, isSingleResultCollection, and getLiveQueryStatusFlags to
@tanstack/db and migrate all five adapters to use them.
- isCollection: one structural, multi-realm-safe collection guard replacing the
per-adapter duck-typing (React/Vue/Svelte/Angular) and Solid's
`instanceof CollectionImpl` (which gives false negatives across dual-package
boundaries — the same hazard the conformance suite hit).
- isSingleResultCollection: shared findOne cardinality check.
- getLiveQueryStatusFlags: status → {isLoading,isReady,isIdle,isError,isCleanedUp};
used by React's snapshot path (the reactive adapters derive each flag as its own
signal/computed, so a shared object-returning helper doesn't fit them — that
duplication is reactivity-coupled and belongs to the observer step).
No behavior change; guarded by the conformance suite. First slice of the RFC
#1623 extraction (step 2). Status derivation, input/disabled classification, and
change→state projection remain — they are reactivity-coupled and land with the
observer.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
kevin-dp
force-pushed
the
refactor/extract-adapter-helpers
branch
from
84f4e93 to
4cf25ae
Compare
kevin-dp
requested review from
KyleAMathews and
samwillis
and removed request for
KyleAMathews
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/angular-db/src/index.ts (1)
281-283: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer a narrower cast than
Signal<any>forcollection.Casting to
Signal<any>discards all type information from the declaredInjectLiveQueryResult/InjectLiveQueryResultWithSingleResultCollectionreturn shapes (Signal<Collection<...> | null>), so the implementation body is no longer checked against either overload'scollectionfield. As per coding guidelines, "Avoid usinganytypes; useunknowninstead when the type is truly unknown, and provide proper type annotations for return values."♻️ Suggested refactor
- // Loosely typed so the impl return stays compatible with every overload - // (the shared `isCollection` guard narrows the computed to `Collection | null`). - collection: collection as Signal<any>, + // Loosely typed so the impl return stays compatible with every overload + // (the shared `isCollection` guard narrows the computed to `Collection | null`). + collection: collection as Signal<Collection<any, any, any> | null>,🤖 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/angular-db/src/index.ts` around lines 281 - 283, The `collection` field in the `injectLiveQuery` return object is cast too broadly to `Signal<any>`, which erases the intended `InjectLiveQueryResult`/`InjectLiveQueryResultWithSingleResultCollection` typing. Update the implementation to use a narrower, specific signal type that matches the computed `Collection<...> | null` shape (or a shared union/unknown-based annotation if needed) so the overload contract stays checked. Locate the change in `injectLiveQuery` where `collection` is assigned and replace the `Signal<any>` cast with a properly typed return annotation or assertion.Source: Coding guidelines
packages/db/src/live-query-adapter.ts (1)
22-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid
anyinside this shared guard. Castvalueonce toRecord<string, unknown>after the object check, then probe the properties from that typed reference.🤖 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/live-query-adapter.ts` around lines 22 - 32, The shared type guard in isCollection currently uses repeated any casts to inspect properties; replace those with a single post-null object cast to Record<string, unknown> and read subscribeChanges, startSyncImmediate, and id from that typed reference. Keep the same runtime checks, but ensure the guard no longer relies on any inside the predicate.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.
Nitpick comments:
In `@packages/angular-db/src/index.ts`:
- Around line 281-283: The `collection` field in the `injectLiveQuery` return
object is cast too broadly to `Signal<any>`, which erases the intended
`InjectLiveQueryResult`/`InjectLiveQueryResultWithSingleResultCollection`
typing. Update the implementation to use a narrower, specific signal type that
matches the computed `Collection<...> | null` shape (or a shared
union/unknown-based annotation if needed) so the overload contract stays
checked. Locate the change in `injectLiveQuery` where `collection` is assigned
and replace the `Signal<any>` cast with a properly typed return annotation or
assertion.
In `@packages/db/src/live-query-adapter.ts`:
- Around line 22-32: The shared type guard in isCollection currently uses
repeated any casts to inspect properties; replace those with a single post-null
object cast to Record<string, unknown> and read subscribeChanges,
startSyncImmediate, and id from that typed reference. Keep the same runtime
checks, but ensure the guard no longer relies on any inside the predicate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f92418ce-9388-43da-b94f-d8cbe3d645d6
📒 Files selected for processing (8)
.changeset/extract-adapter-helpers.mdpackages/angular-db/src/index.tspackages/db/src/index.tspackages/db/src/live-query-adapter.tspackages/react-db/src/useLiveQuery.tspackages/solid-db/src/useLiveQuery.tspackages/svelte-db/src/useLiveQuery.svelte.tspackages/vue-db/src/useLiveQuery.ts
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
2 participants
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.
The first real deduplication slice of RFC #1623 — the first PR that touches adapter
src/and shrinks copy-paste. Follows the conformance suite (#1636), which guards this refactor.Base: stacked on
test/live-query-conformance-suite(#1636) so the conformance suite verifies the refactored adapters. Re-target tomainonce #1636 lands.What's extracted
Three helpers added to
@tanstack/db(packages/db/src/live-query-adapter.ts), and all five adapters migrated to them:isCollection(value)— one structural, multi-realm-safe collection guard. Replaces the duck-typed check copy-pasted in React/Vue/Svelte/Angular and Solid'sinstanceof CollectionImpl.instanceofgives false negatives across dual-package/multi-realm boundaries — the exact hazard the conformance suite hit — so consolidating on duck typing is also a correctness win.isSingleResultCollection(collection)— sharedfindOnecardinality check (wasconfig.singleResult, five times).getLiveQueryStatusFlags(status)—status → { isLoading, isReady, isIdle, isError, isCleanedUp }.Honest scope note
This slice is smaller than the RFC's "7 duplicated items" might suggest — and that's a real finding. Only the reactivity-free logic lifts cleanly into plain functions:
getLiveQueryStatusFlagsfits React's snapshot path, but the reactive adapters (Vue/Svelte/Solid/Angular) each expose every flag as its owncomputed/signal, so an object-returning helper doesn't fit them — forcing it would add code.So this PR takes the clean, universal wins now (notably unifying
isCollectionand killing theinstanceofdivergence). The larger dedup is exactly what the observer (step 3) exists to do — and per the design decision, that observer will carry both a snapshot and theChangeMessage[]so granular adapters (Vue/Svelte) keep fine-grained updates while others consume the snapshot.Verification
No behavior change. All five adapter suites green (including the conformance suite that guards this), and
@tanstack/db(2456 tests) green. Minor changeset for@tanstack/db(new exports) + patch for the adapters.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes