jonschlinkert / global-prefix

Get the npm global path prefix. Same code used internally by npm.
MIT License
28 stars 12 forks source link

Fix path to global node_modules on Windows #5

Closed rmbaad closed 8 years ago

rmbaad commented 8 years ago

process.execPath return path to instlled nodejs on Windows, but not to global node_modules directory. E.g. package npmls use global-prefix for getting path to node_modules, but it's returned wrong path.

jorrit commented 8 years ago

I think you should squash the commits.

jonschlinkert commented 8 years ago

@jorrit I can squash the commits on merge, no problem. However, @kuksikus can you share any additional insight into when APPDATA would be used for the prefix this way?

Since npm itself seems to only use APPDATA or user home for cache, and I can't find any code in npm's codebase that falls back to the path you've defined, I just want to make sure we're correctly setting the variable.

thanks!

rmbaad commented 8 years ago

@jonschlinkert I found the problem with path on Windows, and found this path in process.env while debugging.

A quote from the npm wiki:

By default, npm is installed alongside node in C:\Program Files (x86)\nodejs. npm's globally installed packages (including, potentially, npm itself) are stored separately in a user-specific directory (which is currently C:\Users\AppData\Roaming\npm). Because the installer puts C:\Program Files (x86)\nodejs before C:\Users\AppData\Roaming\npm on your PATH, it will always use version of npm installed with node instead of the version of npm you installed using npm -g install npm@

If I try "npm ls -g" I see "C:\Users\jibon\AppData\Roaming\npm" and list packages. And command "npm config ls -l" return

; builtin config undefined prefix = "C:\Users\jibon\AppData\Roaming\npm"

jonschlinkert commented 8 years ago

thanks for the detail! I'll merge sometime today

tadatuta commented 8 years ago

:+1:

rmbaad commented 8 years ago

I squashed commits

jorrit commented 8 years ago

Isn't it an idea to just read the output of npm config get prefix?

[edit] When scripts are run from npm, the npm_config_prefix env variable is also present. That could also be used before executing npm.

jonschlinkert commented 8 years ago

Isn't it an idea to just read the output of npm config get prefix?

yeah I was thinking of doing something like that to get custom prefixes.

rmbaad commented 8 years ago

I think it's a bad idea, beacause npm config get is slow. Just try time npm config get prefix

jorrit commented 8 years ago

I think it should only be executed once and cached in global-prefix from that moment on.

jonschlinkert commented 8 years ago

I think it's a bad idea, beacause npm config get is slow. Just try time npm config get prefix

that's why I haven't done it yet lol, I mentioned how slow it is somewhere else. I'm very open to suggestions. However, it's also ridden with edge cases apparently. Any suggestions?

jonschlinkert commented 8 years ago

I don't think caching it is a good idea if you consider some common use cases for custom prefixes

jorrit commented 8 years ago

You mean that the prefix changes while the application is running?

jonschlinkert commented 8 years ago

You mean that the prefix changes while the application is running?

yeah you're probably right. I'm overcomplicating it.

fwiw I almost always cache file paths anyway:

the closest lib I've found to what I'd like to use is https://www.npmjs.com/package/rc. seems like a nice lib, but it seems like swatting a fly with a hammer in this case. Preferably someone knows of a lib that simply resolves custom prefixes. If not I might create one

rmbaad commented 8 years ago

I think custom prefixes it's a different story. AFAIK current lib doesn't work with default prefixes on Windows. Maybe solve this problem at first? :)

jorrit commented 8 years ago

@kuksikus I think you're right. The current patch greatly improves the current situation.

jonschlinkert commented 8 years ago

sorry for the delay. thanks @kuksikus, published and released as 0.1.3