-
-
Notifications
You must be signed in to change notification settings - Fork 222
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
output line in package.json file #874
base: main
Are you sure you want to change the base?
Conversation
ca7005e
to
4c3b564
Compare
Quick question, in terminals that do know how to parse this, is it cmd/ctrl clickable so that the file opens on that line in an IDE? |
Tried in VSCode, it does seems to work great (see video). 2024-01-11.15-25-13.mp4 |
No other in mind no, ok this is great, thanks, now I have to review the code :) |
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.
Great ! I commented on github directly to help you understand the code if needed.
rootDir = null, | ||
printJsonLineInKey = null, |
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.
I added two parameters here:
rootDir
to open thepackage.json
file,printJsonLineInKey
is to find the extra dependency in either "dependencies" or "devDependencies"
const packageJson = | ||
rootDir && printJsonLineInKey | ||
? fs.readFileSync(path.join(rootDir, 'package.json'), 'utf8') | ||
: null; |
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.
Open the package.json if needed (only on default output)
May be optimised to avoid memory consumption with a lot of unused deps (but the purpose of depcheck is to have a few unused deps, so extra optimization may be overkill ?)
findLineInFile(packageJson, printJsonLineInKey, dep); | ||
|
||
return `* ${dep}${ | ||
lineNumber !== null ? ` (package.json:${lineNumber})` : '' |
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.
Add the line number if needed
Hi! Do you need any help from me to get this done? |
Add the dependency line number in the standard output. See #872 for more about this PR.
I choose to modify only the cli output for now as it does not break public API nor internal representation.
To do so, I look at the package.json file pretty late in the process. It might be nice to get the line number earlier in the process, but it might change the result object, which may be more intrusive.
Fixes #872