-
Notifications
You must be signed in to change notification settings - Fork 8
add first filter #314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add first filter #314
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a first
parameter to limit the number of results in public GraphQL queries.
- Adds
first
to theQueryParams
type and forwards it throughuseQuery
anduseQueryPublic
. - Updates GraphQL query documents to accept a
$first: Int
argument and applies it in the request. - Sets a default
first = 100
inuseQueryPublic
and demonstrates usage in the playground.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
packages/hypergraph-react/src/use-query.tsx | Added first to params destructuring and forwarding |
packages/hypergraph-react/src/internal/use-query-public.tsx | Extended queries with $first , default handling, and forwarding |
packages/hypergraph-react/src/internal/types.ts | Added optional first to QueryPublicParams |
apps/events/src/components/playground.tsx | Example usage of first parameter |
Comments suppressed due to low confidence (2)
packages/hypergraph-react/src/use-query.tsx:13
- New
first
behavior isn't covered by existing tests; consider adding unit or integration tests to verify pagination limits in both public and local queries.
first?: number;
packages/hypergraph-react/src/use-query.tsx:150
useQueryLocal
doesn't accept afirst
parameter, so local queries cannot be limited; either document this difference or addfirst
support to local queries for consistency.
const localResult = useQueryLocal(type, { enabled: mode === 'private', filter, include, space });
mappingEntry?.typeIds, | ||
relationTypeIdsLevel1, | ||
relationTypeIdsLevel2, | ||
// TODO should `first` be in here? |
Copilot
AI
Jul 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first
parameter should be included in the queryKey
array to ensure cache entries vary by first
value; add first
after relationTypeIdsLevel2
.
// TODO should `first` be in here? | |
first, |
Copilot uses AI. Check for mistakes.
queryDocument = entitiesQueryDocumentLevel2; | ||
} | ||
|
||
console.log('first', first); |
Copilot
AI
Jul 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this debug console.log
statement to avoid noisy logs in production.
console.log('first', first); |
Copilot uses AI. Check for mistakes.
No description provided.