fixes to support hypergraph in nextjs (fix imports, use automerge slim) #180
Conversation
nikgraf
force-pushed
the
ng/nextjs-support
branch
3 times, most recently
from
f130705 to
d26d034
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR updates various import statements and dependency versions to support the hypergraph functionality in Next.js, while switching to the automerge slim build and manually loading the WASM bundle. Key changes include:
- Fixing import paths for Messages, Identity, and related modules across inbox and event files.
- Upgrading "@automerge/automerge-repo" (and its react hooks) from beta 2 to beta 5.
- Adding Next.js specific configuration files and usage instructions in the next-example app.
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/hypergraph/src/inboxes/*.ts | Updated import paths to use local module references. |
| packages/hypergraph/package.json | Upgraded automerge-repo dependency version to beta 5. |
| packages/hypergraph-react/src/HypergraphSpaceContext.tsx | Updated repo lookup to use findWithProgress for better progress reporting. |
| packages/hypergraph-react/src/HypergraphAppContext.tsx | Switched to automerge slim; initialized WASM and fixed import paths. |
| apps/next-example/** | Added/updated config, tsconfig, and provider components for Next.js support. |
| README.md | Improved documentation on running the Next.js example app. |
Comments suppressed due to low confidence (2)
packages/hypergraph-react/src/HypergraphAppContext.tsx:1370
- [nitpick] Consider extracting the hard-coded maxAttempts constant into a configurable parameter or named constant to enhance the maintainability and flexibility of the retry logic.
const maxAttempts = 10;
packages/hypergraph-react/src/HypergraphAppContext.tsx:6
- Consider adding a reference to an external issue or a TODO comment with context regarding the missing type definitions in the Automerge package to improve maintainability.
// @ts-expect-error not properly typed and exported in the automerge package
9dfd86c to
af2a151
Compare
| import { Effect } from 'effect'; | ||
| import { bytesToHex, hexToBytes, stringToUint8Array } from '../utils/index.js'; | ||
| import { canonicalize } from '../utils/index.js'; | ||
| import type * as Inboxes from '../inboxes/types.js'; |
There was a problem hiding this comment.
nit: this could be './types.js'
| }, | ||
| setSpaceFromList: (context, event: { spaceId: string }) => { | ||
| if (!context.repo) { | ||
| return context; |
There was a problem hiding this comment.
This will silently fail to set the space... should we fail more loudly if this happens?
There was a problem hiding this comment.
yeah, I like it, but couldn't think of something useful. What do you have in mind?
(btw setSpaceFromList never runs afaik if repo is not set)
There was a problem hiding this comment.
since it's something that should never happen, maybe throwing an error would be best?
2 participants
The nextjs example could be improved. Added an issue here: #181