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

[Types] CIDs typings incompability #839

Closed
AuHau opened this issue Dec 21, 2020 · 9 comments · Fixed by multiformats/js-cid#139
Closed

[Types] CIDs typings incompability #839

AuHau opened this issue Dec 21, 2020 · 9 comments · Fixed by multiformats/js-cid#139
Labels
kind/bug A bug in existing code (including security flaws)

Comments

@AuHau
Copy link

AuHau commented Dec 21, 2020

CID version 1.1.0 introduced typings generated with aegir which changed the structure of typings as before this release it was placed in /src/ folder. With version 1.1.4 ts compile now fails with:

node_modules/libp2p/dist/src/content-routing.d.ts:86:19 - error TS2307: Cannot find module 'cids/src/' or its corresponding type declarations.

86 type CID = import("cids/src/");
                     ~~~~~~~~~~~


Found 1 error.

With version 1.0.0 of CID the problem is not present.

@AuHau
Copy link
Author

AuHau commented Dec 21, 2020

Hmm in the generated typings I see:

type CID = import("cids/src/");

But the original JSDoc import does not have it: https://github.com/libp2p/js-libp2p/blob/master/src/content-routing.js#L12 😳

Maybe it is related to the version of CID that was used when the typings were released? 🤔

@vasco-santos
Copy link
Member

vasco-santos commented Dec 23, 2020

Thanks for reporting this @AuHau
We had some regressions with the cids. I will need to have a deeper look into this as the cids/src is strange indeed

In theory, this should work as the cids import goes to module "./libp2p/js-libp2p/node_modules/cids/dist/src/index". However, when the build runs it adds the strange /src part. This might be a problem with typescript declaration files generation.

Any ideas @hugomrdias @Gozala ?

@vasco-santos vasco-santos added the kind/bug A bug in existing code (including security flaws) label Dec 23, 2020
@AuHau
Copy link
Author

AuHau commented Dec 23, 2020

Sure thing! Just FYI. downgrading cids to 1.0.2 "solved" the problem...

@hugomrdias
Copy link
Member

@AuHau which ts version are you using?

@vasco-santos
Copy link
Member

@hugomrdias this is one of the cases that the aegir typecheck is not catching errors. If you build libp2p and check /dist/src/content-routing vscode also reports this error. It was not happening before though

@Gozala
Copy link
Contributor

Gozala commented Dec 29, 2020

I'm afraid problem is that TS has a bug that incorrectly resolves module paths microsoft/TypeScript#41284. As to why TS rewrites imports to things like cids/src/ I am pretty clueless, but this has came up before & think the most reasonable solution is to workaround TS issues as per ipfs/aegir#703

@ryanio
Copy link

ryanio commented Jan 7, 2021

I have the same problem here ethereumjs/ethereumjs-monorepo#1027 (comment)

hugomrdias added a commit to multiformats/js-cid that referenced this issue Jan 10, 2021
This PR updates aegir and fixes errors with the new ts config and removes typesVersions.

`typesVersions` workaround makes TS rewrite imports to `cids` to `cids/src` and because this package doesn't really needs this a workaround its just removed.

This should fix libp2p/js-libp2p#839 (comment)
vmx pushed a commit to multiformats/js-cid that referenced this issue Jan 12, 2021
This PR updates aegir and fixes errors with the new ts config and removes typesVersions.

`typesVersions` workaround makes TS rewrite imports to `cids` to `cids/src` and because this package doesn't really needs this a workaround its just removed.

This should fix libp2p/js-libp2p#839 (comment)
@vmx
Copy link
Contributor

vmx commented Jan 13, 2021

@ryanio thanks for pointing to the PR. It helped me to verify that the fix in js-cid actually works. No workarounds for that should be needed if you use cids v1.1.5.

@vasco-santos
Copy link
Member

thanks @vmx and @hugomrdias

@AuHau this should be fixed, feel free to re-open if you have further issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants