-
Notifications
You must be signed in to change notification settings - Fork 54
Compilation fails on Cannot find name 'T' #116
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
Comments
The 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: You can see where we get more specific than 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 |
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 The reason why the typescript is complaining is that that 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. |
the .d.ts files are generated by @Gozala can you weigh in on this one please? |
your build uses I understand the need for innovation and all but seriously, you are building the library that should be a |
and on a really scary note, there is no lock file for the dependencies. |
Plus the devDependencies have a deprecated package |
@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:
|
@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. |
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. |
The issue here is caused by a bug in typescript compiler microsoft/TypeScript#41258. As you can see from the source code type parameter is there: js-multiformats/src/codecs/json.js Lines 9 to 18 in 6915e02
But in generated Good news is TS bug appears fixed, if so resolving this issue would just require updating @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 |
It took a bit more than updating TS unfortunately, it's this pattern of exporting the whole |
Hopefully fixed in v9.4.7 https://unpkg.com/browse/multiformats@9.4.6/types/codecs/json.d.ts thanks for reporting the issue and bearing with us. |
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:
Actual Code:
The text was updated successfully, but these errors were encountered: