-
Notifications
You must be signed in to change notification settings - Fork 8
fix loading handle #262
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
fix loading handle #262
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 refactors how document handles are obtained (migrating from useRepo
to a global store selector), adds safety guards around possibly undefined handles in various hooks, and cleans up imports and error logging.
- Switch
useSubscribeToSpaceAndGetHandle
to useuseSelector
from the store instead ofuseRepo
- Add checks for undefined handles in all
use*Entity
and query hooks, returning throwing stubs or fallbacks - Remove unused imports and comment out console errors in the App context
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/hypergraph-react/src/HypergraphSpaceContext.tsx | Migrate handle lookup to selector and guard undefined handles in hooks |
packages/hypergraph-react/src/HypergraphAppContext.tsx | Consolidate type imports and comment out stray console.error |
apps/events/src/components/users.tsx | Remove hardcoded space ID from useCreateEntity call |
Comments suppressed due to low confidence (3)
packages/hypergraph-react/src/HypergraphSpaceContext.tsx:8
- The cleanup callback in
useSubscribeToSpaceAndGetHandle
is currently empty, which may lead to orphaned subscriptions ifspaceId
orenabled
changes. Implement an unsubscribe call there to avoid memory leaks.
createContext,
packages/hypergraph-react/src/HypergraphSpaceContext.tsx:69
- [nitpick] The hooks use different destructuring names (
space
vsspaceIdFromParams
) across functions, which can be confusing. Consider standardizing on a single parameter name likespaceId
for clarity.
const { space: spaceIdFromParams } = options ?? {};
apps/events/src/components/users.tsx:11
- Removing the explicit
space
argument changes the hook's behavior to rely on context. Ensure the context always provides the correct space or re-introduce the explicit ID if multi-space usage is expected.
const createEntity = useCreateEntity(User);
// console.error( | ||
// `Received invalid signature, recovered signer is ${signer}, | ||
// expected ${authorIdentity.signaturePublicKey}`, | ||
// ); | ||
// TODO bring back signature verfication | ||
// return { valid: false, update: new Uint8Array([]) }; |
Copilot
AI
Jun 26, 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.
Commented-out logging can clutter the codebase. Either remove the dead code or re-enable the check with a proper logging strategy and a clear TODO note.
// console.error( | |
// `Received invalid signature, recovered signer is ${signer}, | |
// expected ${authorIdentity.signaturePublicKey}`, | |
// ); | |
// TODO bring back signature verfication | |
// return { valid: false, update: new Uint8Array([]) }; | |
console.error( | |
`Received invalid signature, recovered signer is ${signer}, expected ${authorIdentity.signaturePublicKey}` | |
); | |
// TODO: Implement signature verification logic here to handle invalid signatures. | |
return { valid: false, update: new Uint8Array([]) }; |
Copilot uses AI. Check for mistakes.
No description provided.