Respect previously nvm-loaded node version when re-sourcing

[zeorin]

Sometimes nvm.sh may be sourced more than once (e.g. when using direnv and nvm-zsh's lazy-loading feature). If nvm has already loaded a version of node that is not system, don't change that version.

In practice this means that if the currently active version of node is not system, nvm.sh will not change the version of node that is active.

If the version is system, it will still activate default.

[zeorin]

Just thought of something: if there's no system node version, what happens then? What does nvm_ls_current return? none? Should I include this in the check somehow?

[ljharb]

Yes, it returns "none".

[ljharb]

What would be sourcing nvm.sh more than once that wouldn't be checking for nvm functions already existing?

[ljharb]

The current behavior is that whenever I re-source nvm.sh, I get the auto-used version. With this change, that won't be the case.

(also, yes, this needs to handle "none")

[zeorin]

Indeed, that's the idea. The fact that NVM assumes, whenever it's sourced, that the environment is uninitialized is a hindrance to me.

(I've pushed a fix to handle none.)

[zeorin]

nvm functions not existing in the current shell doesn't mean that nvm hasn't been executed or that the environment isn't set up to use a specific version of node. Those functions simply may not have made it back up the scope chain.

My particular use case is when a shell script that direnv executes sources nvm.sh and then runs nvm use (there's an .nvmrc file in the folder, too). However, nvm would have been loaded and executed in the sub-shell that direnv uses for this. The environment variables do persist into my shell, but the functions don't.

I'm also using zsh-nvm, with lazy-loading, meaning that when I run nvm for the 1st time in a session myself, nvm.sh is being sourced. Sometimes this is after direnv has run and set up the environment with nvm's help.

Without this fix, that means as soon as I run a global npm cli executable (which are also lazy-loaded by zsh-env) in a direnv managed folder, nvm thinks it's running for the 1st time and changes the node version from the version specified in the .nvmrc file and already loaded by nvm (when it was executed by direnv) to default.

With this fix, the loaded version of node persists.

[ljharb]

I'll leave this open while I think more about it, but as discussed in #1308, there's a few reasons I'm hesitant. "lazy loading" is not a supported use case for nvm - it's just one some people choose to employ, at their own risk - and if the nvm functions aren't "in scope", then (even if nvm originally set up your PATH) nvm is not actually being used. The functions need to be sourced in your main shell (not in a subshell) for you to be using nvm.

[zeorin]

I appreciate that lazy loading isn't officially supported. But without it, NVM is simply too much overhead to be worth it for me. Using lazy loading for NVM changed my shell startup time on my laptop from 4+ seconds to <0.5 seconds. I'm a big tmux user and I'm opening up shells all. the. time. so that startup time has a big impact in my ability to remain in a "flow" state when developing.

Of course the functions have to be loaded for NVM to do its work. Respecting a previously loaded version of node (by another instance of NVM, even if that instance is no longer present in the current shell) opens up possibilities for NVM to be used in more non-interactive ways, not just by direnv, but also by deployment tools & workflows, and many other as-yet-unimagined uses.

Neither #1316 nor this pull request break anything (as far as the tests are able to indicate), so there should be no impact on current users. I do realize that both these pull requests are decisions about the future of NVM, how it might be used, where it fits in. I appreciate your considered attention to these requests.

[zeorin]

if the nvm functions aren't "in scope", then (even if nvm originally set up your PATH) nvm is not actually being used. The functions need to be sourced in your main shell (not in a subshell) for you to be using nvm.

In my use case, as soon as any nvm command is issued in the shell (direnv has run nvm use in a sub-shell at this point), or node, npm or globally installed npm module is executed, nvm is then loaded (by the lazy loader). Then, nvm's initialization loads the functions into the current shell (and clobbers the existing env vars that the direnv-executed nvm set up—hence this pull request).

[zeorin]

@ljharb I've been struggling with this build issue for ages. If I run the code inside the test manually (after first running test/sourcing/setup) it all works just fine. I cannot reproduce this issue on my local machine.

Would you mind taking a look to see if you can determine why this test is failing?

[ljharb]

it might be helpful here to do nvm ls ; die and see what shows up

[zeorin]

@ljharb I managed to figure out how to run the tests locally (for shell in sh dash bash zsh; do node_modules/.bin/urchin -f -s $shell test/sourcing; done;), and as a result I was able to figure out why the test was failing even though it was working 100% fine if I simply executed the test commands in the shell manually.

Basically, it seems that on the TravisCI VM, nvm is loaded before the tests are run, and the sub shell in which a test is run inherits that environment. In test/sourcing/setup, nvm is unloaded, which cleans up that script's local environment, but not the ancestor's one, which is what the test script inherits.

Since this commit specifically makes nvm check whether or not there's already an nvm-managed version of node loaded by inspecting the environment, it won't load the default alias because nvm-specific enviroment variables are present. Working as intended.

My solution is to fix the test by unloading nvm again in the test itself (and caching and setting NVM_DIR, because that is unset on unload¹), which sets up the environment correctly as if nvm had not yet been loaded.

Rebased on master.

1: Is this desired behaviour? If someone has an NVM_DIR that isn't $HOME/.nvm, and they source nvm.sh again without first setting NVM_DIR, is that a problem? Or is it simply up to them to set the correct NVM_DIR before sourcing? I feel like not unsetting NVM_DIR would be a Good Thing, feels like it would make nvm-related scripts more portable.

[ljharb]

@zeorin thanks for the explanation; I think I understand.

[ljharb]

Thanks!