Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CID interface #161

Closed
wants to merge 23 commits into from
Closed

CID interface #161

wants to merge 23 commits into from

Conversation

Gozala
Copy link
Contributor

@Gozala Gozala commented Jan 19, 2022

Fixes #96

Adds CID interface that is far more loose than CID class. This change is intended to enable opt-in based transition at the edges in libraries that take CIDs but do not produce them. E.g writer piece of the @ipld/car is a good example where it does not need require direct dependency on CID implementation and interface would fully suffice.

This should not be a breaking change because CID class remains in place and has the same API as it did. In other words things that took CID class will continue to work as is. Libraries put CID interface in parameters will be able to accept CID class instances because those just implement the CID interface.

Problem MAY only occur if some code is annotated to return CID interface and that return value is then passed into other code that is annotated to take CID class. Generally speaking it would be good idea to accept CID as parameter everywhere and return CID as class. I hope over long period of time we will be able to kill the class.

Other notable changes:

  1. I made CID interface and class optionally generic, what that means things can continue referring to it simply as CID, but other things can be more specific capturing codec, hashing and version info along.
  2. Former _baseCache property got replaced by cache WeakMap in a cid module. Behavior is the same but now things are no longer tied to a specific class implementation. Testing strategy had to change, but caching is still tested.
  3. I have added interface.js files shadowed by interface.ts that way all the types can be imported as module and referenced as e.g. API.MultihashDigest etc.. as opposed to having to copy soup of jsdoc for every import everywhere.

@rvagg @achingbrain @lidel This change should no longer require updating existing code, unless I'm overlooking something. Yet it should enable new code and code at the edges to be bit more relaxed about CID requirements.

@Gozala Gozala requested review from rvagg and achingbrain January 19, 2022 09:13
@achingbrain
Copy link
Member

Can you separate out the CID interface change from the hasher sync/async stuff in this PR? It will make reviewing it easier.

@@ -97,6 +97,9 @@
},
"./codecs/raw": {
"import": "./src/codecs/raw.js"
},
"./interface": {
"import": "./src/interface.js"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so that one could import and use all the types as follows:

import * as API from 'multiformats/interface.js'

/**
 * @param {API.CID} cid
 */
export const test = (cid) => {
   // ...
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a case for exporting ./cid/interface since it's going to be quite common that this is all you want?

src/index.js Outdated Show resolved Hide resolved
@Gozala
Copy link
Contributor Author

Gozala commented Jan 19, 2022

Can you separate out the CID interface change from the hasher sync/async stuff in this PR? It will make reviewing it easier.

I have landed the other PR and cleanup PR to remove unnecessary noise.

@Gozala
Copy link
Contributor Author

Gozala commented Feb 1, 2022

@rvagg @achingbrain any thoughts on the matter ? I keep finding myself having to @ts-ignore which CID class I'm referring to and I would really love to address that one way or the other.

src/bases/base.js Outdated Show resolved Hide resolved
src/bases/base.js Outdated Show resolved Hide resolved
@rvagg
Copy link
Member

rvagg commented Feb 2, 2022

OK, so we first need to assess the scope of breakage and decide if it's worth it or not, for our ecosystem and for our codebases that use this the most (js-ipfs is going to be the most heavily impacted on both counts I think).

For a JavaScript user, it seems to me that the breakage only comes (or is only supposed to come) from the case where you're importing multiformats/cid, in which case a import { CID } from 'multiformats/cid' would need to become import * as CID from 'multiformats/cid'. Users would experience that breakage by getting errors like CID is undefined when they try to use it, or perhaps their loader will pick it up as an unexported object.

Is there any way to reduce that breakage? If we exported a CID symbol as well as the bare export then our types are going to get messed up, like they did for the recent murmur3 breakage (and we had a similar breakage on js-dag-pb a while back iirc). If we continue to export just a CID symbol, as we do now, we get some type + symbol conflicts, don't we? Trying to export an object or class called CID is going to have a bad time if we're also trying to use the CID type in that file? Or can we rename the import of the CID type in that file to something else? .. basically: is there any practical way to remove this bit of breakage for JS users to make this less painful?

For TypeScript users, the breakage is a bit more broad I think, but I'm not super clear on that. @Gozala could you explain the scope of breakage that you see, that we'd need to take into account for an "is it worth it" decision?

I'm just very conscious that we already forced the ecosystem through a very large breakage to adopt this package and we need to be very careful making that a pattern. If we break, we do it for very good reasons and we also try and keep the breakage to a minimum (and document clearly any migration patterns for those that really don't have the time or inclination to understand the subtleties of our decisions).

(cid.js needs a thorough review but it's difficult in the diff here, I can take a look at that if we end up agreeing that this is worthwhile).

@rvagg
Copy link
Member

rvagg commented Feb 2, 2022

@BigLep maybe we could pull you in to help get to a more decisive place on this. At least getting it on a project board to make sure we don't let it slip too far.

  • We’ve already put the JS ecosystem through a huge breakage last year with our universal migration to js-multiformats & the new IPLD stack. It was painful but put us in a much better place.
  • This change has more breakage, although nowhere near as large.
  • CID as a typed interface rather than a strictly concrete class has a lot of upside.
  • Boils down to: Is the cost of breakage for the ecosystem worth it for the benefits.

This'll impact js-ipfs the most, so @achingbrain would need to agree to this, (unless @Gozala is proposing to do the migration work there?). But as per my comment above, I'd like to get a much clearer picture of the scope of the breakage for both JS & TS users so we can do an appropriate cost/benefit on this.

@achingbrain
Copy link
Member

achingbrain commented Feb 2, 2022

Before we go any further I just want to make sure we're solving the right problem here because this is going to be incredibly disruptive and we really need to make sure the change will be worth the pain it's going to inflict on everyone using this module ecosystem.

In fact I often encounter problem when working on this repo, because install seems to bring multiformats into node_modules which then causes misalignment between CIDs.

I keep finding myself having to @ts-ignore which CID class I'm referring to and I would really love to address that one way or the other.

I don't understand why you would keep needing to do this? Do you have multiple copies of the multiformats module in your dependency tree?

If so, and you fix your dependency tree to not have multiple copies of the multiformats module does the problem go away?

@BigLep
Copy link

BigLep commented Feb 3, 2022

Thanks for the flag. On a quick read it seems like:

  1. We don't have clear consensus among maintainers that this change is needed
  2. We don't have a staffing plan to do the work. This doesn't seem high up the "JS Stewards" (team of 1) priority stack.

As a result, without new information, I don't see this moving forward.

@Gozala
Copy link
Contributor Author

Gozala commented Feb 3, 2022

Before we go any further I just want to make sure we're solving the right problem here because this is going to be incredibly disruptive and we really need to make sure the change will be worth the pain it's going to inflict on everyone using this module ecosystem.

I think we can make it less disruptive if we so choose. PR here is on more disruptive side, but we could also explore alternative where interface def basically mimics the class. That would mean some code could take CIDs (and there for class) and other code will continue doing what it does, presumably some day loosening requirements.

I don't understand why you would keep needing to do this? Do you have multiple copies of the multiformats module in your dependency tree?

If so, and you fix your dependency tree to not have multiple copies of the multiformats module does the problem go away?

Indeed in many cases two different dependencies bring different versions of multiformats and introduce problems, even when in practice code that I write just passes things between them never creating CID.

Sure you could resolve this by aligning dependencies everywhere, but that usually not always possible without having to go chasing various libs and releasing updates. You may argue that is what needs to be done, however in my case when my code just accepts some codec implementation and some blockstore implementation without explicitly depending on either this becomes an incidental complexity. All I need to say I implement a think that makes two of these interfaces work, but then it does not because well we don't use interfaces for CIDs so you have issues even if you only ever refer to CID as types.

@Gozala
Copy link
Contributor Author

Gozala commented Feb 3, 2022

This'll impact js-ipfs the most, so @achingbrain would need to agree to this, (unless @Gozala is proposing to do the migration work there?). But as per my comment above, I'd like to get a much clearer picture of the scope of the breakage for both JS & TS users so we can do an appropriate cost/benefit on this.

What I'd be willing to do here is propose changes that would have no effect on js-ipfs, it can continue referring to CID class. I will however change various libraries as needed so that they can take CID interfaces without requiring a CID instance while still produce CID class.

That way:

  1. things I need to interface with will not require CID class.
  2. JS IPFS can continue demanding CID class instances, no change there is needed.
  3. Things I need to interface can still continue producing CID classes so they remain compatible with js-ipfs's demand for CID instance.

If some day js-ipfs allows passing CID interfaces that would not be a breaking change either as long as it continues to return CID instance.

@Gozala
Copy link
Contributor Author

Gozala commented Feb 3, 2022

@BigLep @achingbrain If this is about who's going to do all this work to update js-ipfs we can discuss this. But would not want to commit all the time required to submitting changes for js-ipfs to see them bitrot. In other words it would be great if we could decide whether this is worth doing separate from whether we have resources to do it now, because if answers are Yes & No we could try and figure out the way to find resources, but if first one is No that there is no point in pursuing this any further.

@lidel
Copy link
Member

lidel commented Feb 18, 2022

I think we can make it less disruptive if we so choose. PR here is on more disruptive side, but we could also explore alternative

We discussed this during triage, and this PR as-is, is too disruptive: generates work beyond refactoring everything in JS land, but also hard to measure hours of user support.
An alternative approach should be pursued.

@rvagg
Copy link
Member

rvagg commented Jun 6, 2022

@Gozala yes, absolutely, I want to get this in but am very concerned about breakage so it's waiting for me to spend a good chunk of time going through it with an eye for breakage and maybe even writing some code that exercises it that way. It's on the board at the top of the In Progress list now. I'll try and work on it in the next couple of days.

@rvagg
Copy link
Member

rvagg commented Jun 6, 2022

For reference, on the types level, this is what I'm looking at as the diff between this and master in what's exported:

dist/types diff
diff -Naur /tmp/master-types/src/bases/base.d.ts dist/types/src/bases/base.d.ts
--- /tmp/master-types/src/bases/base.d.ts	2022-06-06 15:55:05.782030667 +1000
+++ dist/types/src/bases/base.d.ts	2022-06-06 15:55:38.262276400 +1000
@@ -1,20 +1,16 @@
-export function or<L extends string, R extends string>(left: import("./interface").UnibaseDecoder<L> | import("./interface").CombobaseDecoder<L>, right: import("./interface").UnibaseDecoder<R> | import("./interface").CombobaseDecoder<R>): ComposedDecoder<L | R>;
-/**
- * @template T
- * @typedef {import('./interface').MultibaseCodec<T>} MultibaseCodec
- */
+export function or<L extends string, R extends string>(left: API.UnibaseDecoder<L> | API.CombobaseDecoder<L>, right: API.UnibaseDecoder<R> | API.CombobaseDecoder<R>): ComposedDecoder<L | R>;
 /**
  * @class
  * @template {string} Base
  * @template {string} Prefix
- * @implements {MultibaseCodec<Prefix>}
- * @implements {MultibaseEncoder<Prefix>}
- * @implements {MultibaseDecoder<Prefix>}
- * @implements {BaseCodec}
- * @implements {BaseEncoder}
- * @implements {BaseDecoder}
+ * @implements {API.MultibaseCodec<Prefix>}
+ * @implements {API.MultibaseEncoder<Prefix>}
+ * @implements {API.MultibaseDecoder<Prefix>}
+ * @implements {API.BaseCodec}
+ * @implements {API.BaseEncoder}
+ * @implements {API.BaseDecoder}
  */
-export class Codec<Base extends string, Prefix extends string> implements MultibaseCodec<Prefix>, MultibaseEncoder<Prefix>, MultibaseDecoder<Prefix>, BaseCodec, BaseEncoder, BaseDecoder {
+export class Codec<Base extends string, Prefix extends string> implements API.MultibaseCodec<Prefix>, API.MultibaseEncoder<Prefix>, API.MultibaseDecoder<Prefix>, API.BaseCodec, API.BaseEncoder, API.BaseDecoder {
     /**
      * @param {Base} name
      * @param {Prefix} prefix
@@ -31,7 +27,7 @@
     /**
      * @param {Uint8Array} input
      */
-    encode(input: Uint8Array): Multibase<Prefix>;
+    encode(input: Uint8Array): API.Multibase<Prefix>;
     /**
      * @param {string} input
      */
@@ -54,41 +50,29 @@
     alphabet: string;
     bitsPerChar: number;
 }): Codec<Base, Prefix>;
-export type BaseEncoder = import('./interface').BaseEncoder;
-export type BaseDecoder = import('./interface').BaseDecoder;
-export type BaseCodec = import('./interface').BaseCodec;
-export type Multibase<T extends string> = import('./interface').Multibase<T>;
-export type MultibaseEncoder<T extends string> = import('./interface').MultibaseEncoder<T>;
-export type MultibaseDecoder<Prefix extends string> = import('./interface').MultibaseDecoder<Prefix>;
-export type UnibaseDecoder<Prefix extends string> = import('./interface').UnibaseDecoder<Prefix>;
-export type CombobaseDecoder<Prefix extends string> = import('./interface').CombobaseDecoder<Prefix>;
-export type Decoders<Prefix extends string> = Record<Prefix, UnibaseDecoder<Prefix>>;
-export type MultibaseCodec<T> = import('./interface').MultibaseCodec<T>;
-/**
- * @template {string} Prefix
- * @typedef {import('./interface').CombobaseDecoder<Prefix>} CombobaseDecoder
- */
+export type Decoders<Prefix extends string> = Record<Prefix, API.UnibaseDecoder<Prefix>>;
+import * as API from "./interface.js";
 /**
  * @template {string} Prefix
- * @typedef {Record<Prefix, UnibaseDecoder<Prefix>>} Decoders
+ * @typedef {Record<Prefix, API.UnibaseDecoder<Prefix>>} Decoders
  */
 /**
  * @template {string} Prefix
- * @implements {MultibaseDecoder<Prefix>}
- * @implements {CombobaseDecoder<Prefix>}
+ * @implements {API.MultibaseDecoder<Prefix>}
+ * @implements {API.CombobaseDecoder<Prefix>}
  */
-declare class ComposedDecoder<Prefix extends string> implements MultibaseDecoder<Prefix>, CombobaseDecoder<Prefix> {
+declare class ComposedDecoder<Prefix extends string> implements API.MultibaseDecoder<Prefix>, API.CombobaseDecoder<Prefix> {
     /**
-     * @param {Record<Prefix, UnibaseDecoder<Prefix>>} decoders
+     * @param {Decoders<Prefix>} decoders
      */
-    constructor(decoders: Record<Prefix, UnibaseDecoder<Prefix>>);
-    decoders: Record<Prefix, import("./interface").UnibaseDecoder<Prefix>>;
+    constructor(decoders: Decoders<Prefix>);
+    decoders: Decoders<Prefix>;
     /**
      * @template {string} OtherPrefix
-     * @param {UnibaseDecoder<OtherPrefix>|ComposedDecoder<OtherPrefix>} decoder
+     * @param {API.UnibaseDecoder<OtherPrefix>|ComposedDecoder<OtherPrefix>} decoder
      * @returns {ComposedDecoder<Prefix|OtherPrefix>}
      */
-    or<OtherPrefix extends string>(decoder: import("./interface").UnibaseDecoder<OtherPrefix> | ComposedDecoder<OtherPrefix>): ComposedDecoder<Prefix | OtherPrefix>;
+    or<OtherPrefix extends string>(decoder: API.UnibaseDecoder<OtherPrefix> | ComposedDecoder<OtherPrefix>): ComposedDecoder<Prefix | OtherPrefix>;
     /**
      * @param {string} input
      * @returns {Uint8Array}
@@ -96,29 +80,16 @@
     decode(input: string): Uint8Array;
 }
 /**
- * @typedef {import('./interface').BaseEncoder} BaseEncoder
- * @typedef {import('./interface').BaseDecoder} BaseDecoder
- * @typedef {import('./interface').BaseCodec} BaseCodec
- */
-/**
- * @template {string} T
- * @typedef {import('./interface').Multibase<T>} Multibase
- */
-/**
- * @template {string} T
- * @typedef {import('./interface').MultibaseEncoder<T>} MultibaseEncoder
- */
-/**
  * Class represents both BaseEncoder and MultibaseEncoder meaning it
  * can be used to encode to multibase or base encode without multibase
  * prefix.
  * @class
  * @template {string} Base
  * @template {string} Prefix
- * @implements {MultibaseEncoder<Prefix>}
- * @implements {BaseEncoder}
+ * @implements {API.MultibaseEncoder<Prefix>}
+ * @implements {API.BaseEncoder}
  */
-declare class Encoder<Base extends string, Prefix extends string> implements MultibaseEncoder<Prefix>, BaseEncoder {
+declare class Encoder<Base extends string, Prefix extends string> implements API.MultibaseEncoder<Prefix>, API.BaseEncoder {
     /**
      * @param {Base} name
      * @param {Prefix} prefix
@@ -130,33 +101,24 @@
     baseEncode: (bytes: Uint8Array) => string;
     /**
      * @param {Uint8Array} bytes
-     * @returns {Multibase<Prefix>}
+     * @returns {API.Multibase<Prefix>}
      */
-    encode(bytes: Uint8Array): Multibase<Prefix>;
+    encode(bytes: Uint8Array): API.Multibase<Prefix>;
 }
 /**
  * @template {string} Prefix
- * @typedef {import('./interface').MultibaseDecoder<Prefix>} MultibaseDecoder
- */
-/**
- * @template {string} Prefix
- * @typedef {import('./interface').UnibaseDecoder<Prefix>} UnibaseDecoder
- */
-/**
- * @template {string} Prefix
  */
 /**
  * Class represents both BaseDecoder and MultibaseDecoder so it could be used
  * to decode multibases (with matching prefix) or just base decode strings
  * with corresponding base encoding.
- * @class
  * @template {string} Base
  * @template {string} Prefix
- * @implements {MultibaseDecoder<Prefix>}
- * @implements {UnibaseDecoder<Prefix>}
- * @implements {BaseDecoder}
+ * @implements {API.MultibaseDecoder<Prefix>}
+ * @implements {API.UnibaseDecoder<Prefix>}
+ * @implements {API.BaseDecoder}
  */
-declare class Decoder<Base extends string, Prefix extends string> implements MultibaseDecoder<Prefix>, UnibaseDecoder<Prefix>, BaseDecoder {
+declare class Decoder<Base extends string, Prefix extends string> implements API.MultibaseDecoder<Prefix>, API.UnibaseDecoder<Prefix>, API.BaseDecoder {
     /**
      * @param {Base} name
      * @param {Prefix} prefix
@@ -172,10 +134,10 @@
     decode(text: string): Uint8Array;
     /**
      * @template {string} OtherPrefix
-     * @param {UnibaseDecoder<OtherPrefix>|ComposedDecoder<OtherPrefix>} decoder
+     * @param {API.UnibaseDecoder<OtherPrefix>|ComposedDecoder<OtherPrefix>} decoder
      * @returns {ComposedDecoder<Prefix|OtherPrefix>}
      */
-    or<OtherPrefix extends string>(decoder: import("./interface").UnibaseDecoder<OtherPrefix> | ComposedDecoder<OtherPrefix>): ComposedDecoder<Prefix | OtherPrefix>;
+    or<OtherPrefix extends string>(decoder: API.UnibaseDecoder<OtherPrefix> | ComposedDecoder<OtherPrefix>): ComposedDecoder<Prefix | OtherPrefix>;
 }
 export {};
 //# sourceMappingURL=base.d.ts.map
\ No newline at end of file
diff -Naur /tmp/master-types/src/block.d.ts dist/types/src/block.d.ts
--- /tmp/master-types/src/block.d.ts	2022-06-06 15:55:05.786030696 +1000
+++ dist/types/src/block.d.ts	2022-06-06 15:55:38.266276429 +1000
@@ -1,77 +1,73 @@
 export type RequiredCreateOptions = {
-    cid: CID;
+    cid: API.CID;
 };
-export type ByteView<T> = import('./codecs/interface').ByteView<T>;
-export type BlockEncoder<Code extends number, T> = import('./codecs/interface').BlockEncoder<Code, T>;
-export type BlockDecoder<Code extends number, T> = import('./codecs/interface').BlockDecoder<Code, T>;
-export type Hasher<Algorithm_1> = import('./hashes/interface').MultihashHasher;
 /**
  * @template T
  * @template {number} Code
- * @template {number} Algorithm
+ * @template {number} Alg
  * @param {Object} options
  * @param {T} options.value
- * @param {BlockEncoder<Code, T>} options.codec
- * @param {Hasher<Algorithm>} options.hasher
+ * @param {API.BlockEncoder<Code, T>} options.codec
+ * @param {API.MultihashHasher<Alg>} options.hasher
  * @returns {Promise<Block<T>>}
  */
-export function encode<T, Code extends number, Algorithm_1 extends number>({ value, codec, hasher }: {
+export function encode<T, Code extends number, Alg extends number>({ value, codec, hasher }: {
     value: T;
-    codec: import("./codecs/interface").BlockEncoder<Code, T>;
-    hasher: import("./hashes/interface").MultihashHasher<number>;
+    codec: API.BlockEncoder<Code, T>;
+    hasher: API.MultihashHasher<Alg>;
 }): Promise<Block<T>>;
 /**
  * @template T
  * @template {number} Code
- * @template {number} Algorithm
+ * @template {number} Alg
  * @param {Object} options
- * @param {ByteView<T>} options.bytes
- * @param {BlockDecoder<Code, T>} options.codec
- * @param {Hasher<Algorithm>} options.hasher
+ * @param {API.ByteView<T>} options.bytes
+ * @param {API.BlockDecoder<Code, T>} options.codec
+ * @param {API.MultihashHasher<Alg>} options.hasher
  * @returns {Promise<Block<T>>}
  */
-export function decode<T, Code extends number, Algorithm_1 extends number>({ bytes, codec, hasher }: {
-    bytes: ByteView<T>;
-    codec: import("./codecs/interface").BlockDecoder<Code, T>;
-    hasher: import("./hashes/interface").MultihashHasher<number>;
+export function decode<T, Code extends number, Alg extends number>({ bytes, codec, hasher }: {
+    bytes: API.ByteView<T>;
+    codec: API.BlockDecoder<Code, T>;
+    hasher: API.MultihashHasher<Alg>;
 }): Promise<Block<T>>;
 /**
  * @template T
  * @template {number} Code
- * @template {number} Algorithm
+ * @template {number} Alg
  * @param {Object} options
- * @param {CID} options.cid
- * @param {ByteView<T>} options.bytes
- * @param {BlockDecoder<Code, T>} options.codec
- * @param {Hasher<Algorithm>} options.hasher
+ * @param {API.CID<Code, Alg>} options.cid
+ * @param {API.ByteView<T>} options.bytes
+ * @param {API.BlockDecoder<Code, T>} options.codec
+ * @param {API.MultihashHasher<Alg>} options.hasher
  * @returns {Promise<Block<T>>}
  */
-export function create<T, Code extends number, Algorithm_1 extends number>({ bytes, cid, hasher, codec }: {
-    cid: CID;
-    bytes: ByteView<T>;
-    codec: import("./codecs/interface").BlockDecoder<Code, T>;
-    hasher: import("./hashes/interface").MultihashHasher<number>;
+export function create<T, Code extends number, Alg extends number>({ bytes, cid, hasher, codec }: {
+    cid: API.CID<Code, Alg, API.CIDVersion>;
+    bytes: API.ByteView<T>;
+    codec: API.BlockDecoder<Code, T>;
+    hasher: API.MultihashHasher<Alg>;
 }): Promise<Block<T>>;
 /**
  * @typedef {Object} RequiredCreateOptions
- * @property {CID} options.cid
+ * @property {API.CID} options.cid
  */
 /**
  * @template T
  * @template {number} Code
- * @param {{ cid: CID, value:T, codec?: BlockDecoder<Code, T>, bytes: ByteView<T> }|{cid:CID, bytes:ByteView<T>, value?:void, codec:BlockDecoder<Code, T>}} options
+ * @param {{ cid: API.CID, value:T, codec?: API.BlockDecoder<Code, T>, bytes: API.ByteView<T> }|{cid:API.CID, bytes:API.ByteView<T>, value?:void, codec:API.BlockDecoder<Code, T>}} options
  * @returns {Block<T>}
  */
 export function createUnsafe<T, Code extends number>({ bytes, cid, value: maybeValue, codec }: {
-    cid: CID;
+    cid: API.CID;
     value: T;
-    codec?: import("./codecs/interface").BlockDecoder<Code, T> | undefined;
-    bytes: ByteView<T>;
+    codec?: API.BlockDecoder<Code, T> | undefined;
+    bytes: API.ByteView<T>;
 } | {
-    cid: CID;
-    bytes: ByteView<T>;
+    cid: API.CID;
+    bytes: API.ByteView<T>;
     value?: void | undefined;
-    codec: import("./codecs/interface").BlockDecoder<Code, T>;
+    codec: API.BlockDecoder<Code, T>;
 }): Block<T>;
 /**
  * @template T
@@ -79,31 +75,32 @@
 export class Block<T> {
     /**
      * @param {Object} options
-     * @param {CID} options.cid
-     * @param {ByteView<T>} options.bytes
+     * @param {API.CID} options.cid
+     * @param {API.ByteView<T>} options.bytes
      * @param {T} options.value
      */
     constructor({ cid, bytes, value }: {
-        cid: CID;
-        bytes: ByteView<T>;
+        cid: API.CID;
+        bytes: API.ByteView<T>;
         value: T;
     });
-    cid: CID;
-    bytes: ByteView<T>;
+    cid: CID<number, number, API.CIDVersion>;
+    bytes: API.ByteView<T>;
     value: T;
     asBlock: Block<T>;
-    links(): Iterable<[string, CID]>;
+    links(): Iterable<[string, CID<number, number, API.CIDVersion>]>;
     tree(): Iterable<string>;
     /**
    * @param {string} [path]
    */
     get(path?: string | undefined): {
-        value: CID;
+        value: CID<number, number, API.CIDVersion>;
         remaining: string;
     } | {
         value: Record<string, any>;
         remaining?: undefined;
     };
 }
+import * as API from "./interface.js";
 import { CID } from "./index.js";
 //# sourceMappingURL=block.d.ts.map
\ No newline at end of file
diff -Naur /tmp/master-types/src/cid/interface.d.ts dist/types/src/cid/interface.d.ts
--- /tmp/master-types/src/cid/interface.d.ts	1970-01-01 10:00:00.000000000 +1000
+++ dist/types/src/cid/interface.d.ts	2022-06-06 15:55:38.262276400 +1000
@@ -0,0 +1,32 @@
+import type { MultihashDigest } from '../hashes/interface';
+import type { MultibaseEncoder, MultibaseDecoder } from '../bases/interface';
+export type { MultihashDigest, MultibaseEncoder, MultibaseDecoder };
+export declare type CIDVersion = 0 | 1;
+export declare type DAG_PB = 0x70;
+export declare type SHA_256 = 0x12;
+export interface CID<Format extends number = number, Alg extends number = number, Version extends CIDVersion = CIDVersion> {
+    readonly version: Version;
+    readonly code: Format;
+    readonly multihash: MultihashDigest<Alg>;
+    readonly byteOffset: number;
+    readonly byteLength: number;
+    readonly bytes: Uint8Array;
+    readonly asCID: this;
+    equals(other: unknown): other is CID<Format, Alg, Version>;
+    toString(base?: MultibaseEncoder<string>): string;
+    toJSON(): {
+        version: Version;
+        code: Format;
+        hash: Uint8Array;
+    };
+    toV0(): CIDv0;
+    toV1(): CIDv1;
+}
+export interface CIDv0 extends CID<DAG_PB, SHA_256, 0> {
+    readonly version: 0;
+}
+export interface CIDv1<Format extends number = number, Alg extends number = number> extends CID<Format, Alg, 1> {
+    readonly version: 1;
+}
+export type { CID as CIDType };
+//# sourceMappingURL=interface.d.ts.map
\ No newline at end of file
diff -Naur /tmp/master-types/src/cid.d.ts dist/types/src/cid.d.ts
--- /tmp/master-types/src/cid.d.ts	2022-06-06 15:55:05.786030696 +1000
+++ dist/types/src/cid.d.ts	2022-06-06 15:55:38.262276400 +1000
@@ -1,93 +1,72 @@
+export * from "./cid/interface.js";
+export function asCID<Format extends number, Alg extends number, Version extends API.CIDVersion, T>(input: T | API.CID<Format, Alg, Version>): API.CID<Format, Alg, Version> | null;
+export function create<Format extends number, Alg extends number, Version extends API.CIDVersion>(version: Version, code: Format, digest: API.MultihashDigest<Alg>): API.CID<Format, Alg, Version>;
+export function createV0(digest: API.MultihashDigest<typeof SHA_256_CODE>): API.CIDv0;
+export function createV1<Code extends number, Alg extends number>(code: Code, digest: API.MultihashDigest<Alg>): API.CIDv1<Code, Alg>;
+export function decode(bytes: Uint8Array): API.CID;
+export function decodeFirst(bytes: Uint8Array): [API.CID, Uint8Array];
+export function inspectBytes(initialBytes: Uint8Array): {
+    version: API.CIDVersion;
+    codec: number;
+    multihashCode: number;
+    digestSize: number;
+    multihashSize: number;
+    size: number;
+};
+export function parse<Prefix extends string>(source: string, base?: API.MultibaseDecoder<Prefix> | undefined): API.CID;
+export function equals<Format extends number, Alg extends number, Version extends API.CIDVersion>(cid: API.CID<Format, Alg, Version>, other: any): other is API.CID<Format, Alg, Version>;
+export function toString(cid: API.CID, base?: API.MultibaseEncoder<string> | undefined): string;
 /**
- * @typedef {import('./hashes/interface').MultihashDigest} MultihashDigest
- * @typedef {0 | 1} CIDVersion
+ * @template {number} [Format=number]
+ * @template {number} [Alg=number]
+ * @template {API.CIDVersion} [Version=API.CIDVersion]
+ * @implements {API.CID<Format, Alg, Version>}
  */
-/**
- * @template Prefix
- * @typedef {import('./bases/interface').MultibaseEncoder<Prefix>} MultibaseEncoder
- */
-/**
- * @template Prefix
- * @typedef {import('./bases/interface').MultibaseDecoder<Prefix>} MultibaseDecoder
- */
-export class CID {
+export class CID<Format extends number = number, Alg extends number = number, Version extends API.CIDVersion = API.CIDVersion> implements API.CID<Format, Alg, Version> {
     /**
      * @param {any} value
      * @returns {value is CID}
      */
-    static isCID(value: any): value is CID;
+    static isCID(value: any): value is CID<number, number, API.CIDVersion>;
     /**
-     * Takes any input `value` and returns a `CID` instance if it was
-     * a `CID` otherwise returns `null`. If `value` is instanceof `CID`
-     * it will return value back. If `value` is not instance of this CID
-     * class, but is compatible CID it will return new instance of this
-     * `CID` class. Otherwise returs null.
-     *
-     * This allows two different incompatible versions of CID library to
-     * co-exist and interop as long as binary interface is compatible.
      * @param {any} value
-     * @returns {CID|null}
      */
-    static asCID(value: any): CID | null;
+    static asCID(value: any): CID<number, number, API.CIDVersion> | null;
     /**
-   *
-   * @param {CIDVersion} version - Version of the CID
-   * @param {number} code - Code of the codec content is encoded in.
-   * @param {MultihashDigest} digest - (Multi)hash of the of the content.
-   * @returns {CID}
-   */
-    static create(version: CIDVersion, code: number, digest: MultihashDigest): CID;
+     * @template {number} Format
+     * @template {number} Alg
+     * @template {API.CIDVersion} Version
+     * @param {Version} version - Version of the CID
+     * @param {Format} code - Code of the codec content is encoded in.
+     * @param {API.MultihashDigest<Alg>} digest - (Multi)hash of the of the content.
+     */
+    static create<Format_1 extends number, Alg_1 extends number, Version_1 extends API.CIDVersion>(version: Version_1, code: Format_1, digest: API.MultihashDigest<Alg_1>): CID<Format_1, Alg_1, Version_1>;
     /**
      * Simplified version of `create` for CIDv0.
-     * @param {MultihashDigest} digest - Multihash.
+     * @param {API.MultihashDigest<typeof SHA_256_CODE>} digest - Multihash.
      */
-    static createV0(digest: MultihashDigest): CID;
+    static createV0(digest: API.MultihashDigest<typeof SHA_256_CODE>): CID<112, 18, 0>;
     /**
    * Simplified version of `create` for CIDv1.
    * @template {number} Code
+   * @template {number} Alg
    * @param {Code} code - Content encoding format code.
-   * @param {MultihashDigest} digest - Miltihash of the content.
-   * @returns {CID}
+   * @param {API.MultihashDigest<Alg>} digest - Miltihash of the content.
    */
-    static createV1<Code extends number>(code: Code, digest: MultihashDigest): CID;
+    static createV1<Code extends number, Alg_2 extends number>(code: Code, digest: API.MultihashDigest<Alg_2>): CID<Code, Alg_2, 1>;
     /**
-     * Decoded a CID from its binary representation. The byte array must contain
-     * only the CID with no additional bytes.
-     *
-     * An error will be thrown if the bytes provided do not contain a valid
-     * binary representation of a CID.
-     *
      * @param {Uint8Array} bytes
-     * @returns {CID}
      */
-    static decode(bytes: Uint8Array): CID;
+    static decode(bytes: Uint8Array): CID<number, number, API.CIDVersion>;
     /**
-     * Decoded a CID from its binary representation at the beginning of a byte
-     * array.
-     *
-     * Returns an array with the first element containing the CID and the second
-     * element containing the remainder of the original byte array. The remainder
-     * will be a zero-length byte array if the provided bytes only contained a
-     * binary CID representation.
-     *
      * @param {Uint8Array} bytes
-     * @returns {[CID, Uint8Array]}
      */
-    static decodeFirst(bytes: Uint8Array): [CID, Uint8Array];
+    static decodeFirst(bytes: Uint8Array): [CID<number, number, API.CIDVersion>, Uint8Array];
     /**
-     * Inspect the initial bytes of a CID to determine its properties.
-     *
-     * Involves decoding up to 4 varints. Typically this will require only 4 to 6
-     * bytes but for larger multicodec code values and larger multihash digest
-     * lengths these varints can be quite large. It is recommended that at least
-     * 10 bytes be made available in the `initialBytes` argument for a complete
-     * inspection.
-     *
      * @param {Uint8Array} initialBytes
-     * @returns {{ version:CIDVersion, codec:number, multihashCode:number, digestSize:number, multihashSize:number, size:number }}
      */
     static inspectBytes(initialBytes: Uint8Array): {
-        version: CIDVersion;
+        version: API.CIDVersion;
         codec: number;
         multihashCode: number;
         digestSize: number;
@@ -95,57 +74,54 @@
         size: number;
     };
     /**
-     * Takes cid in a string representation and creates an instance. If `base`
-     * decoder is not provided will use a default from the configuration. It will
-     * throw an error if encoding of the CID is not compatible with supplied (or
-     * a default decoder).
-     *
      * @template {string} Prefix
      * @param {string} source
-     * @param {MultibaseDecoder<Prefix>} [base]
+     * @param {API.MultibaseDecoder<Prefix>} [base]
      */
-    static parse<Prefix extends string>(source: string, base?: import("./bases/interface").MultibaseDecoder<Prefix> | undefined): CID;
+    static parse<Prefix extends string>(source: string, base?: API.MultibaseDecoder<Prefix> | undefined): CID<number, number, API.CIDVersion>;
     /**
-     * @param {CIDVersion} version
-     * @param {number} code - multicodec code, see https://github.com/multiformats/multicodec/blob/master/table.csv
-     * @param {MultihashDigest} multihash
+     * @param {Version} version
+     * @param {Format} code
+     * @param {API.MultihashDigest<Alg>} multihash
      * @param {Uint8Array} bytes
      *
      */
-    constructor(version: CIDVersion, code: number, multihash: MultihashDigest, bytes: Uint8Array);
-    code: number;
-    version: CIDVersion;
-    multihash: import("./hashes/interface").MultihashDigest<number>;
-    bytes: Uint8Array;
-    byteOffset: number;
-    byteLength: number;
-    /** @private */
-    private asCID;
-    /**
-     * @type {Map<string, string>}
-     * @private
-     */
-    private _baseCache;
-    /**
-     * @returns {CID}
-     */
-    toV0(): CID;
-    /**
-     * @returns {CID}
+    constructor(version: Version, code: Format, multihash: API.MultihashDigest<Alg>, bytes: Uint8Array);
+    /** @readonly */
+    readonly code: Format;
+    /** @readonly */
+    readonly version: Version;
+    /** @readonly */
+    readonly multihash: API.MultihashDigest<Alg>;
+    /** @readonly */
+    readonly bytes: Uint8Array;
+    /** @readonly */
+    readonly byteOffset: number;
+    /** @readonly */
+    readonly byteLength: number;
+    /** @readonly */
+    readonly asCID: CID<Format, Alg, Version>;
+    /**
+     * @returns {CID<API.DAG_PB, API.SHA_256, 0>}
+     */
+    toV0(): CID<API.DAG_PB, API.SHA_256, 0>;
+    /**
+     * @returns {CID<Format, Alg, 1>}
      */
-    toV1(): CID;
+    toV1(): CID<Format, Alg, 1>;
     /**
-     * @param {any} other
+     * @param {unknown} other
+     * @returns {other is CID<Format, Alg, Version>}
      */
-    equals(other: any): any;
+    equals(other: unknown): other is CID<Format, Alg, Version>;
     /**
-     * @param {MultibaseEncoder<any>} [base]
+     * @param {API.MultibaseEncoder<string>} [base]
      * @returns {string}
      */
-    toString(base?: import("./bases/interface").MultibaseEncoder<any> | undefined): string;
+    toString(base?: API.MultibaseEncoder<string> | undefined): string;
     toJSON(): {
-        code: number;
-        version: CIDVersion;
+        code: Format;
+        version: Version;
         hash: Uint8Array;
     };
     get toBaseEncodedString(): void;
@@ -155,8 +131,6 @@
     get prefix(): void;
     get [Symbol.toStringTag](): string;
 }
-export type MultihashDigest = import('./hashes/interface').MultihashDigest;
-export type CIDVersion = 0 | 1;
-export type MultibaseEncoder<Prefix> = import('./bases/interface').MultibaseEncoder<Prefix>;
-export type MultibaseDecoder<Prefix> = import('./bases/interface').MultibaseDecoder<Prefix>;
+import * as API from "./cid/interface.js";
+declare const SHA_256_CODE: 18;
 //# sourceMappingURL=cid.d.ts.map
\ No newline at end of file
diff -Naur /tmp/master-types/src/index.d.ts dist/types/src/index.d.ts
--- /tmp/master-types/src/index.d.ts	2022-06-06 15:55:05.786030696 +1000
+++ dist/types/src/index.d.ts	2022-06-06 15:55:38.262276400 +1000
@@ -1,3 +1,4 @@
+export * from "./interface.js";
 import { CID } from "./cid.js";
 import * as hasher from "./hashes/hasher.js";
 import * as digest from "./hashes/digest.js";
diff -Naur /tmp/master-types/src/interface.d.ts dist/types/src/interface.d.ts
--- /tmp/master-types/src/interface.d.ts	1970-01-01 10:00:00.000000000 +1000
+++ dist/types/src/interface.d.ts	2022-06-06 15:55:38.262276400 +1000
@@ -0,0 +1,5 @@
+export * from './bases/interface';
+export * from './hashes/interface';
+export * from './codecs/interface';
+export * from './cid/interface';
+//# sourceMappingURL=interface.d.ts.map
\ No newline at end of file

@rvagg
Copy link
Member

rvagg commented Jun 6, 2022

@Gozala what are the implications for users of having the CID type changed to CID<number, number, API.CIDVersion> where it's incidental - currently it's in Block (I don't know if Block has any users, I don't tend to recommend it). Does this still carry the "optional generic" thing we discussed above, even though it's parameterised and doesn't have defaults when exposed in this way? Will a user of Block need to start caring about CID parameters and not just use it as a plain CID without those 3 parameters?

I've tried this against a couple of downstream users and it doesn't seem to change anything with current use. @ipld/bitcoin does a lot of work across the multiformats API and it compiles just fine and its own generated are identical with this branch or master, so that's a good sign.

I think the next step downstream is to start using import { asCID, parse as parseCID, create as createCID } from 'multiformats/cid' for use of concrete CID manipulation, and import { CID } from 'multiformats/interface' wherever it's invoked as a type, such as for function arguments and return types.

So I've tried that out @ ipld/js-bitcoin#7, and as I said over there, I come back to the initial question above - what are the implications of these parameterised CIDs for downstream APIs? If I'm now strongly typing a BitcoinBlockCID which always has a stable codec and hasher, do I make it complicated for users who have APIs that just want to accept generic CID objects? What does that look like for them and does it get more verbose?

@Gozala
Copy link
Contributor Author

Gozala commented Jun 10, 2022

@Gozala what are the implications for users of having the CID type changed to CID<number, number, API.CIDVersion> where it's incidental - currently it's in Block (I don't know if Block has any users, I don't tend to recommend it). Does this still carry the "optional generic" thing we discussed above, even though it's parameterised and doesn't have defaults when exposed in this way? Will a user of Block need to start caring about CID parameters and not just use it as a plain CID without those 3 parameters?

So there are couple of things to the block:

  1. Block.create had been updated to include type parameters so that:
    • If you provide both cid and hasher they'd have to align in hashing algorithm. This means it may start complaining if two are misaligned. That said it's probably a not a problem in practice as you probably pass one or the other anyway.
  2. Block instances themself have cid: CID so it carries no information about the codec or hashing algorithm. This may not be ideal if you use e.g. Block.create with specific hasher and encoder and were hoping to get CID with matching type parameters.

If desired I can update Block implementation so it can preserve hasher / encoding info in the CID. I must have neglected that in the PR, mostly because I never use it as I find it pretty useless. Instead I find myself adding another function to BlockCodec interface which looks something lik like:

interface BlockWriter extends  BlockEncoder<Code extends number, T> {
  write <A = number>(data:T, options?:{ hasher?: MultihashHasher<A> }):Await<Block<T, Code, A>>
}

/**
 * Represents an IPLD block (including its CID) that can be decoded to data of type `T`.
 *
 * @template T logical type of the data encoded in the block. This is distinct from the multicodec code of the block's {@link CID}, which is represented by `C`.
 * @template C - multicodec code corresponding to codec used to encode the block
 * @template A - multicodec code corresponding to the hashing algorithm used in creating CID
 */
export interface Block<
  T extends unknown = unknown,
  C extends number = number,
  A extends number = number
  V extends 0|1 = 1
> {
  bytes: ByteView<T>
  cid: Link<T, C, A, V>
}

/**
 * Represents an IPLD link to a specific data of type `T`.
 *
 * @template T logical type of the data being linked to. This is distinct from the multicodec code of the underlying {@link CID}, which is represented by `C`.
 * @template V - CID version
 * @template C - multicodec code corresponding to a codec linked data is encoded with
 * @template A - multicodec code corresponding to the hashing algorithm of the CID
 */

export interface Link<
  T extends unknown = unknown,
  C extends number = number,
  A extends number = number
  V extends 0 | 1 = 1,
> extends CID<C, A, V>,
    Phantom<T> {}

I've tried this against a couple of downstream users and it doesn't seem to change anything with current use. @ipld/bitcoin does a lot of work across the multiformats API and it compiles just fine and its own generated are identical with this branch or master, so that's a good sign.

Thanks for doing this, it really helps! For what it's worth I have been using interface instead of class pretty much in all the new code I worked on which includes

Other than fact that multiformats and that existing codecs expects class instances it had been great. And to work around that I have resorted to annotating return types of things as Link & CID https://github.com/ipld/js-dag-ucan/blob/112ca31646a728287e64e346f5f261e41f44cd35/src/lib.js#L71, which once this lands will become unnecessary.

I think the next step downstream is to start using import { asCID, parse as parseCID, create as createCID } from 'multiformats/cid' for use of concrete CID manipulation, and import { CID } from 'multiformats/interface' wherever it's invoked as a type, such as for function arguments and return types.

👍

So I've tried that out @ ipld/js-bitcoin#7, and as I said over there, I come back to the initial question above - what are the implications of these parameterised CIDs for downstream APIs? If I'm now strongly typing a BitcoinBlockCID which always has a stable codec and hasher, do I make it complicated for users who have APIs that just want to accept generic CID objects? What does that look like for them and does it get more verbose?

I've tried to respond to that here ipld/js-bitcoin#7 (comment) short tldr;

  • If downstream expects generic CID it will work great and code downstream would be able to infer what codec & hashing was used.
  • If downstream is more of a proxy that passes things further downstream which expects something concrete which happens to misalign that would create problems. But I'd argue that is a good think, it raises misalignment. To be completely clear though in this scenario ipld/js-bitcoin has no affect on downstream it's more about if there is a library that takes generic CIDs it can't simply pass them into library that expects concrete CIDs, it needs to check that code / algorithm matches to refine type before passing it on.

@Gozala Gozala force-pushed the feat/cid-interface branch from a98adaa to ec5e3d9 Compare June 10, 2022 17:54
@Gozala Gozala force-pushed the feat/cid-interface branch from 84dad4c to 8e2868f Compare June 13, 2022 15:42
@Gozala
Copy link
Contributor Author

Gozala commented Jun 13, 2022

@rvagg I wanted to address the block module to do the code / alg interfece as discussed earlier, but since 8e2868f I've been fighting ipjs standard and all the other tooling which I can't seem to get to work.

It seems to work on my device but not in CI, which seems to ignore ignorePatterns and standard/ignore in package.json etc..

At this point I really don't know how to stop eslint complaining about lack of named exports. Could you please take a look ? Additionally ipjs also seems to have trouble with types in exports map which is where TS is moving

@Gozala Gozala force-pushed the feat/cid-interface branch 3 times, most recently from 16a20d8 to 6a7dbc6 Compare June 13, 2022 16:57
@Gozala Gozala force-pushed the feat/cid-interface branch from 2c672aa to 7b36f00 Compare June 14, 2022 01:32
@Gozala
Copy link
Contributor Author

Gozala commented Jun 14, 2022

@rvagg I wanted to address the block module to do the code / alg interfece as discussed earlier, but since 8e2868f I've been fighting ipjs standard and all the other tooling which I can't seem to get to work.

It seems to work on my device but not in CI, which seems to ignore ignorePatterns and standard/ignore in package.json etc..

At this point I really don't know how to stop eslint complaining about lack of named exports. Could you please take a look ? Additionally ipjs also seems to have trouble with types in exports map which is where TS is moving

Ok I think I finally managed to get it to work! That said, I find current setup to be really problematic. Every single time I touch this repo I end up in a rabbit hole of trying to make ts, ipjs and eslint get along with each other. In my experience most issues stem from copying source into dist and running things over there instead, which seems to misaligned with flows assumed by other tools.

@Gozala
Copy link
Contributor Author

Gozala commented Jun 14, 2022

@rvagg in the process of trying to fix this I forgot a thing that occurred to me while making these new changes:

It may be worth considering alternative approach to having CID class and an interface with static factory functions. Instead we could just define a Link interface (and associated factory functions), CID class would just implement Link interface and there will be no overlapping names to be confused. Additionally it would:

  1. Map directly with Links in IPLD schemas, &Thing -> Link<Thing>, Link -> Link<any>.
  2. Things could update requirements from CID to Link, which I think would be less confusing than say migrating from CID class to CID interface.

Let me know what you think

@rvagg
Copy link
Member

rvagg commented Jun 27, 2022

@Gozala really sorry for the 2 week delay! not sure how I missed that but I came back here to try and get closure and saw your comment.

re Link - yes, I think this is a neat idea. There is some concern in Go land about the Link - CID relationship - mainly the additional layer of complication in that there is only one form of Link but the whole system is setup for it to be an abstract. So you end up doing casts to the concrete cidlink.Link almost every time you want to do something productive with it.

I don't think that applies here so much, perhaps it would if we pushed it further—the main problem for Link in go-ipld-prime is that it's mostly a non-functional placeholder, so getting to the functionality requires a jump. What we're talking about here involves a more fully-featured interface.

So I think that works, and asLink would be a nice addition to it too.

Let me know if/when you get this changed. I was about to experiment with what this looks like when linked against js-ipfs, just to doubly make sure there's no meaningful breakage of existing code; then we can merge.

@Gozala
Copy link
Contributor Author

Gozala commented Jun 28, 2022

re Link - yes, I think this is a neat idea. There is some concern in Go land about the Link - CID relationship - mainly the additional layer of complication in that there is only one form of Link but the whole system is setup for it to be an abstract. So you end up doing casts to the concrete cidlink.Link almost every time you want to do something productive with it.

That sounds like lack of generics in go showing.

I don't think that applies here so much, perhaps it would if we pushed it further—the main problem for Link in go-ipld-prime is that it's mostly a non-functional placeholder, so getting to the functionality requires a jump. What we're talking about here involves a more fully-featured interface.

I don't think go limitations would apply here

So I think that works, and asLink would be a nice addition to it too.

Do you mean what we've discussed here #185 ?

Let me know if/when you get this changed. I was about to experiment with what this looks like when linked against js-ipfs, just to doubly make sure there's no meaningful breakage of existing code; then we can merge.

I'll create another branch off of this, perhaps tomorrow.

@rvagg
Copy link
Member

rvagg commented Aug 16, 2022

@Gozala I'd really like to get this closed out - how do you want to proceed? Shall we merge this and then add an additional Link interface to implement by the CID interface? Or are you wanting to rework this entirely to have just a Link interface and a concrete CID?

@Gozala
Copy link
Contributor Author

Gozala commented Aug 24, 2022

@Gozala I'd really like to get this closed out - how do you want to proceed? Shall we merge this and then add an additional Link interface to implement by the CID interface? Or are you wanting to rework this entirely to have just a Link interface and a concrete CID?

I would prefer later and have a wip, but then again would prefer former over neither. I think I’ll have some time to work on this while flying to and from dweb camp. How about I try to get it done this week.

@rvagg
Copy link
Member

rvagg commented Sep 6, 2022

#197 and #199

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace CID class with a CID interface
5 participants