feat: simplify block to only be data and cid #31
Conversation
|
ref ipfs/js-ipfs#787 |
|
@dignifiedquire do I need to say it again? no yarn.lock :( |
| * @param {any} other | ||
| * @returns {bool} | ||
| */ | ||
| static isBlock (other) { |
There was a problem hiding this comment.
Still making my mind over this, but currently, I strongly feel that public module methods shouldn't be encapsulated in Class objects and made public with static.
I've seen a lot of cases of people starting to use static and then forgetting what it really means and start mutating the state.
Plus, it is way more understandable from a Node.js/CJS user if the "module.exports" is the only place that tells me what gets exported.
There was a problem hiding this comment.
Declaring Block.isBlock = ... doesn't seem to be any better to me in terms of understanding.
There was a problem hiding this comment.
also we started using this a while ago in modules like cid
There was a problem hiding this comment.
There was a problem hiding this comment.
one bonus is that jsdoc picks up the right docs for this if we write it as static
src/index.js
Outdated
| class Block { | ||
| constructor (data, cid) { | ||
| if (!data || !Buffer.isBuffer(data)) { | ||
| throw new Error('data must be a buffer') |
There was a problem hiding this comment.
first argument must be a Buffer
src/index.js
Outdated
| throw new Error('Block must be constructed with data') | ||
| } | ||
| if (!cid || !CID.isCID(cid)) { | ||
| throw new Error('cid must be a CID') |
There was a problem hiding this comment.
second argument must be a CID
|
applied all cr except the static thing |
Ref ipfs/js-ipfs-repo#123