-
Notifications
You must be signed in to change notification settings - Fork 749
Actually install the version of node we want via NVM/nodenv instead of just checking #434
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| # Only major and minor version should be specified here | ||
| REQUIRED_APOLLO_CLI_VERSION=1.9 | ||
| # Only major version should be specified here | ||
| REQUIRED_NODE_VERSION=8 | ||
| # Specify fully qualified version here | ||
| REQUIRED_NODE_VERSION=8.0.0 | ||
|
|
||
| # Using npx to execute 'apollo' looks for a local install in node_modules before checking $PATH (for a global install) | ||
| APOLLO_CLI="npx --no-install apollo" | ||
|
|
@@ -17,18 +17,33 @@ if [[ -z "$CONFIGURATION" ]]; then | |
| exit 1 | ||
| fi | ||
|
|
||
| # Define NVM_DIR and source the nvm.sh setup script | ||
| [[ -z "$NVM_DIR" ]] && export NVM_DIR="$HOME/.nvm" | ||
| use_correct_node_via_nvm() { | ||
| nvm version $REQUIRED_NODE_VERSION > /dev/null | ||
| if [[ $? -eq 0 ]]; then | ||
| # The version of node that we want is installed. | ||
| nvm use $REQUIRED_NODE_VERSION | ||
| else | ||
| nvm install $REQUIRED_NODE_VERSION | ||
| fi | ||
| } | ||
|
|
||
| use_correct_node_via_nodenv() { | ||
| nodenv install -s $REQUIRED_NODE_VERSION | ||
| nodenv shell $REQUIRED_NODE_VERSION | ||
| } | ||
|
|
||
| if [[ -s "$HOME/.nvm/nvm.sh" ]]; then | ||
| . "$HOME/.nvm/nvm.sh" | ||
| use_correct_node_via_nvm | ||
| elif [[ -x "$(command -v brew)" && -s "$(brew --prefix nvm)/nvm.sh" ]]; then | ||
| . "$(brew --prefix nvm)/nvm.sh" | ||
| use_correct_node_via_nvm | ||
| fi | ||
|
|
||
| # Set up the nodenv node version manager if present | ||
| if [[ -x "$HOME/.nodenv/bin/nodenv" ]]; then | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Full transparency: I just installed a clean version of Nodenv and this file doesn't exist, so I am wondering if this check is incorrect. That being said, I've tested the commands here for installing Node 8.15.0 via Nodenv and they work. Determining if in fact there is an issue in this check should be a separate investigation / PR. |
||
| eval "$("$HOME/.nodenv/bin/nodenv" init -)" | ||
| use_correct_node_via_nodenv | ||
| fi | ||
|
|
||
| parse_version() { | ||
|
|
@@ -37,28 +52,6 @@ parse_version() { | |
| fi | ||
| } | ||
|
|
||
| get_installed_node_version() { | ||
| version=$(node -v) | ||
| if [[ $? -eq 0 ]]; then | ||
| echo "$version" | ||
| fi | ||
| } | ||
|
|
||
| is_mimimum_major_version() { | ||
| [[ "$(parse_version $1 | cut -d. -f1)" -ge $2 ]] | ||
| } | ||
|
|
||
| # Check the installed version of Node, if available | ||
| INSTALLED_NODE_VERSION="$(get_installed_node_version)" | ||
| if [[ -z "$INSTALLED_NODE_VERSION" ]]; then | ||
| echo "error: Apollo CLI requires Node $REQUIRED_NODE_VERSION or higher to be installed." | ||
| exit 1 | ||
| elif ! is_mimimum_major_version "$INSTALLED_NODE_VERSION" $REQUIRED_NODE_VERSION; then | ||
| echo "error: Apollo CLI requires Node $REQUIRED_NODE_VERSION or higher, \ | ||
| but $INSTALLED_NODE_VERSION seems to be installed." | ||
| exit 1 | ||
| fi | ||
|
|
||
| get_installed_cli_version() { | ||
| version=$($APOLLO_CLI -v) | ||
| if [[ $? -eq 0 ]]; then | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think sticking with the major is probably better; especially considering that 8.0.0 is EOL and only the latest 8 (8.15.0 at this writing) is considered LTS.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah good context. Thanks @ljharb. Nodenv was requiring me to specify a fully qualified version. Iāll update this to be 8.15.0 since that is LTS.