multiformats / js-multiformats

Multiformats interface (multihash, multicodec, multibase and CID)
Other
232 stars 54 forks source link

Generated Typescript types fail to compile, are missing some generic arguments #48

Open andymatuschak opened 4 years ago

andymatuschak commented 4 years ago

Hi, all. The generated .d.ts files are missing some generic arguments, making builds fail when using these type definitions. For instance, in base.d.ts:

declare class ComposedDecoder<Prefix extends string> implements MultibaseDecoder, CombobaseDecoder {

MultibaseDecoder and CombobaseDecoder both require a Prefix generic type argument, but none is supplied.

This is the full set of errors I encountered, but it may not be exhaustive, since I'm not importing all submodules:

../../node_modules/multiformats/dist/types/bases/base.d.ts:16:75 - error TS2314: Generic type 'MultibaseCodec' requires 1 type argument(s).

16 export class Codec<Base extends string, Prefix extends string> implements MultibaseCodec, MultibaseEncoder, MultibaseDecoder, BaseCodec, BaseEncoder, BaseDecoder {
                                                                             ~~~~~~~~~~~~~~

../../node_modules/multiformats/dist/types/bases/base.d.ts:16:91 - error TS2314: Generic type 'MultibaseEncoder' requires 1 type argument(s).

16 export class Codec<Base extends string, Prefix extends string> implements MultibaseCodec, MultibaseEncoder, MultibaseDecoder, BaseCodec, BaseEncoder, BaseDecoder {
                                                                                             ~~~~~~~~~~~~~~~~

../../node_modules/multiformats/dist/types/bases/base.d.ts:16:109 - error TS2314: Generic type 'MultibaseDecoder' requires 1 type argument(s).

16 export class Codec<Base extends string, Prefix extends string> implements MultibaseCodec, MultibaseEncoder, MultibaseDecoder, BaseCodec, BaseEncoder, BaseDecoder {
                                                                                                               ~~~~~~~~~~~~~~~~

../../node_modules/multiformats/dist/types/bases/base.d.ts:94:78 - error TS2314: Generic type 'MultibaseEncoder' requires 1 type argument(s).

94 declare class Encoder<Base extends string, Prefix extends string> implements MultibaseEncoder, BaseEncoder {
                                                                                ~~~~~~~~~~~~~~~~

../../node_modules/multiformats/dist/types/bases/base.d.ts:132:78 - error TS2314: Generic type 'MultibaseDecoder' requires 1 type argument(s).

132 declare class Decoder<Base extends string, Prefix extends string> implements MultibaseDecoder, UnibaseDecoder, BaseDecoder {
                                                                                 ~~~~~~~~~~~~~~~~

../../node_modules/multiformats/dist/types/bases/base.d.ts:132:96 - error TS2314: Generic type 'UnibaseDecoder' requires 1 type argument(s).

132 declare class Decoder<Base extends string, Prefix extends string> implements MultibaseDecoder, UnibaseDecoder, BaseDecoder {
                                                                                                   ~~~~~~~~~~~~~~

../../node_modules/multiformats/dist/types/bases/base.d.ts:166:65 - error TS2314: Generic type 'MultibaseDecoder' requires 1 type argument(s).

166 declare class ComposedDecoder<Prefix extends string> implements MultibaseDecoder, CombobaseDecoder {
                                                                    ~~~~~~~~~~~~~~~~

../../node_modules/multiformats/dist/types/bases/base.d.ts:166:83 - error TS2314: Generic type 'CombobaseDecoder' requires 1 type argument(s).

166 declare class ComposedDecoder<Prefix extends string> implements MultibaseDecoder, CombobaseDecoder {
                                                                                      ~~~~~~~~~~~~~~~~

../../node_modules/multiformats/dist/types/codecs/codec.d.ts:19:78 - error TS2314: Generic type 'BlockEncoder' requires 2 type argument(s).

19 export class Encoder<T, Name extends string, Code extends number> implements BlockEncoder {
                                                                                ~~~~~~~~~~~~

../../node_modules/multiformats/dist/types/codecs/codec.d.ts:41:57 - error TS2314: Generic type 'BlockDecoder' requires 2 type argument(s).

41 export class Decoder<Code extends number, T> implements BlockDecoder {
                                                           ~~~~~~~~~~~~

../../node_modules/multiformats/dist/types/codecs/codec.d.ts:64:76 - error TS2314: Generic type 'BlockCodec' requires 2 type argument(s).

64 export class Codec<Name extends string, Code extends number, T> implements BlockCodec {
                                                                              ~~~~~~~~~~
Gozala commented 4 years ago

@andymatuschak thanks for reporting this. It appears that is caused by a bug in typescript's type declaration generation code https://github.com/microsoft/TypeScript/issues/41258, it seems to loose type parameters for jsdoc @implements comment. E.g last error on line 64 is generated from:

https://github.com/multiformats/js-multiformats/blob/f8428808ff4e1053871ce1cf5d09ed0674790b1b/src/codecs/codec.js#L74-L81

@andymatuschak do you still observe compliation with skipLibCheck enabled ? I have not tested that, but I believe it should not cause problems when that option is enabled.

If skipLibCheck does not solve this problem, we could just remove all the @implements, but I would much rather not so we can ensure implementations don't diverge.

andymatuschak commented 4 years ago

@Gozala Nice work reducing the TypeScript issue—looks like they plan to fix it in 4.2.0, so hopefully this won't be an issue long.

Indeed, setting skipLibCheck does solve the problem for me, so that's a good workaround! I don't want to leave that enabled for my project, since I have some other local type definitions, and I want those to be type checked. But at least others can use that workaround.

Should we put a note about this gotcha and its workaround in the Readme until the issue's fixed? Happy to submit a PR for that if it's welcome; if not, let's just close and move on.

Gozala commented 4 years ago

Should we put a note about this gotcha and its workaround in the Readme until the issue's fixed? Happy to submit a PR for that if it's welcome; if not, let's just close and move on.

That would be amazing thanks!