Skip to content
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

Error when checking NODE_PATH #50

Open
BraveThrasher opened this issue Oct 28, 2019 · 1 comment
Open

Error when checking NODE_PATH #50

BraveThrasher opened this issue Oct 28, 2019 · 1 comment

Comments

@BraveThrasher
Copy link

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
Copy link
Member

mshima commented Jan 31, 2020

Can you create the PR?
But probably it should go at:

const stdout = childProcess.execSync('npm -g root --silent');

What os are you running?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants