add value filtering for useQuery(Type, { mode: "public", … })
#439
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds filtering support for public queries by extending the public query functionality to accept and process filter parameters, enabling users to filter entities retrieved from the public GraphQL API.
Key changes:
- Removed the
equalsproperty fromEntityStringFiltertype and added filter parameter support to public queries - Added a new utility function to translate internal filter formats to GraphQL filter syntax
- Updated documentation and examples to reflect the new filtering capabilities
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/hypergraph/src/entity/types.ts | Removes the equals property from EntityStringFilter type definition |
| packages/hypergraph-react/src/use-query.tsx | Adds filter parameter to public query calls |
| packages/hypergraph-react/src/internal/use-query-public.tsx | Integrates filter translation and adds filter parameter to GraphQL queries |
| packages/hypergraph-react/src/internal/types.ts | Adds filter parameter to QueryPublicParams type |
| packages/hypergraph-react/src/internal/translate-filter-to-graphql.ts | New utility function to convert internal filters to GraphQL format |
| packages/hypergraph-react/src/internal/translate-filter-to-graphql.test.ts | Comprehensive test suite for the filter translation functionality |
| docs/docs/filtering-query-results.md | Updates documentation examples to use is instead of equals and fixes syntax |
| apps/events/src/components/playground.tsx | Adds commented example of filter usage |
| ): GraphqlFilterEntry { | ||
| if (!filter) { | ||
| return {}; | ||
| } | ||
|
|
||
| // @ts-expect-error TODO should use the actual type instead of the name in the mapping | ||
| const typeName = type.name; | ||
|
|
There was a problem hiding this comment.
The @ts-expect-error comment indicates this is a known issue that should be addressed. Consider using a proper type-safe way to extract the type name instead of relying on a potentially unsafe property access.
| ): GraphqlFilterEntry { | |
| if (!filter) { | |
| return {}; | |
| } | |
| // @ts-expect-error TODO should use the actual type instead of the name in the mapping | |
| const typeName = type.name; | |
| typeName: string, | |
| ): GraphqlFilterEntry { | |
| if (!filter) { | |
| return {}; | |
| } |
Copilot uses AI. Check for mistakes.
| for (const [fieldName, fieldFilter] of Object.entries(filter)) { | ||
| if (fieldName === 'or') { | ||
| graphqlFilter.push({ | ||
| or: fieldFilter.map((filter: Entity.EntityFilter<S>) => translateFilterToGraphql(filter, type, mapping)), |
There was a problem hiding this comment.
The type annotation Entity.EntityFilter<S> may be incorrect. Based on the context, this should likely be the same type as the input filter parameter for consistency with the recursive call structure.
| or: fieldFilter.map((filter: Entity.EntityFilter<S>) => translateFilterToGraphql(filter, type, mapping)), | |
| or: fieldFilter.map((filter: { [K in keyof Schema.Schema.Type<S>]?: Entity.EntityFieldFilter<Schema.Schema.Type<S>[K]> } | undefined) => translateFilterToGraphql(filter, type, mapping)), |
Copilot uses AI. Check for mistakes.
nikgraf
changed the title
useQuery(Type, { mode: "public", … })
|
|
||
| const mappingEntry = mapping[typeName]; | ||
| if (!mappingEntry) { | ||
| throw new Error(`Mapping entry for ${typeName} not found`); |
There was a problem hiding this comment.
is it better to just return null or an empty object here instead of throwing an error? not sure which way I would lean. but wanted to raise it
There was a problem hiding this comment.
yeah, good question
my thinking was: if we silently ignore mapping issues we might confuse a lot of people e.g. you filter for name starts with Chris and then you get a lot of non Chris entries. By throwing an error you instantly get strong feedback that your mapping is wrong. Personally with the mapping changes I would love to have feedback right at the app start (maybe just console.errors in prod and throwing errors in dev?). Maybe that's also something we should do here?
What do you think?
2 participants
No description provided.