-
-
Notifications
You must be signed in to change notification settings - Fork 222
Tweak Parser typescript definition #718
Conversation
Allow string array return, use `Record` instead of mapped type.
Based on the code, file content is not passed into parsers.
Codecov Report
@@ Coverage Diff @@
## main #718 +/- ##
==========================================
- 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.
|
Hm ok, so win CI is failing with a nice error message
I'm quite sure it's not related to your changes. |
|
||
type Parser = (content: string, filePath: string, deps: ReadonlyArray<string>, rootDir: string) => Node; | ||
type Parser = (filePath: string, deps: ReadonlyArray<string>, rootDir: string) => Node | string[]; |
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.
Not sure I understand this change, the parser doesn't have the content
parameter? Seems weird to change the type definition and not the code at the same time
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.
Well, the function is called here:
https://github.com/depcheck/depcheck/blob/main/src/check.js#L51
And it only passes in the latter three parameters. I guess these types have just been incorrect since this change:
The implementation was updated there but not the types.
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.
Oh! Ok 😅
Ok so the win build is just broken for some obscure Visual Studio reason, I'll just merge this one and take a look at the windows build error... |
Hi, is there any plan to publish this in a release? |
Hi @jtbandes I'll try and push one this week 👍 |
Record
instead of mapped type.content
parameter - this doesn't seem to ever be passed in