Proposal: add some method e.g. .toLink(): CID on CID object

[Gozala]

Rational

Often times when working with partial DAGs we find ourselves with mix of materialized nodes for local blocks and links external blocks. When working in such a domain I often find myself wishing:

1. I could pass either materialized node or a CID into functions without having to turn node into CID (often times that also requires checking if thing at hand is CID or a node).
2. In functions that care about links I wish I could allow passing anything that can be linked via CID.

Both sides of the call would be happier if you could pass something "linkable" instead, that way callers could pass arbitrary nodes (as long as they are linkable) and callee could just use a link without having to inspect arguments.

Proposal

1. I would like to propose introducing a new interface e.g. Linkable:

interface Linkable {
  toLink(): CID
}

2. Make CID implementation of proposed Linkable interface.

That way arbitrary objects could implement Linkable interface which would allow them to be used everywhere Linkable is accepted. Functions that typically want CIDs could also start accepting more general Linkables and there for allow passing arbitrary object that could be linked.

Alternative

For what it's worth we already have something along these lines in form of asCID property. However it is a private property so TS complains about it. We could make it non private and achieve same goal by implementing asCID property on desired nodes.

[rvagg]

Yeah, ok, sounds reasonable I think. So you'd presumably also have an implementation on a ByteView-ish thing that knows its codec & hasher and can produce a result to toLink()? So does that mean we need this to be async (or MaybePromise?) because it may end up hitting an async hash function?

[Gozala]

So you'd presumably also have an implementation on a ByteView-ish thing that knows its codec & hasher and can produce a result to toLink()?

Usually these objects already have a CID (e.g. because they were loaded from block store or a car file), so often they can just return CID.

So does that mean we need this to be async (or MaybePromise?) because it may end up hitting an async hash function?

There are might be other thins in the future which need to compute CID and consequently would not be able to return CID sync. However I don't think that should be a concern for this proposal, for such use cases one may define some other interface e.g.

interface AsyncLinkable {
  toLink(): Promise<CID>
}

And then in the functions that want to support this define input param as (input:Linkable|AsyncLinkable) => ... .

That said, I personally don't like taking things that could potentially run async computations. I much rather prefer to be more clear on who's responsibility it is to run such a computation as in it's either callers responsibility to precompute those things or it is callees (in which case later should probably in more control of what kind of CIDs are created hashing alg etc...).

[rvagg]

By making it sync, you're narrowing down the places where this can be used and potentially putting a lot of complexity onto the user. We already have a strong theme of async wherever hashing is involved in js-multiformats. But in general hashing is a super massive pain in the backside now thanks to the async browser hasher APIs and it impacts all of these things (for the recent @ipld/bitcoin upgrade I opted for @stablelib/sha256 instead of being hit with the async API cost because it goes through everything and I found myself having to add async to a ridiculous number of methods just for this one thing that was often on an optional path).

I guess it's up to you, and we can adjust later, it just might be an awkward adjustment.

Re

Make CID implementation of proposed Linkable interface.

Do you mean just adding a asLink() { return this } on the CID class? Or is there something more to this?

[Gozala]

Do you mean just adding a asLink() { return this } on the CID class? Or is there something more to this?

Assuming you meant toLink, than yes exactly that. Also I'm fine with calling it asLink instead or whatever really, as long as there is some canonical way to get CID for it.

[Gozala]

Alternatively we could just remove @private annotation below and call it a day

js-multiformats/src/cid.js

Lines 41 to 42 in 3d4ae50

	/** @private */
	this.asCID = this

That way other things could just implement asCID getters

[rvagg]

What other implications might there be for removing that @private? We're already using it in the wild anyway, e.g. https://github.com/ipld/js-dag-cbor/blob/c6249cbb6a9f0cbbcae1dd021b5d2b4362b47323/index.js#L21-L23

[Gozala]

What other implications might there be for removing that @private? We're already using it in the wild anyway, e.g. https://github.com/ipld/js-dag-cbor/blob/c6249cbb6a9f0cbbcae1dd021b5d2b4362b47323/index.js#L21-L23

More things having asCID fields and obj.asCID === obj been false. Other than that it might be worth pondering on:

1. Is asCID a better option than say a method like toLink ? (Personally I'm not a fan of disguising computations under field looking things, method seems more honest).
2. If we do ever want to have async version as discussed earlier, method is probably a better option than a getter returning a promise.

[rvagg]

Personally I'm not a fan of disguising computations under field looking things, method seems more honest

Yeah, so to be clear that I'm grokking your concern here - if we remove the @private then we encourage a pattern like const properCid = thing.asCID, and for thing that is not CID then they're going to have to implement a getter. And I agree, that's a garbage API, so probably -1 on removing @private on that one to encourage that style of usage.

toLink() is fine, I guess. But to be clear - my bias is always toward not adding new stuff. Legacy cost can be massive over time if you let iteration get out of control (just look inside cid.js to see that legacy cost in action). I just want to make sure that we have a really good reason to add new things - that it unlocks new workflows, or makes existing workflows much simpler. If it's just about letting someone remove a line of code - especially a line of code that may be usefully descriptive, then that's not so awesome, it's just iteration to simply get the feeling of "progress".

[rvagg]

#197 and #199