yeoman / doctor

Detect potential issues with users system that could prevent Yeoman from working correctly
http://yeoman.io
BSD 2-Clause "Simplified" License
61 stars 17 forks source link

Error when checking NODE_PATH #50

Open BraveThrasher opened 4 years ago

BraveThrasher commented 4 years ago

Today I installed yeoman and yeoman doctor flagged an issue with "NODE_PATH matches the npm root". In this Google did not appear to be my friend so I started investigating myself. I found yeoman-doctor/lib/rules/node-path.js and started debugging the old way by adding console.log lines. In the exported function verify I added a console.log for the error in the catch. The error message read "TypeError: filepath.trim is not a function".

In the complete script file there is only one line with a trim function and that is in the function fixPath. That function is being called on 2 places, for each of the paths in the NODE_PATH environment value and once for the output of the command npm -g root --silent so I added a console.log line to the fixPath function to see the incoming value of filepath. The output of npm -g root --silent appeared to contain a new line character all the rest looked normal so I suspected that new line to be the culprit. So I tried to replace it with filepath.replace(/^\n|\n$/g, '') but it only made the error being thrown by the replace function instead. At this point I wondered how this could be, a quick Google and the wisdom of "don't trust your input" got me the idea of adding toString() to the filepath variable before the trim function. That did the trick, the error went away and "NODE_PATH matches the npm root" was being flagged as okay.

I could create a pull request for my fix but first I want to make sure if I'm not missing anything obvious.

mshima commented 4 years ago

Can you create the PR? But probably it should go at: https://github.com/yeoman/doctor/blob/df93f8a73e1654f5f91414bd5e5d923b15f7812f/lib/rules/node-path.js#L51

What os are you running?