-
-
Notifications
You must be signed in to change notification settings - Fork 148
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
Fix type definitions for moduleResolution nodenext #827
Fix type definitions for moduleResolution nodenext #827
Conversation
cb5973d
to
2e95b87
Compare
Hi @trygveaa, thank you! Do you know whether the problem here is new with the v3.0 release, or was the library also incompatible with nodenext in v2.5? |
Yes, the problem was introduced in v3.0.0 when Btw, I see that the tests failed in CI. This is likely because the dependency on the two other PRs as mentioned in the description (though I did have some issues with some of the tests locally, but I see the same issues in main). |
Thanks @trygveaa! I've merged the other PRs and updated their npm versions on the |
When importing this library in a project using moduleResolution nodenext, the types would give errors because the import paths were missing the file extension which is required for moduleResolution nodenext. Therefore we have to add .js to all imports of local files. See developit/microbundle#1019 (comment) and the issues that comment links to for some more detailed explanation of this. I also changed moduleResolution to nodenext in the tsconfig because when it's set to node, typescript won't give errors for missing file extensions which means they are easy to forget. With it set to nodenext, you will get an error if an import is missing the file extension. As far as I can see, changing this doesn't change the build output. Depends on donmccurdy/property-graph#70 and donmccurdy/KTX-Parse#55 Fixes donmccurdy#824
2e95b87
to
57e9584
Compare
Great! I rebased it now. |
Thanks so much for the investigation and working through all the changes here and upstream! If microbundle were to (someday) start allowing us to bundle the |
No problem, thanks for creating this useful library :)
Yes, without any imports in the
Also, having |
When importing this library in a project using moduleResolution nodenext, the types would give errors because the import paths were missing the file extension which is required for moduleResolution nodenext. Therefore we have to add .js to all imports of local files.
See developit/microbundle#1019 (comment) and the issues that comment links to for some more detailed explanation of this.
I also changed moduleResolution to nodenext in the tsconfig because when it's set to node, typescript won't give errors for missing file extensions which means they are easy to forget. With it set to nodenext, you will get an error if an import is missing the file extension. As far as I can see, changing this doesn't change the build output.
Depends on donmccurdy/property-graph#70 and donmccurdy/KTX-Parse#55
Fixes #824