fix(db): reuse collection descriptors by id - #1770
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthrough
ChangesDescriptor identity reuse
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change reuses collections by descriptor id while preserving collection behavior; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant CollectionDescriptor
participant DbClient
participant optionsFactory
participant MaterializedCollection
CollectionDescriptor->>DbClient: Request materialization by ID
DbClient->>MaterializedCollection: Check existing collection by ID
DbClient->>optionsFactory: Invoke factory when no collection exists
optionsFactory->>DbClient: Return collection options
DbClient->>MaterializedCollection: Reuse matching collection
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description is complete and directly related to the change. It explains the root cause, approach, invariants, trade-offs, non-goals, verification, affected files, and release impact through the included changeset. Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (3 skipped: 3 unsupported.) ✨ Finishing Touches 💡 1📝 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/react-router-with-db
@tanstack/rxdb-db-collection
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/tauri-db-sqlite-persistence
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
|
Size Change: -78 B (-0.05%) Total Size: 156 kB 📦 View Changed
ℹ️ View Unchanged
|
|
Size Change: 0 B Total Size: 7.25 kB ℹ️ View Unchanged
|
DbClientnow treats a collection descriptor'sidas its identity, so separately created descriptors with the same id reuse one collection. React code can create dynamic descriptors from current parameters without keeping a descriptor cache, as long as every collection-defining parameter is included in the id.Root cause
DbClientmemoized materialized collections by descriptor object reference in aWeakMap. A new descriptor object therefore missed the cache even when its id matched an existing collection, and the client rejected the duplicate id. This diverged from the id-based identity used to track hydration, dehydration, cleanup, and live-query collections.Approach
collectionsByIdas the single source of truth for materialized collections.Key invariants
DbClienthas at most one materialized collection for each id.Non-goals
DbClientinstances.Trade-offs
Id-based lookup removes the need for caller-owned descriptor caches and matches the client's other collection bookkeeping. It also makes id collisions meaningful: callers must include every parameter that changes collection behavior in the id, and the first materialized descriptor wins.
Verification
The database package tests cover separately created descriptors and reentrant same-id materialization. The React package test recreates a descriptor across renders and verifies that its factory runs once and its live-query collection stays stable.
Files changed
.changeset/fix-db-client-descriptor-identity.md: record the@tanstack/dbpatch release note.packages/db/src/client.ts: canonicalize collection materialization by descriptor id.packages/db/tests/db-client.test.ts: cover same-id descriptor reuse and reentrant factories.packages/react-db/tests/useLiveQuery.test.tsx: cover fresh dynamic descriptors across React renders.docs/guides/ssr.md: state the per-client id identity rule.docs/collections/query-collection.md: replace descriptor-cache guidance with dynamic id guidance.Summary by CodeRabbit
Bug Fixes
Documentation