fix(tools): nvm support added so node is correctly resolved #535
Conversation
AmplifyTools/amplify-tools.sh
Outdated
| NVM_PATH=~/.nvm/nvm.sh | ||
| if [ -f "${NVM_PATH}" ]; then | ||
| echo "NVM found, initializing it..." | ||
| source $NVM_PATH |
There was a problem hiding this comment.
- If we want to support directories with spaces, this needs to be:
NVM_PATH="${HOME}/.nvm/nvm.sh"
(note that ~ can not be used in a string)
- Then if we assume there is a space in the path, we should change the
sourcecommand on line 19 to have quotes like:
source "${NVM_PATH}"
- Btw, what does
nvm.shexpect to have? i'm guessing it updates some path variables?
There was a problem hiding this comment.
I don't fully understand what is inside nvm.sh, but going to assume it is the right thing to do..
There was a problem hiding this comment.
me neither, it's a fairly long file (~3500 lines) that handles both NVM and NodeJS version initialization. Based on my research, that safe to call and there are NVM+Xcode issues out there that suggest the same thing to fix it.
I was able to repro the issue, Xcode - for some reason - doesn't pick my default NodeJS version, which is controlled by NVM. That results in the wrong version of amplify CLI being used by our script, so even when I do npm -g install @aws-amplify/cli in my terminal Xcode still picks the wrong version. By forcing the NVM initialization inside the Xcode Build Phase the problem is gone.
| set -e | ||
|
|
There was a problem hiding this comment.
Sorry, I should have caught this before....
set -e should probably come before export PATH=$PATH:$(npm bin -g) in case npm bin fails.
Issue # #488
Description of changes: When NVM is installed it needs to be manually initialized inside the Xcode script runtime, otherwise a different NodeJS will be used.
ps: also did some formatting on the file (actually VSCode did).
Diff excluding the formatting changes: 0b4f9bc
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.