feat(vue-db): add infinite query binding - #1724
Open
tannerlinsley wants to merge 2 commits into
Open
Conversation
Co-authored-by: miguelrk <miguelromerokaram@gmail.com>
Contributor
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdded the Vue ChangesVue live infinite query
Estimated code review effort: 3 (Moderate) | ~25 minutes Mergeability Score: 🟡 Moderate · up to The new infinite-query binding changes pagination and loading behavior, but required regression coverage for no-op promises, nullish inputs, and concurrent fetches is still missing; merge should wait for those tests or explicit owner acceptance. Sequence Diagram(s)sequenceDiagram
participant VueComponent
participant useLiveInfiniteQuery
participant LiveQueryCollection
participant WindowController
VueComponent->>useLiveInfiniteQuery: provide query input and reactive dependencies
useLiveInfiniteQuery->>LiveQueryCollection: resolve or create bounded collection
useLiveInfiniteQuery->>WindowController: create controller
WindowController-->>useLiveInfiniteQuery: publish snapshots
useLiveInfiniteQuery-->>VueComponent: expose reactive pages and state
VueComponent->>useLiveInfiniteQuery: call fetchNextPage()
useLiveInfiniteQuery->>WindowController: fetch next page
Possibly related PRs
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: 0 B Total Size: 132 kB ℹ️ View Unchanged
|
![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 (1)
packages/vue-db/src/useLiveInfiniteQuery.ts (1)
132-162: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
UtilsRecordfor the utility constraint.Replace both
Record<string, any>constraints withUtilsRecord. This removes directanyfrom the exported API and keeps the constraint aligned withCollection.Proposed fix
- TUtils extends Record<string, any>, + TUtils extends UtilsRecord, ... - TUtils extends Record<string, any>, + TUtils extends UtilsRecord,As per coding guidelines,
**/*.{ts,tsx}: “Avoid usinganytypes; useunknowninstead when the type is truly unknown.”🤖 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/vue-db/src/useLiveInfiniteQuery.ts` around lines 132 - 162, Replace the TUtils constraints using Record<string, any> in UseLiveInfiniteQueryReturnWithCollection and useLiveInfiniteQuery with the existing UtilsRecord type, keeping the exported API aligned with Collection and removing direct any usage.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/vue-db/src/useLiveInfiniteQuery.ts`:
- Around line 53-61: Update normalizePageSize to reject values that would
overflow the later peek-ahead increment, not merely values that are safe
integers. Use the existing peek-ahead limit or equivalent upper-bound symbol
when validating pageSize, while preserving DEFAULT_PAGE_SIZE for invalid inputs
and accepting valid positive sizes below that boundary.
In `@packages/vue-db/tests/useLiveInfiniteQuery.test.ts`:
- Around line 172-194: Expand the pagination boundary tests around the existing
“handles $label pagination boundaries” cases to cover resolved no-op behavior
from fetchNextPage() after the final page, concurrent fetchNextPage() calls
before the first resolves with only one page request applied, and
initialPageParam values of undefined and null both defaulting to 0. Assert the
returned promise and query data remain unchanged where applicable, reusing the
existing posts/query helpers and page expectations.
---
Nitpick comments:
In `@packages/vue-db/src/useLiveInfiniteQuery.ts`:
- Around line 132-162: Replace the TUtils constraints using Record<string, any>
in UseLiveInfiniteQueryReturnWithCollection and useLiveInfiniteQuery with the
existing UtilsRecord type, keeping the exported API aligned with Collection and
removing direct any usage.
🪄 Autofix
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 Plus
Run ID: 9c09b24f-0330-4d82-bed2-6da26b9862e6
📒 Files selected for processing (5)
.changeset/curly-ravens-listen.mdpackages/vue-db/src/index.tspackages/vue-db/src/useLiveInfiniteQuery.tspackages/vue-db/tests/useLiveInfiniteQuery.test-d.tspackages/vue-db/tests/useLiveInfiniteQuery.test.ts
Comment on lines
+172
to
+194
| it.each([ | ||
| { label: `empty`, count: 0, pageSize: 5, pageLengths: [0], limit: 6 }, | ||
| { label: `single row`, count: 1, pageSize: 5, pageLengths: [1], limit: 6 }, | ||
| { | ||
| label: `zero page size`, | ||
| count: 1, | ||
| pageSize: 0, | ||
| pageLengths: [1], | ||
| limit: 21, | ||
| }, | ||
| ])( | ||
| `handles $label pagination boundaries`, | ||
| async ({ label, count, pageSize, pageLengths, limit }) => { | ||
| const posts = createPostsCollection(`vue-infinite-${label}`, count) | ||
| const query = mountPostsQuery(posts, { pageSize }) | ||
| await flushVue() | ||
|
|
||
| expect(query.pages.value.map((page) => page.length)).toEqual(pageLengths) | ||
| expect(query.hasNextPage.value).toBe(false) | ||
| expect( | ||
| (query.collection.value.utils as LiveQueryCollectionUtils).getWindow(), | ||
| ).toEqual({ offset: 0, limit }) | ||
| }, |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Cover the remaining required pagination corner cases.
The boundary tests only assert hasNextPage is false. Add a test that calls fetchNextPage() after the final page and asserts that the promise resolves with unchanged data. Add a test that starts two fetchNextPage() calls before the first resolves and asserts that only one page request applies. Add runtime coverage for initialPageParam: undefined and initialPageParam: null, which both default to 0.
As per coding guidelines, “Test corner cases including: empty arrays/sets, single-element collections, undefined vs null values, resolved promises, async race conditions, and limit/offset edge cases.”
🤖 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/vue-db/tests/useLiveInfiniteQuery.test.ts` around lines 172 - 194,
Expand the pagination boundary tests around the existing “handles $label
pagination boundaries” cases to cover resolved no-op behavior from
fetchNextPage() after the final page, concurrent fetchNextPage() calls before
the first resolves with only one page request applied, and initialPageParam
values of undefined and null both defaulting to 0. Assert the returned promise
and query data remain unchanged where applicable, reusing the existing
posts/query helpers and page expectations.
Source: Coding guidelines
Contributor
|
Size Change: 0 B Total Size: 3.79 kB ℹ️ View Unchanged
|
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
useLiveInfiniteQueryto@tanstack/vue-dbThis replaces the duplicated pagination implementation in #1513 while preserving its API intent. Miguel Romero Karam is credited as a co-author on the commit.
Validation
pnpm --filter @tanstack/vue-db test(68 tests, no type errors)pnpm --filter @tanstack/vue-db buildgit diff --checkChecklist
pnpm --filter @tanstack/vue-db test.Release Impact
Summary by CodeRabbit
useLiveInfiniteQueryfor reactive infinite pagination with live data.