Fix/live query record drop on subset unmount#1656
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe PR reorders row tracking and ownership persistence in ChangesQuery Collection Ownership Fix
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
775e89d to
66f109f
Compare
…w updates, as it was previously reading stale owners before any updates causing rows contained within 2 subsets to be removed on unmount
66f109f to
fb9fa21
Compare
|
Closing as superseded after RFC #1657 Phase 0 review. The immediate persisted-metadata clobber described here was addressed more fundamentally by #1626, and #1664 has since refactored the ownership paths touched by this branch. The added React test uses mockSyncCollectionOptions and does not exercise query-db ownership. Broader ownership, provider-lifetime, and destructive-diff semantics should be handled by RFC #1657 PR-5's coverage registry rather than landing this partial ownership expansion. A genuine query-db-level red/green regression test can still be harvested separately if it exposes remaining behavior. Thank you for the investigation and contribution. |
🎯 Changes
When a row was first inserted by subset query A,
write({ type: 'insert' })cleared the row's pending metadata.setPersistedOwnerswas called beforewrite, so the ownership it set was immediately overwritten by the insert's metadata-delete.When a later live query called
getHydratedOwnedRowsForQueryBaseline, it read the stale persisted state and overwrote the in-memoryrowToQueriesmap, dropping query A's hash from the owner set. When that query was GC'd, it found zero remaining owners and deleted the row — removing it from unrelated list views.✅ Checklist
pnpm test.🚀 Release Impact
Notes
Currently facing the below issue with react-db and I believe the below re-ordering of lines fixes it for myself but need to understand the impact elsewhere or if there is a better place to add this fix.
Any new live queries seem to take ownership of existing rows when mounting. Meaning that when the new live query unmounts it causes that record to be removed from the collection even if other useLiveQueries still should have that row existing.
I am unsure if this is a similar impact across the other vue/svelte db packages I am currently only using react-db so any advise if this change would negatively impact other areas would be helpful.
One thing noticed - my team have observed that the row will re-appear in the correct live query if we force a re-render during local development
Summary by CodeRabbit
useLiveQuerydoes not remove records from another still-mounted, unfiltered listuseLiveQuery.