-
-
Notifications
You must be signed in to change notification settings - Fork 222
fix: allow using @types-module for typescript type-only imports #670
Conversation
This is only required to make depcheck pass - we're only importing the types, which are given by `@types/inquirer`, but depcheck doesn't understand this. I created a PR[1] against depcheck to fix this, but it is hard to know when/if this patch will land, so this seems like a reasonable workaround given it is only a development dependency. [1]: depcheck/depcheck#670
This is only required to make depcheck pass - we're only importing the types, which are given by `@types/inquirer`, but depcheck doesn't understand this. I created a PR[1] against depcheck to fix this, but it is hard to know when/if this patch will land, so this seems like a reasonable workaround given it is only a development dependency. [1]: depcheck/depcheck#670
This is only required to make depcheck pass - we're only importing the types, which are given by `@types/inquirer`, but depcheck doesn't understand this. I created a PR[1] against depcheck to fix this, but it is hard to know when/if this patch will land, so this seems like a reasonable workaround given it is only a development dependency. [1]: depcheck/depcheck#670
This is only required to make depcheck pass - we're only importing the types, which are given by `@types/inquirer`, but depcheck doesn't understand this. I created a PR[1] against depcheck to fix this, but it is hard to know when/if this patch will land, so this seems like a reasonable workaround given it is only a development dependency. [1]: depcheck/depcheck#670
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like an easy enough fix 👍
This is only required to make depcheck pass - we're only importing the types, which are given by `@types/inquirer`, but depcheck doesn't understand this. I created a PR[1] against depcheck to fix this, but it is hard to know when/if this patch will land, so this seems like a reasonable workaround given it is only a development dependency. [1]: depcheck/depcheck#670
This is only required to make depcheck pass - we're only importing the types, which are given by `@types/inquirer`, but depcheck doesn't understand this. I created a PR[1] against depcheck to fix this, but it is hard to know when/if this patch will land, so this seems like a reasonable workaround given it is only a development dependency. [1]: depcheck/depcheck#670
This is only required to make depcheck pass - we're only importing the types, which are given by `@types/inquirer`, but depcheck doesn't understand this. I created a PR[1] against depcheck to fix this, but it is hard to know when/if this patch will land, so this seems like a reasonable workaround given it is only a development dependency. [1]: depcheck/depcheck#670
This is only required to make depcheck pass - we're only importing the types, which are given by `@types/inquirer`, but depcheck doesn't understand this. I created a PR[1] against depcheck to fix this, but it is hard to know when/if this patch will land, so this seems like a reasonable workaround given it is only a development dependency. [1]: depcheck/depcheck#670
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good to me, sorry for the late review...
@rexxars thanks for the PR, could you amend your commit and force push to kickstart the CI please, I have no idea why I can run the actions on your PR... |
3ae07d3
to
8643cd3
Compare
Sure. Rebased. |
Codecov Report
@@ Coverage Diff @@
## main #670 +/- ##
==========================================
- Coverage 98.64% 98.45% -0.18%
==========================================
Files 49 49
Lines 1023 1031 +8
==========================================
+ Hits 1009 1015 +6
- Misses 14 16 +2
Continue to review full report at Codecov.
|
Thanks @rexxars |
Thanks for merging! Any chance of seeing this one in a release anytime soon? |
Fixes #568
Fixes #526
I'm not sure if this is the best approach, given none of the other detectors seems to be using the second
deps
argument, but the alternative would be to change the returned value from an array of strings to an object that includes dependency names and an additional property for the type of import, or similar.