-
Notifications
You must be signed in to change notification settings - Fork 11
fix: replace node buffers with uint8arrays #69
Conversation
This module now accepts Uint8Arrays as well as node Buffers and returns Uint8Arrays. Internally it converts non-Buffers into Buffers because the ethereum libs require that. BREAKING CHANGES: - `util.serialize` returns a `Uint8Array` - `util.cid` returns `CID`s with a breaking API change - see multiformats/js-cid#117 for changes
Codecov Report
@@ Coverage Diff @@
## master #69 +/- ##
==========================================
- Coverage 99.37% 99.24% -0.14%
==========================================
Files 20 20
Lines 801 792 -9
==========================================
- Hits 796 786 -10
- Misses 5 6 +1
Continue to review full report at Codecov.
|
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.
@achingbrain I guess we still need to use Buffer
due to the underlying Ethereum library?
Nevermind, you write about that in the commit message. |
* @returns {Object} | ||
*/ | ||
deserialize, | ||
deserialize: (serialized) => { |
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.
Is this change because upstream only supports Buffers and they also support arrays of Buffers?
Probably needs a const { Buffer } = require('buffer')
too, right?
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.
Good spot, I've PR'd it in: #70
This module now accepts Uint8Arrays as well as node Buffers and
returns Uint8Arrays. Internally it converts non-Buffers into Buffers
because the ethereum libs require that.
BREAKING CHANGES:
util.cid
returnsCID
s with a breaking API change - see fix: replace node buffers with uint8arrays multiformats/js-cid#117 for changes