Using constants for multicodec

[vmx]

I propose using constants for multicodec related things.

Currently we use strings. E.g. if you create a new CID you do:

new CID(1, 'dag-pb', multihash)

Instead I'd like to change it to

new CID(1, multicodec.DAG_PB, multihash)

Reasons:

• js-multicodec really becomes the source of truth

• custom codecs are easier. As we always operate on numbers, someone could e.g. do for prototyping purpose:

    new CID(1, 0xffaa, multihash)
  

  You wouldn't need to register a string somewhere.

• Current js-multicodec is based around Node Buffers. With using constants (with numbers as values) it becomes more browser friendly.

I've opened a PR at multiformats/js-multicodec#35 implementing it. Please note that it even isn't a breaking change, as it just adds additional things. I'd then like to deprecate the old stuff in the future, once we update the whole code base. Though there surely is no rush in doing that.

I'd like to get feedback from the whole @ipfs/javascript-team.

[daviddias]

Sounds great to me!

[hacdias]

@vmx agreed!

[achingbrain]

I'm all for exporting constants to reduce programmer error. I'm less for what might appear to users as change for the sake of change from an API point of view, but as long as it's backwards compatible with using strings then all good.

You wouldn't need to register a string somewhere.

I might be missing something, but ipld uses the codec to look up the resolver, right? So if you are messing around with a custom codec, event with the string to number change you'd still need to register a number somewhere instead?

js-multicodec is based around Node Buffers

+1 to removing Buffers wherever possible. Ideally we'd just operate on array-like objects instead so we can use Buffers, Arrays, Uint8Array, bl, whatever.

[alanshaw]

js-multicodec really becomes the source of truth

Could you explain why it isn't already?

You wouldn't need to register a string somewhere.

As @achingbrain said, I think you'd still need to register it to be able to use it - there's separate discussions to be had around supporting missing codecs in our domain objects e.g. multiformats/multiaddr#70.

Current js-multicodec is based around Node Buffers. With using constants (with numbers as values) it becomes more browser friendly.

Could you explain a bit more I'm not sure I understand why it's more browser friendly?

---

IMHO it's better DX to use a string. It's easier to remember than the number, and if you use the number directly (and people will) anyone looking at your code will have to look up what that number refers to i.e. harder to maintain, understand and less readable. If you use constants like multicodec.DAG_PB you'll need to require the multicodec dependency, making your bundle bigger which is undesireable if you never use anything other than one constant.

I'm maginally pro being flexible and allowing use of either the code or the name, but I just know that people will use the number directly and not the exported constant and so I'm less happy about giving developers the rope to make their code less maintainable.

I am -1 on deprecating and removing the use of the string.

[vmx]

I might be missing something, but ipld uses the codec to look up the resolver, right? So if you are messing around with a custom codec, event with the string to number change you'd still need to register a number somewhere instead?

Yes. Sorry I was talking on the js-ipld level, not the js-ipfs one.

[vmx]

js-multicodec really becomes the source of truth

Could you explain why it isn't already?

It might be the source of truth, but it's not used in all libraries that should use it. For example: https://github.com/multiformats/js-multihash/blob/443a8ebdd145582f2cac6e708ffe890401d5de08/src/constants.js

Current js-multicodec is based around Node Buffers. With using constants (with numbers as values) it becomes more browser friendly.

Could you explain a bit more I'm not sure I understand why it's more browser friendly?

You wouldn't need to use the Buffer polyfill, but could work with JavaScript native data types like TypedArrays.

If you use constants like multicodec.DAG_PB you'll need to require the multicodec dependency, making your bundle bigger which is undesireable if you never use anything other than one constant.

If it becomes the source of truth, it's very likely that it is bundled already anyway. Also js-cid has a dependency on it, which is likely to be included anyway.

I'm maginally pro being flexible and allowing use of either the code or the name, but I just know that people will use the number directly and not the exported constant and so I'm less happy about giving developers the rope to make their code less maintainable.

Or they are happy to have a constant as their IDE can autocomplete it.

I am -1 on deprecating and removing the use of the string.

I'd remove the strings to make the bundle size smaller. This way we'd also get rid of the dependency on Buffer in the Browser (though, you can make the same argument as I did above that Buffer is very likely to be bundled anyway).

[alanshaw]

You wouldn't need to use the Buffer polyfill, but could work with JavaScript native data types like TypedArrays.

Understood, but I don't think using numbers expressly enables that change. Or am I missing something?

If it becomes the source of truth, it's very likely that it is bundled already anyway. Also js-cid has a dependency on it, which is likely to be included anyway.

Yes fair enough.

Or they are happy to have a constant as their IDE can autocomplete it.

Yes of course, this is the big win, and outweighs the chance of people using new CID(1, 7, mh) which is why I'm "maginally pro" it!

I'd remove the strings to make the bundle size smaller

Yeah, maybe, I think after gzip the difference would probably be negligable.

This way we'd also get rid of the dependency on Buffer in the Browser (though, you can make the same argument as I did above that Buffer is very likely to be bundled anyway).

In this case I'm less concerned about bundle size and more about DX - Buffer is not in the browser so the developer somehow has to get hold of it or IPFS has to export it (which it does for that reason, and I'd like IPFS to not have to do that). It's also confusing that IPFS doesn't just support browser buffers, there's not real reason why it shouldn't be flexible.

[vmx]

You wouldn't need to use the Buffer polyfill, but could work with JavaScript native data types like TypedArrays.

Understood, but I don't think using numbers expressly enables that change. Or am I missing something?

Not at the moment. js-cid would also need to support it, resp. get rid of Buffers. I'd like that, but I'm not sure if that will really happen.

So it's more a "if we don't get rid of Buffers as multicodec values, we might not be able to gt rid of Buffers at the core of things". Of course you don't have to use constants for this, you could just define the current strings to use numbers instead of Buffers. Though this would be a hard to upgrade breaking change.