-
Notifications
You must be signed in to change notification settings - Fork 8
switch to uuid #219
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
switch to uuid #219
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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
Switches identifier generation and handling from Base58-encoded values to standard UUID v4 strings across the codebase.
generateId
now returns a v4 UUID instead of a Base58-encoded string.idToAutomergeId
andautomergeIdToId
useuuid
parse/stringify with Base58Check.- Tests updated to use and validate UUIDs in place of Base58 IDs.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
packages/hypergraph/src/utils/generateId.ts | Changed generateId to return uuidv4() , removed Base58 logic. |
packages/hypergraph/src/utils/automergeId.ts | Updated conversions to use uuid.parse /stringify with Base58Check. |
packages/hypergraph/test/utils/generateId.test.ts | Adjusted test to validate UUID format and updated test title. |
packages/hypergraph/test/messages/signed-update-message.test.ts | Swapped spaceId hex literal to a UUID string. |
packages/hypergraph/test/entity/findMany.test.ts | Swapped spaceId Base58 literal to a UUID string. |
packages/hypergraph/test/entity/entity.test.ts | Swapped spaceId Base58 literal to a UUID string. |
packages/hypergraph-react/test/HypergraphSpaceContext.test.tsx | Swapped spaceId Base58 literal to a UUID string. |
Comments suppressed due to low confidence (2)
packages/hypergraph/src/utils/generateId.ts:5
- The docstring still describes removing dashes and Base58 encoding, which the implementation no longer does. Please update or remove these lines to accurately reflect the current behavior.
* Remove the dashes to make it a 32bit value.
packages/hypergraph/src/utils/automergeId.ts:7
- The updated
idToAutomergeId
andautomergeIdToId
functions aren’t directly covered by unit tests. Consider adding tests to verify Base58Check encoding/decoding and UUID round-tripping.
export function idToAutomergeId(uuid: string, _versionByte = 0x00) {
import { generateId } from '../../src/utils/generateId.js'; | ||
|
||
it('should generate a base58 encoded uuid of 22 char length', () => { | ||
it('should generate a a valid uuid', () => { |
Copilot
AI
Jun 16, 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.
There's a typo with duplicated 'a' in the test title; consider changing it to it('should generate a valid uuid', ...)
for clarity.
it('should generate a a valid uuid', () => { | |
it('should generate a valid uuid', () => { |
Copilot uses AI. Check for mistakes.
3b849bf
to
d48d42d
Compare
No description provided.