Skip to content

fix!: identity hash is Hasher<"identity", 0> #314

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

SgtPooki
Copy link

@SgtPooki SgtPooki commented Dec 2, 2024

Breaking change because return type no longer supports sync (aligns with other hashers)

Fixes #313

@SgtPooki SgtPooki linked an issue Dec 2, 2024 that may be closed by this pull request
@SgtPooki SgtPooki self-assigned this Dec 2, 2024
@SgtPooki
Copy link
Author

SgtPooki commented Feb 4, 2025

@achingbrain any qualms against this one?

@SgtPooki SgtPooki requested a review from achingbrain February 5, 2025 13:03
@achingbrain
Copy link
Member

achingbrain commented Jul 29, 2025

With hindsight identity should be a SyncMultihashHasher.

E.g.

// identity.ts
import type { SyncMultihashHasher } from './interface.js'

// ... other code

export const identity: SyncMultihashHasher<0x00> = { code, name, encode, digest }

One problem is that identity then does not have type overlap with other Hasher instances as MultihashHasher and SyncMultihashHasher don't expose an encode property so the above will fail to compile without removing the encode prop from the export.

One (breaking) solution would be remove the encode prop above and also change the return type of from to use the MultihashHasher interface instead of the Hasher concrete implementation.

Alternatively (non-breaking) we can add the encode sig to MultihashHasher and SyncMultihashHasher though it doesn't make much sense for it to be there.

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.

bug: identity hash doesn't export proper type of Hasher<Name, Code>
3 participants