Check detected default npm path is really existing

[rhysd]

Fixes #23924

[mhegazy]

@sheetalkamat and @Andy-MS can you please review.

[sheetalkamat]

Why do we look in "node" folder at all? Why not always use it as "npm" so that we use default npm location and users wanting to use custom npm location can use the --npmLocation

[mhegazy]

Why do we look in "node" folder at all?

VS does not have npm installed globally, it is installed in a local folder, next to node.

Why not always use it as "npm" so that we use default npm location and users wanting to use custom npm location can use the --npmLocation

we could, but that would break existing users of the server. and back then we did not have the npm location flag.

[sheetalkamat]

Actually do we need to check it as fileExists or Any type of entry is ok?

[rhysd]

You mean we should also check the file has executable permission? Maybe. But I think there is no case where npm file is not executable (e.g. directory) in the same directory as node command.

[mhegazy]

The check checks for the existence of a file called npm which never exists, it is an executable file though, so either npm.exe or npm.cmd or npm.bat.. so this ends up breaking VS.

Reverted in #24425.

We need a better fix for the original issue.

[rhysd]

@mhegazy ok, so should we check one of npm, npm.exe, npm.cmd or npm.bat exists?

[mhegazy]

I think we should list the directory and see if an npm file exist.

[rhysd]

@mhegazy I see. Actually I'm not a Windows user. So I don't know the list of possible installation directories for npm command...

[mhegazy]

this is not a windows thing. it is a VS thing. VS ships node and npm in-box but in some custom folders. the only thing we know is that there is an npm (likely npm.bat) next to node.exe that is used to run the server.