jorgebucaran / nvm.fish

The Node.js version manager you'll adore, crafted just for Fish
https://git.io/nvm.fish
MIT License
2.13k stars 71 forks source link

Modify path for the current session only #101

Closed andreiborisov closed 3 years ago

andreiborisov commented 4 years ago

đź‘‹

What's the problem?

Proposed solution

Are you willing to review and accept a PR for this?

Related: https://github.com/jorgebucaran/fish-nvm/issues/93

yegorius commented 4 years ago

This has already been discussed in #39. Please see my last comment there. I hope @jorgebucaran will include proposed changes into official version, as this feature has been requested multiple times.

jorgebucaran commented 4 years ago

@yegorius @schrodincat Maybe you folks can help me test @schrodincat's proposal by writing a list of pros and cons for that approach. 🙏

yegorius commented 4 years ago

In my case I am almost always satisfied with my distro's default package of node (Arch Linux is rolling release distro so I get the latest official version of node in a matter of days), except for the special cases when some project requires an older version. In this case I temporarily switch with nvm in particular fish session while working on this project and I don't need the switch to be global or persistent.

ujwal-setlur commented 4 years ago

I need to different node versions in different shells...have older projects that work on node 10 only :-(

jorgebucaran commented 3 years ago

We could check for an .nvmrc file whenever PWD changes, and switch versions for that specific session only. But how do we persist that version when your Node program spawns a child process?

@yegorius @andreiborisov @ujwal-setlur @jackwestmoretab @thernstig

andreiborisov commented 3 years ago

But how do we persist that version when your Node program spawns a child process?

Modify the global PATH instead of universal fish_user_paths. PATH is exported and should be inherited by child node processes.

jorgebucaran commented 3 years ago

Of course! Thanks, @andreiborisov!

thernstig commented 3 years ago

@jorgebucaran I definitely agree to modifying PATH for the current session only. I think that still having a possibility to modify "the global node version" is a good idea. Using/finding an .nvmrc in the directory tree would take precedence over the global one. This means you can easily walk in/out of project dirs but when outside them you still have some globally configured node version one can use e.g. for one-off npx install.

Even with that solution (allowing both "local" and "global" versions), I still have a slight problem with fish_user_paths. Reason being the way it works via specifying the full path to a path. This is bad for everyone using dotfiles, syncing their config/dotfiles across machines. The solution to this might however lie upon fish itself, see https://github.com/fish-shell/fish-shell/issues/6641. But if it can be solved in this project as well that would be great. One way to solve it even for this project is to alter PATH as already suggested here for the project-local logic. In other words PATH would contain the path to the global configured node version, but when entering a directory it prepends the local, project-specific node version looking at .nvmrc.

But is the idea of automatically doing this by default when finding .nvmrc when PWD changes great? Some of us use tools like https://direnv.net/ which takes care of this for us for much more than just Node, so for us it could be problematic if you auto-switch.

jorgebucaran commented 3 years ago

Yes, and that's why we should only touch $PATH. I fully agree about avoiding $fish_user_paths.

jorgebucaran commented 3 years ago

Automatically switching versions when changing PWD should be easy to implement in userland (or perhaps even configurable—let's see), but never a default.

jorgebucaran commented 3 years ago

Hey! I just released 2.0. Now we modify your PATH only for the current session, thus installing (or activating) a Node version only changes your local environment.

See the README for documentation. đź‘‹

@yegorius @ujwal-setlur @andreiborisov @thernstig