feat: define sync multihash hasher interface #160
Conversation
| @@ -28,13 +28,21 @@ | |||
| "skipLibCheck": true, | |||
| "stripInternal": true, | |||
| "resolveJsonModule": true, | |||
| "noEmit": true | |||
| "noEmit": true, | |||
| "paths": { | |||
There was a problem hiding this comment.
😭 I am having a really bad day with TS today 😖 I thought #157 took care of this, but as it turns out node_modules/multiformats takes precedence without this
| code: 0x0, | ||
| encode: (input) => coerce(input) | ||
| }) | ||
| /** @type {import('./interface').SyncMultihashHasher<typeof code>} */ |
There was a problem hiding this comment.
How much does this matter here? Is it just for internal consistency within this file? Is it showing up in the exported types?
There was a problem hiding this comment.
I should have added a code comment, this is mostly to ensure that module will end up compatible with desired interface as there is no way to say module @implements {SyncMultihashHasher<typeof code>}
There was a problem hiding this comment.
👍
My only hesitance is that we could give a similar treatment to sha2.js; although not sha2-browser.js. By passing the hashers in sha2.js through from() we get an async digest() function involved even though the crypto calls are sync. While I'd be fine exporting those hashers as the original MultihashHasher type, it'd be nice if the user could feature-detect the output of digest to see if it's a Promise or not and act accordingly. When you're in the browser you'll have to await but when on Node.js you shouldn't need to, which would be really nice.
## [9.6.0](v9.5.9...v9.6.0) (2022-01-19) ### Features * add sync multihash hasher ([#160](#160)) ([c3a650c](c3a650c))
|
🎉 This PR is included in version 9.6.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Fix API unintentional API change introduced by #160 which removed `encode` method.
Fix API unintentional API change introduced by #160 which removed `encode` method.
This implements proposal #159