Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NODE_VERSION_PREFIX default value should be empty string? #207

Closed
ldeck opened this issue Mar 22, 2016 · 15 comments
Closed

NODE_VERSION_PREFIX default value should be empty string? #207

ldeck opened this issue Mar 22, 2016 · 15 comments

Comments

@ldeck
Copy link

ldeck commented Mar 22, 2016

I'm using NVM to manage node versions. So node versions are stored in ~/.nvm/versions/node.

In my .envrc I have

set -e
use node

And in my ~/.bashrc I've exported NODE_VERSIONS

export NODE_VERSIONS=$NVM_DIR/versions/node
export NODE_VERSIONS_PREFIX=""

use_node fails the find query because it's defaulting to adding a prefix of 'node-v' instead of the empty string.

As a workaround I've added the following function to ~/.direnvrc but I'm wondering if there was something I missed? Otherwise, can the default value be empty for NODE_VERSION_PREFIX?

set -e 

use_nvm() {
  local version=$1
  local via=""
  local node_wanted
  local node_prefix

  if [[ -z $version ]] && [[ -f .nvmrc ]]; then
      version=$(< .nvmrc)
      via=".nvmrc"
  fi

  if [[ -z $version ]]; then
      log_error "I do not know which NodeJS version to load via nvm because one has not been specified!"
      return 1
  fi

  node_wanted=${NVM_VERSION_PREFIX:-""}$version
  node_prefix=$(find "$NVM_DIR" -maxdepth 3 -mindepth 1 -type d -name "$node_wanted*" | sort -r -t . -k 1,1n -k 2,2n -k 3,3n | head -1)

  if [[ ! -d $node_prefix ]]; then
      log_error "Unable to find NodeJS version ($node_wanted) via nvm!"
      return 1
  fi

  if [[ ! -x $node_prefix/bin/node ]]; then
      log_error "Unable to load NodeJS binary (node) for version ($version) via nvm!"
      return 1
  fi

  load_prefix "$node_prefix"

  if [[ -z $via ]]; then
      log_status "Successfully loaded NodeJS $(node --version), from prefix ($node_prefix)"
  else
      log_status "Successfully loaded NodeJS $(node --version) (via $via), from prefix ($node_prefix)"
  fi
}
@ldeck
Copy link
Author

ldeck commented Mar 22, 2016

Actually if I remove the 'v' prefix from my .nvmrc and export NODE_VERSION_PREFIX='v' it works okay.

Of course it'd be nice if an .nvmrc file is located that the NVM_DIR is used or nvm which <version>.

@wilmoore
Copy link
Contributor

I've just updated the wiki page so it reflects all of this: https://github.com/direnv/direnv/wiki/Node

If you set both NODE_VERSIONS=$HOME/.nvm/versions/node and NODE_VERSIONS_PREFIX="v" then add a .nvmrc file; you should be good to go.

@zimbatm
Copy link
Member

zimbatm commented Mar 22, 2016

Thanks. If you have any ideas on how to improve the stdlib.sh just let me know. I don't have a setup with nvm installed right now to test.

@wilmoore
Copy link
Contributor

Currently NODE_VERSIONS_PREFIX does default to node-v because that's how they get pulled down from: http://nodejs.org/dist/latest

For example: http://nodejs.org/dist/latest/node-v5.9.0-darwin-x64.tar.gz

I typically pull them down like so:

cd $NODE_VERSIONS_PREFIX
mkdir node-v5.9.0
curl -L# 'http://nodejs.org/dist/latest/node-v5.9.0-darwin-x64.tar.gz' | tar xz --strip-components=1 -C "node-v5.9.0"

I started doing node-v when io-js was a thing so there was a distinction; however, it probably isn't all that relevant anymore.

It might be best to make the default either "v" or "". I'm happy to hear some suggestions and then based on feedback make the appropriate change.

Whatever feels like the most unsurprising default is what it should be.

The line in question is:

node_wanted=${NODE_VERSION_PREFIX:-"node-v"}$version

@ldeck
Copy link
Author

ldeck commented Mar 22, 2016

One way to improve it would using nvm itself to resolve node. That would reduce the need for redundant environment vars too. e.g., say I have the following versions of node installed:

08:54 $ nvm ls
        v0.12.2
->       v5.6.0
         v5.7.1
         v5.9.0
default -> 5.6 (-> v5.6.0)
node -> stable (-> v5.9.0) (default)
stable -> 5.9 (-> v5.9.0) (default)
iojs -> N/A (default)

nvm can resolve the exact location for you like this:

$ nvm which 5.9
/Users/ldeck/.nvm/versions/node/v5.9.0/bin/node
$ nvm which 5.7
/Users/ldeck/.nvm/versions/node/v5.7.1/bin/node

i.e., if you find an .nvmrc file, and nvm is on the PATH, then use nvm to resolve node rather than find. e.g.,

use_node() {
  local version=$1
  local via=""
  local node_wanted
  local node_prefix

  if [[ -z $version ]] && [[ -f .nvmrc ]]; then
    version=$(< .nvmrc)
    node_prefix=$(dirname $(dirname $(nvm which 5.7)))
  else
    if [[ -z $NODE_VERSIONS ]] || [[ ! -d $NODE_VERSIONS ]]; then
    log_error "You must specify a \$NODE_VERSIONS environment variable and the directory specified must exist!"
    return 1
    fi
    ...
  fi

  if [[ ! -d $node_prefix ]]; then
    log_error "Unable to find NodeJS version ($version) in ($NODE_VERSIONS)!"
    return 1
  fi

  if [[ ! -x $node_prefix/bin/node ]]; then
    log_error "Unable to load NodeJS binary (node) for version ($version) in ($NODE_VERSIONS)!"
    return 1
  fi

  load_prefix "$node_prefix"

  if [[ -z $via ]]; then
    log_status "Successfully loaded NodeJS $(node --version), from prefix ($node_prefix)"
  else
    log_status "Successfully loaded NodeJS $(node --version) (via $via), from prefix ($node_prefix)"
  fi
}

@zimbatm
Copy link
Member

zimbatm commented Mar 22, 2016

Is load_prefix useful other than setting the PATH ? Otherwise this could almost be reduced to:

use_node() {
  PATH_add "$(nvm which $(< .nvmrc))"
}

@wilmoore
Copy link
Contributor

So, I should have realized why you are even doing this when you are already using nvm which itself supports loading .nvmrc files. I'm guessing you are annoyed by having to manually type nvm use manually?

If that is the case, maybe you can add the following to your .envrc file:

if [[ -f .nvmrc ]]; then
  nvm use
fi

@wilmoore
Copy link
Contributor

Also, if you are wondering why one wouldn't just use nvm (I do not), then ping me and I'll tell you why I don't use it.

@zimbatm
Copy link
Member

zimbatm commented Mar 23, 2016

@wilmoore please do, it's always interesting to hear people's opinion.

For nvm you might want to put a shim like this in your ~/.direnvrc to activate it on first use:

nvm() {
  . ~/.nvm/nvm.sh
  nvm "$@"
}

@wilmoore
Copy link
Contributor

@wilmoore please do, it's always interesting to hear people's opinion.

I kinda wanted to avoid openly disparaging someone's work, especially given it's a free too and the author works on it in his free time; however, you've asked so I will answer.

The short and sweet answer is that I feel it is unnecessarily opinionated and has subtle but unexpected (annoying) behavior. For example, it will actually refuse to work if you have certain legitimate npm configurations enabled. Also, in trying to use --reinstall-packages-from I've found it to be a bit obtuse.

For me, the cons outweigh the pros. I've spoken to other developers about it and some feel the pros are worth it. For me, the pros I have replicated by using direnv which is unopinionated and minimal and I use it for other language. For someone that isn't even invested in NodeJS, probably should just install the latest version with homebrew and not worry about it.

I outlined earlier how to install node versions via curl. I more recently wrote and started using wilmoore/nodejs-install to install NodeJS versions which is just a slightly nicer wrapper around curl. It's super minimal and I sometimes feel like I don't even need it, but the one nice thing is that it lets you list the installable versions of node which is sometimes handy.

I also have tried using n a few times and each time I decided it wasn't worth it. Both nvm and n have some really surprising behavior which always makes me question why I installed them in the first place.

Here are some links which will point you to some of my gripes:

@zimbatm
Copy link
Member

zimbatm commented Mar 23, 2016

Interesting. That's generally the same conclusion I had in regards to ruby version managers (and lead to this tool). For ruby I use ruby-install that puts different version of ruby inside of ~/.rubies and then direnv for the version switching.

Adding your nodejs-install method to the wiki would be helpful to others I believe.

@wilmoore
Copy link
Contributor

Interesting. That's generally the same conclusion I had in regards to ruby version managers (and lead to this tool).

Indeed. I've bounced around a few different ruby version managers as well. I even wrote one and threw it out eventually :)

For ruby I use ruby-install that puts different version of ruby inside of ~/.rubies and then direnv for the version switching.

Yup, same here.

Adding your nodejs-install method to the wiki would be helpful to others I believe.

I was thinking about it. I was hoping to clean up the docs a bit to make installing easier and potentially setup an actual brew tap so you don't have to install via a URL.

That being said, I'll go ahead and add it and hopefully I'll do that cleanup soon.

@zimbatm
Copy link
Member

zimbatm commented Mar 23, 2016

Cool. In my experience, something is better than a perfect nothing :) At some point I would love to create a guide book and having pointers from the wiki will be very helpful.

@wilmoore
Copy link
Contributor

👍

@zimbatm
Copy link
Member

zimbatm commented Mar 23, 2016

Alright. Closing since we discussed all the options and there is no clear actionable for now (except improve the wiki). But feel free to pile it on if you feel that there is more to say about it.

I'm also often on IRC on the freenode #direnv channel if you want to discuss about anything. Or just open another issue with the topic you have in mind if you prefer async discussions.

@zimbatm zimbatm closed this as completed Mar 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants