rework space context #249
rework space context #249
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors how “space” context is provided and consumed across the codebase by introducing a streamlined HypergraphSpaceProvider and a new useSpace hook. It removes direct context coupling in entity hooks, centralizes subscription logic, and updates all routes and components to use the new API.
- Add a
spaceparameter to every query/entity hook and propagate it throughuseQueryLocalanduseCreateEntity-style hooks - Simplify
HypergraphSpaceContextto only hold a space ID and move handle subscription intouseSubscribeToSpaceAndGetHandle - Update all React routes and components to use
useSpaceinstead of manualsubscribeToSpacecalls and remove the olduseHypergraphSpace
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/hypergraph/src/entity/findMany.ts | Extracted FindManySubscription type and updated return type |
| packages/hypergraph-react/src/use-query.tsx | Added space to QueryParams and passed it into useQueryLocal |
| packages/hypergraph-react/src/internal/use-query-public.tsx | Switched to useHypergraphSpaceInternal for public queries |
| packages/hypergraph-react/src/index.ts | Removed old useHypergraphSpace export in favor of useSpace |
| packages/hypergraph-react/src/HypergraphSpaceContext.tsx | Overhauled provider and hook implementations to use new model |
| apps/events/src/routes/space/$spaceId/* | Updated all page components to wrap in HypergraphSpaceProvider and drop manual subscriptions |
| apps/events/src/components/* | Updated components to use useSpace and pass explicit space |
Comments suppressed due to low confidence (1)
packages/hypergraph-react/src/use-query.tsx:12
- You've added an optional
spaceparameter here, but there are no unit tests verifying that overriding the context default works correctly. Consider adding tests foruseQuery,useQueryLocal, anduseCreateEntitywhenspaceis passed explicitly.
space?: string;
| <div className="flex flex-col gap-4 max-w-(--breakpoint-sm) mx-auto py-8"> | ||
| <HypergraphSpaceProvider space={spaceId}> | ||
| <SpaceChat spaceId={spaceId} /> | ||
| </HypergraphSpaceProvider> | ||
| <SpaceChat spaceId={spaceId} /> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
This page no longer wraps its children in HypergraphSpaceProvider, so calling useSpace inside SpaceChat will throw. Please wrap the returned JSX in <HypergraphSpaceProvider space={spaceId}>.
Copilot uses AI. Check for mistakes.
| export function useUpdateEntity<const S extends Entity.AnyNoContext>(type: S) { | ||
| const hypergraph = useHypergraphSpaceInternal(); | ||
| return Entity.update(hypergraph.handle, type); | ||
| export function useCreateEntity<const S extends Entity.AnyNoContext>(type: S, options?: { space?: string }) { |
There was a problem hiding this comment.
[nitpick] useCreateEntity, useUpdateEntity, useDeleteEntity, etc. all share nearly identical handle-lookup logic. Consider extracting the shared subscription retrieval into a helper to reduce duplication and improve maintainability.
Copilot uses AI. Check for mistakes.
| }, [spaceId, repo]); | ||
|
|
||
| const { subscribeToSpace, isConnecting } = useHypergraphApp(); | ||
| useEffect(() => { |
There was a problem hiding this comment.
The effect that calls subscribeToSpace does not store or call a cleanup/unsubscribe function. This could lead to repeated subscriptions if spaceId changes. Capture and call the unsubscribe function (if available) in the cleanup callback.
Copilot uses AI. Check for mistakes.
1 participant
No description provided.