Compilation fails on Cannot find name 'T'

[woss]

It's obvious that the T should not be there since it is unknown and generic. this should be typed differently. Anyone else knows how to make this library actually usable when used with Typescript?

Error:

[typescript] Using TypeScript version 4.2.4
[eslint] Using ESLint version 7.32.0
[typescript] Encountered 4 TypeScript issues:
[typescript] Error: ../../common/temp/node_modules/.pnpm/multiformats@9.4.6/node_modules/multiformats/types/codecs/json.d.ts:3:29 - (TS2304) Cannot find name 'T'.
[typescript] Error: ../../common/temp/node_modules/.pnpm/multiformats@9.4.6/node_modules/multiformats/types/codecs/json.d.ts:3:66 - (TS2304) Cannot find name 'T'.
[typescript] Error: ../../common/temp/node_modules/.pnpm/multiformats@9.4.6/node_modules/multiformats/types/codecs/json.d.ts:4:61 - (TS2304) Cannot find name 'T'.
[typescript] Error: ../../common/temp/node_modules/.pnpm/multiformats@9.4.6/node_modules/multiformats/types/codecs/json.d.ts:4:68 - (TS2304) Cannot find name 'T'.

Actual Code:

export const name: string;
export const code: 512;
export const encode: (data: T) => import("./interface").ByteView<T>;
export const decode: (bytes: import("./interface").ByteView<T>) => T;
export type BlockCodec<Code extends number, T_1> = import('./interface').BlockCodec<Code, T_1>;
//# sourceMappingURL=json.d.ts.map

[rvagg]

The T is intended to carry over the original input type, e.g. PBNode for dag-pb.

The export here is roughly the same as for our other codecs that build on the same interfaces, including dag-json: https://unpkg.com/browse/@ipld/dag-json@8.0.0/types/index.d.ts, which is actively tested as being consumable as a TypeScript import: https://github.com/ipld/js-dag-json/blob/master/test/ts-use/src/main.ts

Can you just declare the codec like so: codec: BlockCodec<512, any>?

You can see where we get more specific than any in dag-pb: https://unpkg.com/browse/@ipld/dag-pb@2.1.9/types/src/index.d.ts

dag-jose is another codec that gets a bit specific, but there are many others that don't just accept any object that conforms to the IPLD data model. Hence the T.

[woss]

wouldn't it be better to actually write this in pure TS and then compile it properly? JSDoc seems to do a bad job with generics. Providing the .d.ts files manually is not going to be easy to maintain and expectations of this project are huge and a lot of people will be switching from deprecated ones.

The reason why the typescript is complaining is that that .d.ts is not valid, since T is not considered a generic rather than a type that doesn't exist.

Here is the sample code with commonjs generic

export function withGeneric<T>(param: T): void {
  console.log(param)
}

and compiled commonjs:

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.withGeneric = void 0;
function withGeneric(param) {
    console.log(param);
}
exports.withGeneric = withGeneric;

// and gen.d.ts
export declare function withGeneric<T>(param: T): void;
//# sourceMappingURL=gen.d.ts.map

and esm

export function withGeneric(param) {
    console.log(param);
}

// gen.d.ts
export declare function withGeneric<T>(param: T): void;

I hope you see the difference.

[rvagg]

the .d.ts files are generated by tsc --build, not manually

@Gozala can you weigh in on this one please?

[woss]

your build uses ipjs? why another builder? why not stick with standard builders and battle-tested approaches?

I understand the need for innovation and all but seriously, you are building the library that should be a defacto standard for any IPFS/multiformats operations, you should not use the builders for the sake of promoting group of people.

[woss]

and on a really scary note, there is no lock file for the dependencies.

[woss]

Plus the devDependencies have a deprecated package "cids": "^1.1.6", https://github.com/multiformats/js-cid#readme

[rvagg]

@woss your critique is bordering on unhelpful and rude, please try and keep it constructive and try and stay focused on the issue at hand.

Regarding your comments unrelated to the original issue:

• ipjs doesn't have anything to do with the typescript generation, it helps us bundle for maximum cjs and esm and it's developed by a number of us to get the right outcomes.
• package-lock.json is up to you as a user, it's standard for library developers to not use them but where you are concerned about dependency creep as an application developer then that's where it's appropriate
• cids is included to check compatibility with an older version

[woss]

@rvagg it was aimed to be direct, not rude. You could also take it in a way that there are people out there who are very irritated by the libraries that are so essential and that they ( including myself ) need to do so much just to make our code work with yours.

I want to help, that's all.

[woss]

If you guys think you don't need help, just say so, I'll fork this repo, rewrite it in TS as it should be and continue on my own.

[Gozala]

The issue here is caused by a bug in typescript compiler https://github.com/microsoft/TypeScript/issues/41258. As you can see from the source code type parameter is there:

https://github.com/multiformats/js-multiformats/blob/6915e0262c34ab6516423cde86146ccebdc0d50b/src/codecs/json.js#L9-L18

But in generated json.d.ts file T type parameter is no longer present.

Good news is TS bug appears fixed, if so resolving this issue would just require updating typescirpt version and regenerating .d.ts files.

@woss If you would like to help trying above suggestion would be my invite ?

Rewriting libraries in TS had been considered in the past and after evaluating pros and cons it was decided that doing TS via JSDOC syntax was the best compromise which is what this and other libs in IPFS ecosystem are doing today. While it may make sense to reevaluate pros and cons, not much has changed since last reevaluation and there are lot of more pressing issues to deal today. We still have have libraries that have not been entyped fully and bunch of @ts-ignores in the code base, I think help addressing those would be more productive use of everyones time who wishes for a better typescript support.

[rvagg]

It took a bit more than updating TS unfortunately, it's this pattern of exporting the whole BlockCodec<T> (that we previously tried but reverted on js-dag-json and js-dag-cbor) that seems to be the problem, so in this we're just exporting the individual constituent pieces and it seems to work. But I'd appreciate a review: https://github.com/multiformats/js-multiformats/pull/117

[rvagg]

Hopefully fixed in v9.4.7

https://unpkg.com/browse/multiformats@9.4.6/types/codecs/json.d.ts vs https://unpkg.com/browse/multiformats@9.4.7/types/codecs/json.d.ts

thanks for reporting the issue and bearing with us.