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.

Sign up for GitHub

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

Jump to bottom

ipfs-unixfs-importer: import type declaration imports UnixFS from the wrong location #214

Closed
mistermoe opened this issue May 18, 2022 · 8 comments · Fixed by #219 or #251
Closed

ipfs-unixfs-importer: import type declaration imports UnixFS from the wrong location #214

mistermoe opened this issue May 18, 2022 · 8 comments · Fixed by #219 or #251
Labels
need/triage Needs initial labeling and prioritization released

Comments

@mistermoe
Copy link

Summary

ipfs-unixfs-importer: import type declaration imports UnixFS from the wrong location

Impact

unable to use ipfs-unixfs-importer package in typescript projects. attempting to transpile typescript project using tsc fails with the following error:

> tsc

node_modules/ipfs-unixfs-importer/types/src/index.d.ts:26:20 - error TS2307: Cannot find module 'ipfs-unixfs/src' or its corresponding type declarations.

26     unixfs: import("ipfs-unixfs/src").UnixFS | undefined;
                      ~~~~~~~~~~~~~~~~~


Found 1 error.

Potential Cause

  • typesVersions in package.json of ipfs-unixfs has a path mapping that checks both types/* and types/src/*. Removing typesVersions entirely and regenerating the type declarations yields unixfs: import("ipfs-unixfs").UnixFS | undefined which solves the problem because typescript resolves the type using the types property in package.json. Though, i'm not sure if this is the way we want to solve the problem. I'm not sure whether downstream projects are importing types that aren't exported from index.d.ts. I think ideally, using something like npm-dts to bundle all types into 1 index.d.ts that is made available at the root of the package tarball would remove a lot of complexity around configuring types

Somewhat related

  • type declaration files are duplicated in the package
    image
  • I think that the reset npm script needs to delete the types directory in every package because it's auto-generated. not deleting them prevents types from being regenerated
@mistermoe mistermoe added the need/triage Needs initial labeling and prioritization label May 18, 2022
@mistermoe
Copy link
Author

@achingbrain Let me know if there's anything that i can help provide clarity on.

achingbrain added a commit that referenced this issue May 27, 2022
@achingbrain
fix: remove typesVersions from package.json
9f11be8 
We have `typesVersions` to allow deep imports from these modules, but
they cause incorrect paths to be detected by tsc (see #214).

Since #161 we control exports using the exports map, and we only export
the root from each module so `typesVersions` isn't necessary.

Since it's causing problems and we don't need it, remove it.  We can
revisit once we publish ESM-only but right now this fixes up the types
tsc generates.

Fixes #214
@achingbrain achingbrain mentioned this issue May 27, 2022
Merged
achingbrain added a commit that referenced this issue May 27, 2022
@achingbrain
fix: remove typesVersions from package.json (#219)
465670e 
We have `typesVersions` to allow deep requires from these modules, but
they cause incorrect paths to be detected by tsc (see #214).

Since #161 we control exports using the exports map, and we only export
the root from each module so deep requires are disallowed and consequently
`typesVersions` isn't necessary.

Since it's causing problems and we don't need it, remove it.  We can
revisit once we publish ESM-only but right now this fixes up the types
tsc generates.

Fixes #214
github-actions bot pushed a commit that referenced this issue May 27, 2022
@semantic-release-bot
chore(release): 6.0.9 [skip ci]
b280c1d 
## [ipfs-unixfs-v6.0.9](ipfs-unixfs-v6.0.8...ipfs-unixfs-v6.0.9) (2022-05-27)

### Bug Fixes

* remove typesVersions from package.json ([#219](#219)) ([465670e](465670e)), closes [#214](#214) [#161](#161) [#214](#214)

### Trivial Changes

* update dep-check command ([#221](#221)) ([5802bd3](5802bd3))
@github-actions
Copy link

🎉 This issue has been resolved in version ipfs-unixfs-v6.0.9 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions bot pushed a commit that referenced this issue May 27, 2022
@semantic-release-bot
chore(release): 9.0.10 [skip ci]
97bdb9c 
## [ipfs-unixfs-importer-v9.0.10](ipfs-unixfs-importer-v9.0.9...ipfs-unixfs-importer-v9.0.10) (2022-05-27)

### Bug Fixes

* remove typesVersions from package.json ([#219](#219)) ([465670e](465670e)), closes [#214](#214) [#161](#161) [#214](#214)

### Trivial Changes

* update dep-check command ([#221](#221)) ([5802bd3](5802bd3))
@github-actions
Copy link

🎉 This issue has been resolved in version ipfs-unixfs-importer-v9.0.10 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions bot pushed a commit that referenced this issue May 27, 2022
@semantic-release-bot
chore(release): 7.0.11 [skip ci]
dd6253f 
## [ipfs-unixfs-exporter-v7.0.11](ipfs-unixfs-exporter-v7.0.10...ipfs-unixfs-exporter-v7.0.11) (2022-05-27)

### Bug Fixes

* remove typesVersions from package.json ([#219](#219)) ([465670e](465670e)), closes [#214](#214) [#161](#161) [#214](#214)

### Trivial Changes

* update dep-check command ([#221](#221)) ([5802bd3](5802bd3))
@github-actions
Copy link

🎉 This issue has been resolved in version ipfs-unixfs-exporter-v7.0.11 🎉

The release is available on:

Your semantic-release bot 📦🚀

@mhchu-multiverse
Copy link

It looks like this was resolved by removing typesVersions from package.json in commit 465670e but was reintroduced in commit 9fccb7c. Between these two, there have been no published version, so there is currently no version I have found using ipfs-unixfs-importer which compiles with tsc.

@achingbrain
Copy link
Member

tsc keeps choosing the wrong type when inferring return types. I think the trick is to specify return types on exported functions, then you get what you expect, whereas when you let the compiler figure it out you frequently get weird frankentypes.

@achingbrain achingbrain reopened this Aug 30, 2022
achingbrain added a commit that referenced this issue Aug 30, 2022
@achingbrain
fix: specify return type of the importer to generate correct types
c282ccc 
If we don't to this, we end up importing deep paths from `unixfs` in
the generated types which aren't in the exports map so fail when
people are compiling their dependencies.

Fixes #214
@achingbrain achingbrain mentioned this issue Aug 30, 2022
Merged
achingbrain added a commit that referenced this issue Aug 31, 2022
@achingbrain
fix: specify return type of the importer to generate correct types (#251
3343366 
)

If we don't to this, we end up importing deep paths from `unixfs` in the generated types which aren't in the exports map so fail when people are compiling their dependencies.

Fixes #214
github-actions bot pushed a commit that referenced this issue Aug 31, 2022
@semantic-release-bot
chore(release): 10.0.2 [skip ci]
e409fc4 
## [ipfs-unixfs-importer-v10.0.2](ipfs-unixfs-importer-v10.0.1...ipfs-unixfs-importer-v10.0.2) (2022-08-31)

### Bug Fixes

* specify return type of the importer to generate correct types ([#251](#251)) ([3343366](3343366)), closes [#214](#214)
@github-actions
Copy link

🎉 This issue has been resolved in version ipfs-unixfs-importer-v10.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions bot pushed a commit that referenced this issue Aug 31, 2022
@semantic-release-bot
chore(release): 8.0.5 [skip ci]
99a830d 
## [ipfs-unixfs-exporter-v8.0.5](ipfs-unixfs-exporter-v8.0.4...ipfs-unixfs-exporter-v8.0.5) (2022-08-31)

### Bug Fixes

* specify return type of the importer to generate correct types ([#251](#251)) ([3343366](3343366)), closes [#214](#214)
@github-actions
Copy link

🎉 This issue has been resolved in version ipfs-unixfs-exporter-v8.0.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
need/triage Needs initial labeling and prioritization released
Projects
None yet
Milestone
No milestone
3 participants
@achingbrain @mistermoe @mhchu-multiverse