Skip to content

fix!: did bytes schema#378

Closed
alanshaw wants to merge 3 commits intomainfrom
fix/did-bytes-schema
Closed

fix!: did bytes schema#378
alanshaw wants to merge 3 commits intomainfrom
fix/did-bytes-schema

Conversation

@alanshaw
Copy link
Member

@alanshaw alanshaw commented May 12, 2025

This PR fixes the didBytes schema so that it returns the type it is designed to read - Uint8Array. Concretely this changes the return type from read to be a PrincipalView, which is a subclass of Uint8Array.

The issue is that while this schema (in its current form) can read and validate data encoded as a Uint8Array, you cannot create an invocation/delegation with a value of a Uint8Array - it expects a string DID:

Type 'Uint8Array<ArrayBufferLike>' is not assignable to type '`did:${string}:${string}` & `did:${string}` & Phantom<{ protocol: "did:"; }>'.

If you pass a string, it will be encoded as a string, which is not the value that can be read by the same schema.

In real terms, I'm trying to create a location claim with a byte encoded space DID but I cannot 😢

In hindsight, this schema type should perhaps be called principal. I'd be open to changing this PR to add that and deprecate didBytes instead if folks feel strongly.

@alanshaw alanshaw requested a review from a team May 12, 2025 14:01
@alanshaw alanshaw changed the title fix: did bytes schema fix!: did bytes schema May 12, 2025
Copy link
Member

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm and I prefer the rename to Principal if we have bandwidth to do it.

alanshaw added a commit that referenced this pull request May 13, 2025
This PR adds `Schema.principal()` as a replacement to
`Schema.didBytes()`.

The `Schema.didBytes()` schema has issues as described here
#378. This change allows those
issues to be fixed without a breaking change. This PR also deprecates
`Schema.didBytes()` via jsdoc comments.

closes #378
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants