feat(db): ordered snapshot / layout-revision contract (RFC #1623 phase 4)#1669
Open
kevin-dp wants to merge 4 commits into
Open
feat(db): ordered snapshot / layout-revision contract (RFC #1623 phase 4)#1669kevin-dp wants to merge 4 commits into
kevin-dp wants to merge 4 commits into
Conversation
…1623 phase 4) An `orderBy` live query that reorders its rows without changing any projected row value (an "order-only move") was swallowed by the collection's value-diff: `.values()`/`.entries()` re-sorted, but no change event fired, so subscribers kept the stale order. This is the last universal expected-fail in the cross-adapter conformance suite (issue #1601). Phase 4 of the live-query platform RFC calls for an explicit layout-revision contract rather than a forged row `update`. This does that: - The live-query flush captures the retracted side of each change and, after commit, detects an order-only move (value deep-equal, `orderByIndex` moved) and publishes a first-class empty layout-change notification via a new `CollectionChangesManager.emitLayoutChangeEvent()`. - The shared observer snapshot gains `layoutRevision`, which increments on any visible membership, ordering, or order-only-move change. All five adapters pick this up through their existing wholesale re-read, so the `order-only-move` conformance scenario is removed from UNIVERSAL_EXPECTED_FAIL and now passes on React, Vue, Svelte, Solid, and Angular. Distinct from PR #1601 (v-anton), which fixes the same bug via a forced row `update`; this uses the RFC's layout-revision approach instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
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: +413 B (+0.33%) Total Size: 127 kB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
|
Size Change: 0 B Total Size: 3.81 kB ℹ️ View Unchanged
|
Addresses independent review of the layoutRevision contract: - The join-with-separator signature could collide: a key value equal to the concatenation of neighboring keys around the separator produces the same string as two separate keys, so a real layout change (a membership change whose combined key spans the separator) was missed. Compare the ordered key sequence directly instead - collision-free, and it avoids materializing a large string on every snapshot rebuild (a new key array is only allocated when the layout actually moved). Adds a regression test. - Correct the layoutRevision doc comment: it is NOT in lockstep with snapshot identity (a value-only update yields a new snapshot but the same layoutRevision). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Phase 4 of the live-query platform RFC (#1623): the ordered snapshot / layout contract. Stacked on #1642 (observer migration) — review/merge that first; this PR's base is
refactor/live-query-observerso the diff is Phase 4 only.Problem
An
orderBylive query that reorders its rows without changing any projected row value (an "order-only move") is swallowed by the collection's value-diff:.values()/.entries()re-sort internally, but no change event fires, souseLiveQuerykeeps rendering the stale order. This is the last universal expected-fail in the cross-adapter conformance suite (tracked as #1601).Approach
The RFC is explicit that this should be "an explicit layout-revision requirement, not a hidden forced-update path". So instead of forging a row
update:commit(), detects an order-only move (projected value deep-equal,orderByIndexmoved) and publishes a first-class empty layout-change notification via a newCollectionChangesManager.emitLayoutChangeEvent(). It reuses the existing empty-batch delivery already used for the ready signal — subscribers re-read, nothing is faked.layoutRevision, which increments on any visible membership, ordering, or order-only-move change. This is the canonical contract the RFC wants for future fine-grained materializers; adapters currently pick up the reorder through their existing wholesale re-read.Result
order-only-moveis removed fromUNIVERSAL_EXPECTED_FAILand now passes on all five adapters (React, Vue, Svelte, Solid, Angular) — 26/26 conformance each. Full@tanstack/dbsuite green (2464 tests), all five adapter suites green.Coverage
packages/db/tests/live-query-order-only-move.test.ts— core mechanism: republish +layoutRevisionbump on an order-only move; no bump when order is unchanged (no spurious notification); bump on membership change.order-only-movescenario now a real pass across all adapters.Relationship to #1601
@v-anton's #1601 fixes the same bug via a forced row
update. This PR takes the RFC-sanctioned layout-revision approach instead (a distinct, first-class notification +layoutRevision), so it supersedes rather than duplicates that path. Happy to coordinate on which lands — flagging for maintainer decision.🤖 Generated with Claude Code