Infinite scroll (#2334) [bump version to v0.22.165]#2334
Merged
drew-harris merged 30 commits intomainfrom Mar 19, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds bidirectional cursor-based infinite-query support across React and core: new useInfiniteQuerySubscription hook, useInfiniteQuery API, cursor/UUID arithmetic, end-cursor handling in instaql, stronger Deferred typings, weakHash change, TS target bumps, and an infinite-scroll demo page. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant React as "React Component"
participant Hook as "useInfiniteQuerySubscription"
participant Core as "InstantDB Core"
participant DB as "Database"
User->>React: mount / call loadMore
React->>Hook: useInfiniteQuery(entity, query)
Hook->>Core: subscribeQuery(initial query with limit/order)
Core->>DB: execute query (limit, order, after/before)
DB-->>Core: data + pageInfo (endCursor, hasNextPage)
Core-->>Hook: onData(chunk, pageInfo)
Hook->>Hook: update forward & reverse chunks
Hook-->>React: return InfiniteQueryResult (data, chunks)
User->>React: click loadMore
React->>Hook: loadMore()
Hook->>Hook: freeze forward, compute next cursor
Hook->>Core: subscribeQuery(next page after endCursor)
DB-->>Core: next data + pageInfo
Core-->>Hook: onData(new chunk)
Hook-->>React: update data with appended chunk
sequenceDiagram
participant Item as "Item"
participant Compare as "compareToCursor"
participant Trip as "compareOrderTriples"
Item->>Compare: compare item vs cursor
Compare->>Trip: compareOrderTriples(order, itemAttr, cursorAttr)
Trip-->>Compare: -1 | 0 | 1
Compare->>Item: numeric result used for boundary checks (isBefore/isAfter)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Contributor
|
View Vercel preview at instant-www-js-drewh-infinite-scroll-jsv.vercel.app. |
drew-harris
force-pushed
the
drewh/infinite-scroll
branch
from
dab91d3 to
c259753
Compare
![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: 6
🧹 Nitpick comments (2)
client/packages/core/src/utils/Deferred.ts (1)
1-4:Deferred<T>still leaksanythrough method signatures.Line 13 and Line 17 are untyped, so the generic
Tis not enforced at call sites.♻️ Proposed typing cleanup
export class Deferred<T = any> { promise: Promise<T>; _resolve: (value: T) => void; - _reject: (...reason: any) => void; + _reject: (reason?: any) => void; @@ - resolve(value) { + resolve(value: T) { this._resolve(value); } - reject(reason) { + reject(reason?: any) { this._reject(reason); } }Also applies to: 13-18
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/core/src/utils/Deferred.ts` around lines 1 - 4, The Deferred<T> class currently leaks any through its resolver signatures; update the field types so they enforce the generic T: change _resolve to (value: T | PromiseLike<T>) => void and change _reject to (reason?: any) => void (or use unknown for stricter typing) while keeping promise: Promise<T>; ensure the constructor initializes promise and assigns these typed functions to _resolve and _reject so callers get proper T typing when using Deferred<T>.client/packages/react-common/src/uuidMath.ts (1)
1-1: Use a type-only import forCursor.
Cursoris used only in type annotations (lines 3 and 7), never as a runtime value. Usingimport typefollows best practices and prevents potential issues if stricter TypeScript settings are enabled.-import { Cursor } from '@instantdb/core'; +import type { Cursor } from '@instantdb/core';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/react-common/src/uuidMath.ts` at line 1, Replace the runtime import of Cursor with a type-only import since Cursor is only used in type annotations in uuidMath.ts; change the import statement to use "import type { Cursor } from '@instantdb/core'" and ensure no runtime references to Cursor remain in the functions or signatures (e.g., any function parameter or return type annotations that mention Cursor) so TypeScript treats it purely as a type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/packages/core/src/instaql.ts`:
- Around line 646-653: The isBefore/isAfter functions rely on exact equality to
1 or -1 from compareToCursor but must use sign checks because comparators can
return any positive/negative integer; update isBefore to compute const cmp =
compareToCursor(startCursor, orderAttr, idVec) and return direction === 'desc' ?
cmp > 0 : cmp < 0, and update isAfter to compute const cmp =
compareToCursor(endCursor, orderAttr, idVec) and return direction === 'desc' ?
cmp < 0 : cmp > 0 (i.e., replace === 1 / === -1 checks with >0 / <0 sign
comparisons in isBefore/isAfter).
In `@client/packages/core/src/queryTypes.ts`:
- Around line 498-516: ValidInfiniteQueryObject lacks the same keyof T guard
used by ValidQueryObject, so callers can pass extra arbitrary keys; update
ValidInfiniteQueryObject to enforce "keyof T extends keyof
Schema['entities'][EntityName]['links'] | '$'" (or use a conditional type that
yields never when the constraint fails) so only link keys and '$' are allowed,
while preserving the existing $ shape (where, pageSize, fields, order) and the
per-link nested ValidQueryObject recursion.
In `@client/packages/react-common/src/useInfiniteQuerySubscription.ts`:
- Around line 104-110: The subscription callbacks (e.g., the one passing
frozenData into setReverseChunk) are dereferencing frozenData.data and
frozenData.pageInfo with non-null assertions and will crash on error emissions;
update each callback (the one using frozenData at the shown location and the
similar callbacks around lines 131-136, 158-163, 193-198, 218-225) to first
check frozenData && frozenData.data and frozenData.pageInfo before accessing
properties, and gracefully handle missing fields (for example set status to
'error' or return early) so setReverseChunk and other setters never receive
undefined.data or undefined.pageInfo. Ensure you reference the same identifiers
(frozenData, data, pageInfo, setReverseChunk) when implementing the guards so
the fixes apply to all affected callbacks.
- Around line 290-292: The returned loading flags are hardcoded false; update
the return in useInfiniteQuerySubscription to compute isLoading and
isLoadingMore from the hook's actual state (e.g., use the internal flags like
loading/initialLoading or isFetching/isFetchingNextPage from the query result
used by useInfiniteQuerySubscription) and keep loadMore as-is; specifically, set
isLoading to the initial fetch indicator (e.g., loading || isFetching when no
pages loaded) and set isLoadingMore to the next-page fetch indicator (e.g.,
isFetchingNextPage or a local isLoadingMore state used around the loadMore
function) so consumers can distinguish bootstrap vs. pagination loading.
- Around line 204-231: The effect currently using core.subscribeQuery should
list its true dependencies (core, entity, query, pushNewForward, pushNewReverse)
instead of an empty array so a new subscription is created whenever any of those
change; on cleanup it must unsubscribe the previous subscription and reset
hasKickstarted.current so state doesn't become stale (refer to the useEffect
block, core.subscribeQuery, hasKickstarted, pushNewForward, pushNewReverse).
Also stop returning hardcoded isLoading/isLoadingMore: compute and return proper
loading flags (update the hook's returned InfiniteQueryResult) based on
subscription state/response lifecycle so the public API matches the type
(replace the hardcoded false values for isLoading and isLoadingMore).
In `@client/packages/react-common/src/uuidMath.ts`:
- Around line 11-48: The decrementUuid and incrementUuid functions currently
perform hex arithmetic on possibly-invalid input, producing NaNs and corrupted
output; add a validation step that strips hyphens, verifies the resulting string
is exactly 32 characters and matches /^[0-9a-fA-F]{32}$/, and throw a clear
error (or return/propagate) if validation fails before doing parseInt and
arithmetic; implement this as a small helper (e.g., validateUuidHex or
normalizeUuidHex) and use it at the start of both decrementUuid and
incrementUuid to ensure only valid 32-char hex UUIDs are processed.
---
Nitpick comments:
In `@client/packages/core/src/utils/Deferred.ts`:
- Around line 1-4: The Deferred<T> class currently leaks any through its
resolver signatures; update the field types so they enforce the generic T:
change _resolve to (value: T | PromiseLike<T>) => void and change _reject to
(reason?: any) => void (or use unknown for stricter typing) while keeping
promise: Promise<T>; ensure the constructor initializes promise and assigns
these typed functions to _resolve and _reject so callers get proper T typing
when using Deferred<T>.
In `@client/packages/react-common/src/uuidMath.ts`:
- Line 1: Replace the runtime import of Cursor with a type-only import since
Cursor is only used in type annotations in uuidMath.ts; change the import
statement to use "import type { Cursor } from '@instantdb/core'" and ensure no
runtime references to Cursor remain in the functions or signatures (e.g., any
function parameter or return type annotations that mention Cursor) so TypeScript
treats it purely as a type.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
client/packages/core/src/Reactor.jsclient/packages/core/src/index.tsclient/packages/core/src/instaql.tsclient/packages/core/src/queryTypes.tsclient/packages/core/src/utils/Deferred.tsclient/packages/core/src/utils/weakHash.tsclient/packages/react-common/src/InstantReactAbstractDatabase.tsxclient/packages/react-common/src/useInfiniteQuerySubscription.tsclient/packages/react-common/src/uuidMath.tsclient/packages/react-common/tsconfig.dev.jsonclient/packages/react-common/tsconfig.jsonclient/packages/react/src/index.tsclient/sandbox/react-nextjs/components/EphemeralAppPage.tsxclient/sandbox/react-nextjs/pages/play/infinite-scroll.tsxclient/sandbox/react-nextjs/pages/play/paginated-messages.tsxserver/src/instant/admin/routes.clj
💤 Files with no reviewable changes (1)
- client/sandbox/react-nextjs/pages/play/paginated-messages.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
client/packages/react-common/src/useInfiniteQuerySubscription.ts (2)
224-251:⚠️ Potential issue | 🟠 MajorBootstrap effect has empty dependency array but captures changing values.
The effect uses
core,entity,query, and internally defined functions (pushNewForward,pushNewReverse), but the dependency array is empty. If these props change, the hook will continue using stale subscriptions.Consider adding proper dependencies and resetting state when they change:
useEffect(() => { + // Reset state on prop changes + for (const unsub of subs.current.values()) { + unsub?.(); + } + subs.current.clear(); + setForwardChunks(new Map()); + setBackwardChunks(new Map()); + hasKickstarted.current = false; + const sub = core.subscribeQuery( // ... ); return sub; - }, []); + }, [core, entity, JSON.stringify(query)]);Note: The inner functions (
pushNewForward, etc.) would need to be either memoized withuseCallbackor the effect would need restructuring to avoid stale closures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/react-common/src/useInfiniteQuerySubscription.ts` around lines 224 - 251, The effect that calls core.subscribeQuery (inside the useEffect block) captures changing values (core, entity, query, pushNewForward, pushNewReverse, hasKickstarted) but uses an empty dependency array, causing stale closures; update the dependency array to include core, entity, query (or the specific query.$ props used), and the handler functions, and either memoize pushNewForward and pushNewReverse with useCallback or move their logic inside the effect so closures stay fresh; also ensure you properly reset hasKickstarted and any related state when those dependencies change and return a cleanup that unsubscribes the previous subscription from core.subscribeQuery to avoid leaks.
310-311:⚠️ Potential issue | 🟠 MajorLoading flags are hardcoded to
false, breaking the API contract.Consumers cannot determine loading state with these hardcoded values. Compute them from actual state:
+ const isLoading = forwardChunks.size === 0 && reverseChunks.size === 0; + const isLoadingMore = Array.from(forwardChunks.values()).some( + (chunk) => chunk.status === 'bootstrapping', + ); + return { // ... - isLoading: false, - isLoadingMore: false, + isLoading, + isLoadingMore, loadMore: loadMore, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/react-common/src/useInfiniteQuerySubscription.ts` around lines 310 - 311, The returned flags isLoading and isLoadingMore are hardcoded to false in useInfiniteQuerySubscription; change them to reflect the real hook state by computing isLoading from the query status/isFetching (e.g., status === 'loading' || isFetching) and isLoadingMore from the fetch-next-page state (e.g., isFetchingNextPage || fetchStatus === 'fetchMore' or similar variables used in this hook). Update the return object in useInfiniteQuerySubscription so consumers receive dynamic booleans derived from the existing state variables (status, isFetching, isFetchingNextPage/fetchStatus) instead of constant false.
🧹 Nitpick comments (7)
client/packages/react-common/src/useInfiniteQuerySubscription.ts (7)
69-79: Parameter naming: use camelCase for variables.The parameter
Chunkuses PascalCase, which is conventionally reserved for types/classes. Usechunkinstead.- const setChunk = (startCursor: Cursor, Chunk: Chunk) => { + const setChunk = (startCursor: Cursor, chunk: Chunk) => { setForwardChunks((prev) => - new Map(prev).set(JSON.stringify(startCursor), Chunk), + new Map(prev).set(JSON.stringify(startCursor), chunk), ); }; - const setReverseChunk = (startCursor: Cursor, Chunk: Chunk) => { + const setReverseChunk = (startCursor: Cursor, chunk: Chunk) => { setBackwardChunks((prev) => - new Map(prev).set(JSON.stringify(startCursor), Chunk), + new Map(prev).set(JSON.stringify(startCursor), chunk), ); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/react-common/src/useInfiniteQuerySubscription.ts` around lines 69 - 79, Change the parameter name from PascalCase to camelCase in both setter functions: rename the parameter Chunk to chunk in setChunk and setReverseChunk and update their bodies to use the new variable (e.g., set(JSON.stringify(startCursor), chunk)). This keeps naming consistent with conventions and avoids confusion with type/class names; update any local references inside those functions accordingly.
62-64: Inconsistent naming between state and setter.The state variable is
reverseChunksbut the setter issetBackwardChunks. This inconsistency could cause confusion during maintenance.- const [reverseChunks, setBackwardChunks] = useState<Map<string, Chunk>>( + const [reverseChunks, setReverseChunks] = useState<Map<string, Chunk>>(Then update all usages of
setBackwardChunkstosetReverseChunks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/react-common/src/useInfiniteQuerySubscription.ts` around lines 62 - 64, The state setter name is inconsistent: the state variable is reverseChunks but the setter is setBackwardChunks; rename the setter to setReverseChunks and update every usage accordingly (replace setBackwardChunks -> setReverseChunks) so the pair is reverseChunks / setReverseChunks (check the useState call that creates reverseChunks and all places where setBackwardChunks is referenced, e.g., any callbacks or effects) to maintain consistent naming.
11-11: Remove.tsextension from import path.Including the
.tsextension in the import may cause issues with certain bundlers and module resolution strategies. TypeScript's module resolution typically expects paths without extensions.-import { decrementCursor, incrementCursor } from './uuidMath.ts'; +import { decrementCursor, incrementCursor } from './uuidMath';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/react-common/src/useInfiniteQuerySubscription.ts` at line 11, Update the import in useInfiniteQuerySubscription.ts to remove the .ts extension: change the import that currently references './uuidMath.ts' to './uuidMath' so decrementCursor and incrementCursor are imported without a file extension, ensuring compatibility with TypeScript/module bundlers and standard resolution rules.
90-103: Heavy use ofas anybypasses type safety.Multiple subscription calls cast the query object to
any. This hides potential type mismatches that could cause runtime errors. Consider creating properly typed query builder helpers or refining the generic constraints to avoid these casts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/react-common/src/useInfiniteQuerySubscription.ts` around lines 90 - 103, The subscription call in useInfiniteQuerySubscription uses an unsafe cast ("as any") when calling core.subscribeQuery with the constructed query (see newSub, core.subscribeQuery, entity, query); remove the cast and instead create a properly typed query shape or helper that enforces the expected GraphQL/query types (e.g., a buildPagedQuery(entity, query, { after, before, where, fields, order }) function) or tighten the generic constraints on useInfiniteQuerySubscription/core.subscribeQuery so the object matches the required type; update all subscription call sites to use that typed helper or adjusted generics rather than casting to any, ensuring type-safe properties like $.where, $.fields and $.order are preserved.
287-296: Unnecessaryasynckeyword.The function is declared
asyncbut doesn'tawaitanything. This is misleading and the returnedPromise<void>resolves immediately regardless of whether the subscription callbacks have completed.If the intent is to wait for the next page to load, consider returning a Promise that resolves when the subscription emits data. Otherwise, remove
async:- const loadMore = async () => { + const loadMore = () => { if (!canLoadMore) return; // ... };And update the type:
- loadMore: () => Promise<void>; + loadMore: () => void;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/react-common/src/useInfiniteQuerySubscription.ts` around lines 287 - 296, The loadMore function is declared async but doesn't await anything, which is misleading; either remove the async keyword and change its signature to return void (update any callers/types expecting Promise<void>), or if you intended to wait for the next subscription emission, implement and return a Promise that resolves when the subscription emits new data (e.g., listen for the subscription event after calling freezeForward and pushNewForward and resolve when the next page arrives). Locate loadMore in useInfiniteQuerySubscription.ts and adjust its signature and callers accordingly; keep references to forwardChunks, canLoadMore, freezeForward, and pushNewForward intact while performing the change.
316-330:reverseOrderonly handles single-key ordering.If
ordercontains multiple keys (compound ordering), only the first key is reversed and the rest are dropped. This could produce unexpected results.If compound ordering isn't supported, consider adding validation or documentation. Otherwise, reverse all keys:
const reverseOrder = (order?: Order<any, any>): Order<any, any> => { if (!order) { return { serverCreatedAt: 'asc', }; } - const key = Object.keys(order).at(0); - if (!key) - return { - serverCreatedAt: 'asc', - }; - return { - [key]: order[key] === 'asc' ? 'desc' : 'asc', - }; + const keys = Object.keys(order); + if (keys.length === 0) { + return { serverCreatedAt: 'asc' }; + } + return Object.fromEntries( + keys.map((key) => [key, order[key] === 'asc' ? 'desc' : 'asc']), + ); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/react-common/src/useInfiniteQuerySubscription.ts` around lines 316 - 330, The reverseOrder function currently only flips the first key of an Order<any, any> and drops any additional keys; update reverseOrder to handle compound ordering by iterating over all keys in the provided order and producing a new object where each key's direction is flipped ('asc' <-> 'desc'), while preserving key order; if order is undefined or empty, return the default { serverCreatedAt: 'asc' } as before. If compound ordering is intentionally unsupported, instead add explicit validation in reverseOrder that throws or logs a clear error when Object.keys(order).length > 1 and document this behavior.
89-89: Address TODO comment for unsubscription.There's a TODO comment indicating the unsubscription logic may need attention. Please clarify if additional cleanup is needed here or remove the comment if the current implementation is sufficient.
Would you like me to help analyze the subscription lifecycle to ensure proper cleanup?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/react-common/src/useInfiniteQuerySubscription.ts` at line 89, The TODO "unsub" indicates missing cleanup for the subscription created in useInfiniteQuerySubscription; implement proper unsubscription by returning a cleanup function from the effect that created the subscription (or calling subscription.unsubscribe()/observer.unsubscribe() or abortController.abort() for the specific subscription variable used in useInfiniteQuerySubscription) to stop updates on unmount or when dependencies change, or remove the TODO if no cleanup is required because the subscription is already handled elsewhere; reference the subscription variable and the useEffect inside useInfiniteQuerySubscription to locate where to add the return () => { /* unsubscribe */ } cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/packages/react-common/src/useInfiniteQuerySubscription.ts`:
- Around line 242-244: The empty catch around calling decrementCursor hides
errors; replace it with explicit error handling—wrap the
pushNewForward(decrementCursor(pageInfo.startCursor) as any) call in try/catch
and in the catch log the error and context (e.g., console.error or the module's
logger) including pageInfo.startCursor and a clear message, or call an existing
error handler/prop if available, rather than swallowing the exception silently;
keep pushNewForward and decrementCursor usage unchanged but ensure failures are
surfaced for debugging.
- Around line 277-285: The effect uses reverseChunks, freezeBackward, and
pushNewReverse but only lists canLoadPrevious in its dependency array, risking
stale closures; also the non-null assertion on chunksInOrder.at(-1)! can throw
if the map becomes empty. Fix by adding reverseChunks, freezeBackward, and
pushNewReverse to the effect dependencies (and memoize freezeBackward and
pushNewReverse with useCallback where they are defined to prevent render loops),
and replace the unconditional destructuring of .at(-1)! with a safe guard that
checks chunksInOrder.length > 0 before accessing the last entry and ensuring
lastChunk.endCursor exists before calling freezeBackward and pushNewReverse.
---
Duplicate comments:
In `@client/packages/react-common/src/useInfiniteQuerySubscription.ts`:
- Around line 224-251: The effect that calls core.subscribeQuery (inside the
useEffect block) captures changing values (core, entity, query, pushNewForward,
pushNewReverse, hasKickstarted) but uses an empty dependency array, causing
stale closures; update the dependency array to include core, entity, query (or
the specific query.$ props used), and the handler functions, and either memoize
pushNewForward and pushNewReverse with useCallback or move their logic inside
the effect so closures stay fresh; also ensure you properly reset hasKickstarted
and any related state when those dependencies change and return a cleanup that
unsubscribes the previous subscription from core.subscribeQuery to avoid leaks.
- Around line 310-311: The returned flags isLoading and isLoadingMore are
hardcoded to false in useInfiniteQuerySubscription; change them to reflect the
real hook state by computing isLoading from the query status/isFetching (e.g.,
status === 'loading' || isFetching) and isLoadingMore from the fetch-next-page
state (e.g., isFetchingNextPage || fetchStatus === 'fetchMore' or similar
variables used in this hook). Update the return object in
useInfiniteQuerySubscription so consumers receive dynamic booleans derived from
the existing state variables (status, isFetching,
isFetchingNextPage/fetchStatus) instead of constant false.
---
Nitpick comments:
In `@client/packages/react-common/src/useInfiniteQuerySubscription.ts`:
- Around line 69-79: Change the parameter name from PascalCase to camelCase in
both setter functions: rename the parameter Chunk to chunk in setChunk and
setReverseChunk and update their bodies to use the new variable (e.g.,
set(JSON.stringify(startCursor), chunk)). This keeps naming consistent with
conventions and avoids confusion with type/class names; update any local
references inside those functions accordingly.
- Around line 62-64: The state setter name is inconsistent: the state variable
is reverseChunks but the setter is setBackwardChunks; rename the setter to
setReverseChunks and update every usage accordingly (replace setBackwardChunks
-> setReverseChunks) so the pair is reverseChunks / setReverseChunks (check the
useState call that creates reverseChunks and all places where setBackwardChunks
is referenced, e.g., any callbacks or effects) to maintain consistent naming.
- Line 11: Update the import in useInfiniteQuerySubscription.ts to remove the
.ts extension: change the import that currently references './uuidMath.ts' to
'./uuidMath' so decrementCursor and incrementCursor are imported without a file
extension, ensuring compatibility with TypeScript/module bundlers and standard
resolution rules.
- Around line 90-103: The subscription call in useInfiniteQuerySubscription uses
an unsafe cast ("as any") when calling core.subscribeQuery with the constructed
query (see newSub, core.subscribeQuery, entity, query); remove the cast and
instead create a properly typed query shape or helper that enforces the expected
GraphQL/query types (e.g., a buildPagedQuery(entity, query, { after, before,
where, fields, order }) function) or tighten the generic constraints on
useInfiniteQuerySubscription/core.subscribeQuery so the object matches the
required type; update all subscription call sites to use that typed helper or
adjusted generics rather than casting to any, ensuring type-safe properties like
$.where, $.fields and $.order are preserved.
- Around line 287-296: The loadMore function is declared async but doesn't await
anything, which is misleading; either remove the async keyword and change its
signature to return void (update any callers/types expecting Promise<void>), or
if you intended to wait for the next subscription emission, implement and return
a Promise that resolves when the subscription emits new data (e.g., listen for
the subscription event after calling freezeForward and pushNewForward and
resolve when the next page arrives). Locate loadMore in
useInfiniteQuerySubscription.ts and adjust its signature and callers
accordingly; keep references to forwardChunks, canLoadMore, freezeForward, and
pushNewForward intact while performing the change.
- Around line 316-330: The reverseOrder function currently only flips the first
key of an Order<any, any> and drops any additional keys; update reverseOrder to
handle compound ordering by iterating over all keys in the provided order and
producing a new object where each key's direction is flipped ('asc' <-> 'desc'),
while preserving key order; if order is undefined or empty, return the default {
serverCreatedAt: 'asc' } as before. If compound ordering is intentionally
unsupported, instead add explicit validation in reverseOrder that throws or logs
a clear error when Object.keys(order).length > 1 and document this behavior.
- Line 89: The TODO "unsub" indicates missing cleanup for the subscription
created in useInfiniteQuerySubscription; implement proper unsubscription by
returning a cleanup function from the effect that created the subscription (or
calling subscription.unsubscribe()/observer.unsubscribe() or
abortController.abort() for the specific subscription variable used in
useInfiniteQuerySubscription) to stop updates on unmount or when dependencies
change, or remove the TODO if no cleanup is required because the subscription is
already handled elsewhere; reference the subscription variable and the useEffect
inside useInfiniteQuerySubscription to locate where to add the return () => { /*
unsubscribe */ } cleanup.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
client/packages/core/src/instaql.tsclient/packages/react-common/src/useInfiniteQuerySubscription.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- client/packages/core/src/instaql.ts
dwwoelfel
reviewed
Mar 3, 2026
dwwoelfel
reviewed
Mar 3, 2026
dwwoelfel
reviewed
Mar 3, 2026
dwwoelfel
reviewed
Mar 3, 2026
dwwoelfel
reviewed
Mar 3, 2026
stopachka
reviewed
Mar 18, 2026
stopachka
reviewed
Mar 18, 2026
stopachka
reviewed
Mar 18, 2026
stopachka
reviewed
Mar 18, 2026
stopachka
reviewed
Mar 18, 2026
stopachka
reviewed
Mar 18, 2026
stopachka
reviewed
Mar 18, 2026
stopachka
reviewed
Mar 18, 2026
stopachka
reviewed
Mar 18, 2026
stopachka
reviewed
Mar 18, 2026
stopachka
reviewed
Mar 18, 2026
stopachka
reviewed
Mar 18, 2026
stopachka
reviewed
Mar 18, 2026
Merged
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
3 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.
Adds a subscribeInfiniteQuery function to core and a useInfiniteQuery hook to react package.
Sandbox available at http://localhost:4000/play/infinite-scroll
Example Usage:
Limit represents how many items to load at once.
New type exports from Core:
Notes: core/weakHash was changed to ignore fields with the value undefined as part of the hash.
Note: Deferred in core migrated to typescript file, + optional generic for what the promise resolves to (defaults to
any). This isn't linked to anything in current infinite query implementation.Note: Added a new option to the nextjs sandbox ephemeral page component to wait for the db to initialize before rendering the page. This simulates what it would be like to do a client side navigation to this page.