feat: add noTypeDefinitions option #130
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi there 👋
first of all: Thank you for this useful project! 😄
I use a lot of TypeScript and I've found a small inconvenience when using your project with it. Filing cabinet has two ways of resolving TS imports, the first one - the one that is used by default - resolves these to the type definitions. The other one resolves to the JavaScript implementations instead.
So why should you be able to resolve to the
.js
files and not to thed.ts
files?The problem with just resolving to the type definitions is, that definition files don't usually contain imports of all dependencies, so we're missing transitive dependencies here.
Picture this scenario: You're using some dependency A which is written in JS, but provides TS definitions. A uses B as its dependency. When we resolve A to its definition file, that file doesn't include the import for B, so we're missing B in the dependency tree. If we resolve to the JS file instead, this doesn't happen.
So this is what I implemented here. I've add the
noDefinitionFiles
option from filing cabinet to this packages' options. With this option, it is possible to toggle between both behaviors. To keep the code compatible with older versions, the default is the same as before - resolve to definition files.I've added test cases for this as well. 🎉
Also, since I want to use this with TypeScript, I've added type definitions to your package. They should be compatible with the ones provided by DefinitelyTyped and reflect the new changes, but feel free to revert this commit if you don't want to add types at this point. All changes related to the addition of types are contained in 149151f, so you're able to discard these more easily. I'd also be happy to create a second PR for the type definitions, if that's how you'd like to handle it.