Add framework DB adapter package#1699
Open
jherr wants to merge 4 commits into
Open
Conversation
Add a new framework integration package with live query hooks, tests, docs, and monorepo registration aligned with existing react-db patterns. Co-authored-by: Cursor <cursoragent@cursor.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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds ChangesOctane DB adapter
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant OctaneComponent
participant useLiveQuery
participant Collection
participant ReactStore
OctaneComponent->>useLiveQuery: invoke query hook
useLiveQuery->>Collection: create or reuse live query
useLiveQuery->>ReactStore: subscribe and read snapshot
Collection-->>ReactStore: publish live changes
ReactStore-->>OctaneComponent: render updated data
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
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/octane-db
@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: 125 kB ℹ️ View Unchanged
|
Contributor
|
Size Change: 0 B Total Size: 4.22 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: 8
🧹 Nitpick comments (11)
packages/octane-db/src/useLiveInfiniteQuery.ts (1)
182-191: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
JSON.stringifyfor dep comparison rejects deps thatuseLiveQueryaccepts.
useLiveQuerycompares deps by element identity, so a collection, aDate, a function, or any non-serializable value is a valid dep there. Here the same array throws. Beyond the divergence, stringifying on every render is O(size of deps) and treats{a:1,b:2}and{b:2,a:1}as different.Element-wise identity comparison (as
useLiveQueryalready does) removes the whole failure mode.As per coding guidelines: "Be mindful of time complexity in algorithms".
♻️ Proposed refactor
- // Track deps for query functions (stringify for comparison) - let depsKey: string - try { - depsKey = JSON.stringify(deps) - } catch { - throw new Error( - `useLiveInfiniteQuery: dependency array contains values that cannot be serialized (e.g. circular references). ` + - `Ensure all dependency values are JSON-serializable.`, - ) - } - const prevDepsKeyRef = useRef(depsKey, subSlot(slot, `deps-key-ref`)) + // Track deps for query functions (identity comparison, matching useLiveQuery) + const prevDepsRef = useRef<Array<unknown>>(deps, subSlot(slot, `deps-ref`)) + const depsChanged = + prevDepsRef.current.length !== deps.length || + prevDepsRef.current.some((dep, i) => dep !== deps[i])The effect at lines 204-213 then keys off
depsChangedand assignsprevDepsRef.current = deps. Note this also makes thethrows a descriptive error when deps contain non-serializable valuestest atpackages/octane-db/tests/useLiveInfiniteQuery.test.tsxlines 1876-1906 obsolete.🤖 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/octane-db/src/useLiveInfiniteQuery.ts` around lines 182 - 191, Replace the JSON.stringify-based dependency key logic in useLiveInfiniteQuery with element-wise identity comparison matching useLiveQuery, using the existing previous-dependencies ref and deriving depsChanged from that comparison. Preserve the effect’s update behavior by assigning the current deps to prevDepsRef.current after processing, and remove the obsolete serialization-error path and test.Source: Coding guidelines
packages/octane-db/tests/useLiveInfiniteQuery.test.tsx (2)
1640-1670: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSkipped test leaves the
orderByvalidation error path uncovered.The throw at
packages/octane-db/src/useLiveInfiniteQuery.tslines 240-244 now has no coverage. Since Octane surfaces it from a passive effect rather than during render, assert on it via the effect instead ofexpect(renderHook).toThrow— e.g. spy on the error channel after flushing effects, or expose the validation as a synchronous check during render so it behaves like the other input validation at lines 154-159.Want me to write the effect-based version of this test, or open an issue to track it?
As per coding guidelines: "Always add unit tests that reproduce a bug before fixing it to ensure the bug is fixed and prevent regression".
🤖 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/octane-db/tests/useLiveInfiniteQuery.test.tsx` around lines 1640 - 1670, Replace the skipped test around useLiveInfiniteQuery with an active regression test for the missing-orderBy validation. Because the error is emitted from a passive effect rather than render, flush effects and assert through the established error channel instead of using expect(renderHook).toThrow; preserve the existing collection setup and ORDER BY/setWindow() validation expectation.Source: Coding guidelines
10-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a shared test entry instead of importing
@tanstack/dbinternals.
mockSyncCollectionOptionsis only exported frompackages/db/tests/utils.js, andcreateFilterFunctionFromExpressioncomes frompackages/db/src/collection/change-events.js, so this adapter test is coupled to another package’s internal path instead of using the package’s public API or a shared test-utils entry.🤖 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/octane-db/tests/useLiveInfiniteQuery.test.tsx` around lines 10 - 11, Update useLiveInfiniteQuery.test.tsx to stop importing mockSyncCollectionOptions and createFilterFunctionFromExpression through packages/db test or source internals; use the shared test entry or public `@tanstack/db` API that exposes these helpers instead, while preserving the test behavior.packages/octane-db/tests/useLiveSuspenseQuery.test.tsx (2)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
children: unknownis looser than needed. Use Octane's node type (e.g.JSX.Element/ the framework'sNodetype) so the wrappers get real type checking on what they render.🤖 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/octane-db/tests/useLiveSuspenseQuery.test.tsx` at line 13, Update the ChildrenProps type used by the test wrappers to replace children: unknown with Octane’s node type, such as JSX.Element or the framework’s established Node type, so rendered children receive proper type checking.Source: Coding guidelines
662-668: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest reaches into
collection._syncinternals. Underscore-prefixed access couples this suite to private plumbing; if@tanstack/dbexposes (or can expose) a public helper for tracking a load promise, prefer that.🤖 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/octane-db/tests/useLiveSuspenseQuery.test.tsx` around lines 662 - 668, Update the test around the live query collection’s load-promise setup to use a public API for tracking the promise instead of reaching through collection._sync.trackLoadPromise. Reuse an existing exposed helper if available, or add the smallest public helper needed, while preserving the assertions that isLoadingSubset is true and status remains ready.Source: Coding guidelines
packages/octane-db/tests/usePacedMutations.test.tsx (2)
44-47: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
let txis implicitlyany. Annotate asTransaction<Item>(orTransaction<Item> | undefined) sotx!.stateandtx!.isPersistedare actually type-checked.Also applies to: 83-86, 121-124
🤖 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/octane-db/tests/usePacedMutations.test.tsx` around lines 44 - 47, Annotate the transaction variables assigned from result.current in the affected usePacedMutations test cases as Transaction<Item> (or Transaction<Item> | undefined), including the cases around lines 44, 83, and 121, so subsequent tx!.state and tx!.isPersisted accesses are type-checked.Source: Coding guidelines
21-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the identical "create + ready a collection" setup. The same six lines (create with
mockSyncCollectionOptionsNoInitialState, preload, begin/commit/markReady, await) appear verbatim in all three smoke tests, and all three reuse the idtest. A small helper taking a unique id removes the duplication and the id collision.♻️ Suggested helper
+async function createReadyCollection(id: string) { + const collection = createCollection( + mockSyncCollectionOptionsNoInitialState<Item>({ + id, + getKey: (item) => item.id, + }), + ) + const preloadPromise = collection.preload() + collection.utils.begin() + collection.utils.commit() + collection.utils.markReady() + await preloadPromise + return collection +}- const collection = createCollection( - mockSyncCollectionOptionsNoInitialState<Item>({ - id: `test`, - getKey: (item) => item.id, - }), - ) - - const preloadPromise = collection.preload() - collection.utils.begin() - collection.utils.commit() - collection.utils.markReady() - await preloadPromise + const collection = await createReadyCollection(`paced-debounce`)Also applies to: 60-71, 98-109
🤖 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/octane-db/tests/usePacedMutations.test.tsx` around lines 21 - 32, Extract the repeated collection initialization sequence from the three smoke tests into a shared async helper that accepts a unique collection id, creates the collection with mockSyncCollectionOptionsNoInitialState, preloads it, runs begin/commit/markReady, awaits preload completion, and returns the ready collection. Replace each duplicated setup with this helper and pass a distinct id for each test instead of reusing “test”.Source: Coding guidelines
packages/octane-db/tests/useLiveQuery.test.tsx (2)
1227-1228: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThree comments state the opposite of the assertion they precede. Line 1227 says "isLoading should be true" before
expect(...isLoading).toBe(false); Line 1463 says "transitions from false to true" (it's true→false); Line 1506 says "Initially should be false" beforeexpect(...).toBe(true). Inverted comments are worse than none here.Also applies to: 1462-1463, 1506-1507
🤖 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/octane-db/tests/useLiveQuery.test.tsx` around lines 1227 - 1228, Correct the three misleading comments in the live-query tests: update the comment near the pre-created syncing collection to state that isLoading is false, the transition comment near line 1463 to describe true-to-false behavior, and the initial-state comment near line 1506 to state that isLoading is true. Leave the assertions unchanged.Source: Coding guidelines
90-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate collection ids reused across independent tests. Both ported suites create multiple collections under the same ids, which muddies debugging and risks interference if any id-keyed registry behavior changes.
packages/octane-db/tests/useLiveQuery.test.tsx#L90-L90: give each collection a unique id instead of reusingtest-persons(L90, L126) andtest-persons-2(L168, L202, L237, L274).packages/octane-db/tests/useLiveQuery.test-d.tsx#L27-L27: give each of the four type tests its own id instead of reusingtest-persons-2(L27, L50, L74, L104).🤖 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/octane-db/tests/useLiveQuery.test.tsx` at line 90, Use distinct collection ids for every independent test in packages/octane-db/tests/useLiveQuery.test.tsx, replacing repeated test-persons and test-persons-2 values at all referenced locations. Also assign unique ids to each of the four type tests in packages/octane-db/tests/useLiveQuery.test-d.tsx instead of reusing test-persons-2; no other test behavior needs to change.packages/octane-db/tests/conformance.test.tsx (1)
1-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStale React references in the Octane driver docs. The header says the realm-sensitive imports come from the "React package's
@tanstack/db", and Line 139 calls null-return "React's disabled convention". Retitle both to Octane to avoid confusion for the next porter.♻️ Suggested wording
- * Everything realm-sensitive — collection creation and query operators — is - * imported here (React package's `@tanstack/db`) and handed to the shared - * scenarios, so instances match what this package's `useLiveQuery` expects. + * Everything realm-sensitive — collection creation and query operators — is + * imported here (this package's `@tanstack/db` realm) and handed to the shared + * scenarios, so instances match what this package's `useLiveQuery` expects.- // React's disabled convention: the query callback returns null. + // Octane adapter's disabled convention: the query callback returns 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/octane-db/tests/conformance.test.tsx` around lines 1 - 11, Update the Octane driver documentation to remove stale React references: revise the header comment to identify the imports as coming from Octane’s `@tanstack/db`, and update the null-return wording near the disabled-convention documentation to refer to Octane rather than React. Keep the documented behavior unchanged.Source: Coding guidelines
packages/octane-db/tests/useLiveQuery.test-d.tsx (1)
42-44: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign
expectTypeOfmatchers totoExtend. This file already usestoExtendfor collection assertions, but thedataassertions still use the deprecatedtoMatchTypeOf; replace the remaining occurrences at lines 42, 66, 96, and 124.🤖 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/octane-db/tests/useLiveQuery.test-d.tsx` around lines 42 - 44, Replace the deprecated toMatchTypeOf matcher with toExtend in the data assertions of useLiveQuery tests at the referenced occurrences, while preserving each existing expected type and assertion structure.
🤖 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 `@docs/framework/octane/overview.md`:
- Around line 26-33: Update the Octane documentation examples, including
TodoList and the snippets around postsCollection, category, and gt, so every
referenced identifier is either declared in the example setup or imported from
the appropriate package. Add only the minimal shared setup needed, or explicitly
indicate that values are defined earlier, and ensure each snippet compiles as
written.
In `@packages/octane-db/skills/octane-db/SKILL.md`:
- Around line 220-232: Update the comment following the useLiveQuery projection
to document project.issues as an array of objects containing id and title,
matching the objects selected inside toArray, rather than string[].
- Around line 135-143: Update the useLiveInfiniteQuery call to either apply
category in the query predicate so changing category affects results, or remove
category from the dependency array if the query is intentionally
category-independent; keep the remaining pagination and ordering behavior
unchanged.
- Around line 19-23: Register useLiveQueryEffect consistently across the Octane
metadata: in packages/octane-db/skills/octane-db/SKILL.md at lines 19-23 add
useLiveQueryEffect.ts to the source list, and at lines 57-59 add its hook
section; in _artifacts/skill_tree.yaml at lines 172-185 include the hook in the
description and source references; in domain_map.yaml at lines 306-308 add it to
config_surface.
In `@packages/octane-db/src/slot.ts`:
- Around line 23-29: Adopt splitTrailingSlot as the single non-mutating
implementation for trailing slot extraction. In
packages/octane-db/src/slot.ts:23-29, retain or extend splitTrailingSlot to
provide the parsed argument array needed by callers; update
packages/octane-db/src/useLiveQuery.ts:322-326,
packages/octane-db/src/useLiveQueryEffect.ts:39-42,
packages/octane-db/src/useLiveSuspenseQuery.ts:160-164,
packages/octane-db/src/usePacedMutations.ts:101-104, and
packages/octane-db/src/useLiveInfiniteQuery.ts:141-145 to call it, using the
returned args for deps where required and eliminating inline rest-array
mutation.
In `@packages/octane-db/src/useLiveInfiniteQuery.ts`:
- Around line 18-22: Update isLiveQueryCollectionUtils to handle null and
undefined inputs before accessing setWindow, returning false for absent utils
while preserving the existing function-type check for populated values.
- Around line 147-148: Update the pageSize defaulting in useLiveInfiniteQuery to
use nullish coalescing so an explicit 0 is not silently replaced with 20. If
pageSize 0 is invalid, add explicit validation and reject it rather than
allowing empty-page fetchNextPage loops; leave initialPageParam unchanged.
In `@packages/octane-db/src/useLiveSuspenseQuery.ts`:
- Around line 170-184: Namespace nested useLiveQuery calls to prevent ref-slot
collisions: in packages/octane-db/src/useLiveSuspenseQuery.ts lines 170-184,
pass subSlot(slot, 'lq') to the useLiveQuery call; in
packages/octane-db/src/useLiveInfiniteQuery.ts lines 172-175, pass the same
namespaced slot to both useLiveQuery calls. Preserve the existing
collection-change reset logic in each wrapper.
---
Nitpick comments:
In `@packages/octane-db/src/useLiveInfiniteQuery.ts`:
- Around line 182-191: Replace the JSON.stringify-based dependency key logic in
useLiveInfiniteQuery with element-wise identity comparison matching
useLiveQuery, using the existing previous-dependencies ref and deriving
depsChanged from that comparison. Preserve the effect’s update behavior by
assigning the current deps to prevDepsRef.current after processing, and remove
the obsolete serialization-error path and test.
In `@packages/octane-db/tests/conformance.test.tsx`:
- Around line 1-11: Update the Octane driver documentation to remove stale React
references: revise the header comment to identify the imports as coming from
Octane’s `@tanstack/db`, and update the null-return wording near the
disabled-convention documentation to refer to Octane rather than React. Keep the
documented behavior unchanged.
In `@packages/octane-db/tests/useLiveInfiniteQuery.test.tsx`:
- Around line 1640-1670: Replace the skipped test around useLiveInfiniteQuery
with an active regression test for the missing-orderBy validation. Because the
error is emitted from a passive effect rather than render, flush effects and
assert through the established error channel instead of using
expect(renderHook).toThrow; preserve the existing collection setup and ORDER
BY/setWindow() validation expectation.
- Around line 10-11: Update useLiveInfiniteQuery.test.tsx to stop importing
mockSyncCollectionOptions and createFilterFunctionFromExpression through
packages/db test or source internals; use the shared test entry or public
`@tanstack/db` API that exposes these helpers instead, while preserving the test
behavior.
In `@packages/octane-db/tests/useLiveQuery.test-d.tsx`:
- Around line 42-44: Replace the deprecated toMatchTypeOf matcher with toExtend
in the data assertions of useLiveQuery tests at the referenced occurrences,
while preserving each existing expected type and assertion structure.
In `@packages/octane-db/tests/useLiveQuery.test.tsx`:
- Around line 1227-1228: Correct the three misleading comments in the live-query
tests: update the comment near the pre-created syncing collection to state that
isLoading is false, the transition comment near line 1463 to describe
true-to-false behavior, and the initial-state comment near line 1506 to state
that isLoading is true. Leave the assertions unchanged.
- Line 90: Use distinct collection ids for every independent test in
packages/octane-db/tests/useLiveQuery.test.tsx, replacing repeated test-persons
and test-persons-2 values at all referenced locations. Also assign unique ids to
each of the four type tests in packages/octane-db/tests/useLiveQuery.test-d.tsx
instead of reusing test-persons-2; no other test behavior needs to change.
In `@packages/octane-db/tests/useLiveSuspenseQuery.test.tsx`:
- Line 13: Update the ChildrenProps type used by the test wrappers to replace
children: unknown with Octane’s node type, such as JSX.Element or the
framework’s established Node type, so rendered children receive proper type
checking.
- Around line 662-668: Update the test around the live query collection’s
load-promise setup to use a public API for tracking the promise instead of
reaching through collection._sync.trackLoadPromise. Reuse an existing exposed
helper if available, or add the smallest public helper needed, while preserving
the assertions that isLoadingSubset is true and status remains ready.
In `@packages/octane-db/tests/usePacedMutations.test.tsx`:
- Around line 44-47: Annotate the transaction variables assigned from
result.current in the affected usePacedMutations test cases as Transaction<Item>
(or Transaction<Item> | undefined), including the cases around lines 44, 83, and
121, so subsequent tx!.state and tx!.isPersisted accesses are type-checked.
- Around line 21-32: Extract the repeated collection initialization sequence
from the three smoke tests into a shared async helper that accepts a unique
collection id, creates the collection with
mockSyncCollectionOptionsNoInitialState, preloads it, runs
begin/commit/markReady, awaits preload completion, and returns the ready
collection. Replace each duplicated setup with this helper and pass a distinct
id for each test instead of reusing “test”.
🪄 Autofix (Beta)
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: a9941cbc-6d8f-4dea-93f1-cb4c32476bdb
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
.changeset/octane-db-package.md_artifacts/skill_tree.yamldocs/framework/octane/overview.mddocs/framework/octane/reference/index.mddocs/installation.mddomain_map.yamlpackages/db/src/live-query-adapter.tspackages/octane-db/CHANGELOG.mdpackages/octane-db/README.mdpackages/octane-db/package.jsonpackages/octane-db/skills/octane-db/SKILL.mdpackages/octane-db/src/index.tspackages/octane-db/src/slot.tspackages/octane-db/src/useLiveInfiniteQuery.tspackages/octane-db/src/useLiveQuery.tspackages/octane-db/src/useLiveQueryEffect.tspackages/octane-db/src/useLiveSuspenseQuery.tspackages/octane-db/src/usePacedMutations.tspackages/octane-db/tests/conformance.test.tsxpackages/octane-db/tests/test-setup.tspackages/octane-db/tests/useLiveInfiniteQuery.test.tsxpackages/octane-db/tests/useLiveQuery.eager-onstorechange.test.tsxpackages/octane-db/tests/useLiveQuery.test-d.tsxpackages/octane-db/tests/useLiveQuery.test.tsxpackages/octane-db/tests/useLiveQueryEffect.test.tsxpackages/octane-db/tests/useLiveSuspenseQuery.test.tsxpackages/octane-db/tests/usePacedMutations.test.tsxpackages/octane-db/tsconfig.docs.jsonpackages/octane-db/tsconfig.jsonpackages/octane-db/vite.config.tsscripts/generate-docs.tsskill_spec.md
- Fix critical slot collision: useLiveSuspenseQuery and useLiveInfiniteQuery passed their own slot symbol verbatim into useLiveQuery, so both derived the same subSlot(slot, 'coll-ref') symbol and aliased one ref cell — defeating each wrapper's "did the collection change?" reset. Namespace the nested call with subSlot(slot, 'lq'). - Guard isLiveQueryCollectionUtils against null/undefined utils before reading setWindow. - useLiveInfiniteQuery: treat pageSize with ?? (not ||) and reject pageSize <= 0 instead of silently defaulting 0 to 20. - Adopt the previously-unused splitTrailingSlot helper across all five hooks instead of re-implementing the trailing-slot extraction inline. - Docs/metadata: make octane overview examples self-contained, register useLiveQueryEffect in SKILL.md/skill_tree.yaml/domain_map.yaml, use category in the infinite-query example, and correct the toArray() result type comment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/framework/octane/overview.md`:
- Around line 70-83: Update the useLiveInfiniteQuery example to demonstrate
pagination by adding a “Load more” control that is rendered only when
hasNextPage is true and invokes fetchNextPage when activated; keep the existing
query and page-parameter logic unchanged.
🪄 Autofix (Beta)
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: 61b61441-d0d3-4c4b-b7e7-8776228b6c34
📒 Files selected for processing (9)
_artifacts/skill_tree.yamldocs/framework/octane/overview.mddomain_map.yamlpackages/octane-db/skills/octane-db/SKILL.mdpackages/octane-db/src/useLiveInfiniteQuery.tspackages/octane-db/src/useLiveQuery.tspackages/octane-db/src/useLiveQueryEffect.tspackages/octane-db/src/useLiveSuspenseQuery.tspackages/octane-db/src/usePacedMutations.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- domain_map.yaml
- _artifacts/skill_tree.yaml
- packages/octane-db/skills/octane-db/SKILL.md
- packages/octane-db/src/useLiveQueryEffect.ts
- packages/octane-db/src/usePacedMutations.ts
- packages/octane-db/src/useLiveInfiniteQuery.ts
- packages/octane-db/src/useLiveSuspenseQuery.ts
- packages/octane-db/src/useLiveQuery.ts
Address CodeRabbit follow-up: the infinite-query example destructured fetchNextPage/hasNextPage but never used them. Render the posts and a guarded "Load more" button, and drop the unused `pages` binding. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
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.
Summary
Test plan
Made with Cursor
Summary by CodeRabbit