multiformats / js-multiformats

Multiformats interface (multihash, multicodec, multibase and CID)
Other
222 stars 51 forks source link

BlockEncoder/Decoder/Codec interface generics usage #293

Open tabcat opened 3 weeks ago

tabcat commented 3 weeks ago

https://github.com/multiformats/js-multiformats/blob/dba64626a17f13471db4f23cb8eb9806c28010e4/src/codecs/interface.ts#L9

https://github.com/multiformats/js-multiformats/blob/dba64626a17f13471db4f23cb8eb9806c28010e4/src/codecs/json.ts#L9

Why can the json implementation of encode handle generic types but the BlockEncoder interface's encode cannot?

rvagg commented 3 weeks ago

what am I missing @tabcat ? the T is on the interface declaration BlockEncoder<Code extends number, T> so I think it gets covered on the method, no?

achingbrain commented 3 weeks ago

I think maybe it's that BlockEncoder is constrained to a specific type (like BlockEncoder<5, DagPB>) whereas the JSON codec accepts anything you throw at it?

tabcat commented 3 weeks ago
import { BlockCodec } from "multiformats/interface";
import * as json from "multiformats/codecs/json";

const codec: BlockCodec<typeof json.code, any> = {
  ...json,
};

const a = codec.encode("asdf"); // type is ByteView<any>
const b = json.encode("asdf"); // type is ByteView<string>

The interface methods should be generic like those of the json impl no? Could this be supported? Does seem good to also have the constraint on the types given to the BlockCodec to show what is supported by the codec.

e.g.

export interface BlockEncoder<Code extends number, Universe> {
  encode<T extends Universe>(data: T): ByteView<T>
}
rvagg commented 3 weeks ago

maybe open a PR with a suggestion and we'll see who comes out of the woodwork

tabcat commented 3 weeks ago

Sounds good :+1: . Was wondering if this was intentional/known.