sorin-ionescu / prezto

The configuration framework for Zsh
MIT License
13.96k stars 4.49k forks source link

NVM (node module) not honoring default node version #1956

Open yorch opened 3 years ago

yorch commented 3 years ago

Description

Since https://github.com/sorin-ionescu/prezto/commit/af46875c5e2c7088b25d7d16e50b6955926b44a4, node module no longer honor nvm default NodeJS version.

Expected behavior

node module should honor nvm default version when opening a new terminal / session.

Actual behavior

NodeJS version in new opened terminal sessions is always system.

Steps to Reproduce

  1. Install nvm (curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.38.0/install.sh | bash).
  2. Install a couple different versions of Node (nvm install 12, nvm install 14)
  3. Install NodeJS using package manager (ie: apt install nodejs)
  4. Enable node zprezto module in .zpreztorc
  5. Set one of the nvm NodeJS versions as default nvm alias default 12
  6. Open a new session and check the currently selected Node version nvm ls, it will be system instead of the previously setup default version.

Already tested nvm in isolation outside of zprezto and works as expected.

Versions

yorch commented 3 years ago

So find that the module is calling nvm with --no-use option (https://github.com/sorin-ionescu/prezto/blob/master/modules/node/init.zsh#L26), which basically forces you to always have to call nvm use node when you open a new session.

Is there is any reason behind this behaviour? or was an intended change?

belak commented 3 years ago

I think the goal was to make nvm loading lazy - it's pretty slow to start up without --no-use. @indrajitr could you weigh in?

kltdwrds commented 2 years ago

Bump - this is still an issue for me.

kjs3 commented 2 years ago

This bit me on a new install and took awhile to figure out where the issue was.

I think it's worth exploring how to make nvm/node load faster but I'd revert the --no-use in the meantime.

srijanshetty commented 2 years ago

Orthogonally, I would suggest using fnm as it lightyears ahead of nvm in loading times and is API compatible

malixsys commented 1 year ago

I put this at the end of .zshrc:

autoload -U add-zsh-hook
load-nvmrc() {
  local node_version="$(nvm version)"
  local nvmrc_path="$(nvm_find_nvmrc)"

  if [ -n "$nvmrc_path" ]; then
    local nvmrc_node_version=$(nvm version "$(cat "${nvmrc_path}")")

    if [ "$nvmrc_node_version" = "N/A" ]; then
      nvm install
    elif [ "$nvmrc_node_version" != "$node_version" ]; then
      nvm use
    fi
  elif [ "$node_version" != "$(nvm version default)" ]; then
    echo "Reverting to nvm default version"
    nvm use default
  fi
}
add-zsh-hook chpwd load-nvmrc
load-nvmrc

Update: Please see https://github.com/sorin-ionescu/prezto/issues/1956#issuecomment-1526785615 below - @indrajitr

indrajitr commented 1 year ago

I think the goal was to make nvm loading lazy - it's pretty slow to start up without --no-use. @indrajitr could you weigh in?

This pretty much sums up the reason behind the change. This was part of a series of changes back then to optimize startup time as much as possible. While I don't have the benchmark data available at this moment, for this specific change in isolation the overall reduction in startup time was significant.

I agree the change is an irritant and did consider adding an inline warning to alert users. But I couldn't figure out a way to do it in a way without nagging users every time a new shell is opened.

Suggestions, or even better, a PR is very welcome.

indrajitr commented 1 year ago

I put this at the end of .zshrc:

autoload -U add-zsh-hook
load-nvmrc() {
  local node_version="$(nvm version)"
  local nvmrc_path="$(nvm_find_nvmrc)"

  if [ -n "$nvmrc_path" ]; then
    local nvmrc_node_version=$(nvm version "$(cat "${nvmrc_path}")")

    if [ "$nvmrc_node_version" = "N/A" ]; then
      nvm install
    elif [ "$nvmrc_node_version" != "$node_version" ]; then
      nvm use
    fi
  elif [ "$node_version" != "$(nvm version default)" ]; then
    echo "Reverting to nvm default version"
    nvm use default
  fi
}
add-zsh-hook chpwd load-nvmrc
load-nvmrc

⚠️ For folks arriving at this solution via search: while this will probably work, this implementation is adding in a chpwd hook that introduces a non-trivial load-nvmrc() which will be invoked every single time you are changing directory.

See docs for more and please exercise caution before throwing this into .zshrc.