actions / setup-node

Set up your GitHub Actions workflow with a specific version of node.js
MIT License
3.84k stars 1.26k forks source link

Support .nvmrc #32

Closed dougmoscrop closed 2 years ago

dougmoscrop commented 5 years ago

It would be nice if setup-node understood an .nvmrc file (even if it does not use nvm under the hood, the idea is the node-version is just the contents of the file).

If GitHub Actions as a whole could read the contents of a file as part of an expression, I could do something like

uses: setup-node@v1
with:
  node-version: {{ contents(.nvmrc) }}

But I don't know where to log that type of feature request!

Perhaps '.nvmrc' could be used as a special placeholder, so that:

uses: setup-node@v1
with:
  node-version: '.nvmrc'

Instructs this action to read the contents and treat the contents as the desired version?

damccorm commented 5 years ago

If GitHub Actions as a whole could read the contents of a file as part of an expression

I think its probably unlikely we're going to expand our expression parsing to that degree - in general computation isn't supposed to happen there and we want to discourage moving in that direction, and keep that more towards just reading existing variables. Note that you could do something like the following here:

run: nvmrc=$(cat .nvmrc)

uses: setup-node@v1
with:
  node-version: $nvmrc

Perhaps '.nvmrc' could be used as a special placeholder

I'm interested in this option. We're looking at doing something similar in setup-dotnet: https://github.com/actions/setup-dotnet/pull/15. I see a couple options here:

1) If there's a .nvmrc at the root of the repo (or the current working directory) and no node-version specified, automatically use the version in the .nvmrc file. Maybe have a variable that allows you to turn this on or off. 2) Allow node-version to be a path to a .nvmrc file. 3) Don't allow this option.

@chrispat, any thoughts on this one? I think maybe we should establish a pattern here and with setup-dotnet for similar requests (I'm sure node and dotnet aren't the only tools with config files that can specify versions).

dougmoscrop commented 5 years ago

Thanks for the tip* about just using an output, that makes sense and is good enough for me, though the simpler developer experience would be nice.

chingc commented 5 years ago

I think its probably unlikely we're going to expand our expression parsing to that degree - in general computation isn't supposed to happen there and we want to discourage moving in that direction, and keep that more towards just reading existing variables. Note that you could do something like the following here:

run: nvmrc=$(cat .nvmrc)

uses: setup-node@v1
with:
  node-version: $nvmrc

I tried to do this and it seems to be treating the environment variable as a literal instead of getting its value.

Here's the error and my config:

##[error]Unable to find Node version '${NVMRC}' for platform linux and architecture x64.

- name: Read .nvmrc
  run: NVMRC=$(cat .nvmrc)

- name: Use Node.js (.nvmrc)
  uses: actions/setup-node@v1
  with:
    node-version: ${NVMRC}
damccorm commented 5 years ago

Sorry, had a lapse when I wrote this. Try changing your workflow to:

- name: Read .nvmrc
  run: echo "##[set-output name=NVMRC;]$(cat .nvmrc)"
  id: nvm

- name: Use Node.js (.nvmrc)
  uses: actions/setup-node@v1
  with:
    node-version: "${{ steps.nvm.outputs.NVMRC }}"

Env variables are scoped to a given step so they don't automatically carry over, but we can use outputs instead.

chingc commented 5 years ago

@damccorm That worked thank you!

travi commented 4 years ago

I see a couple options here:

  1. If there's a .nvmrc at the root of the repo (or the current working directory) and no node-version specified, automatically use the version in the .nvmrc file. Maybe have a variable that allows you to turn this on or off.
  2. Allow node-version to be a path to a .nvmrc file.
  3. Don't allow this option.

if it's helpful in making the decision, travis-ci has the behavior of 1. and is very often sufficient for my projects. it also has the benefit of removing the need to keep the versions in sync for what is intended for use with local development and the version intended for CI since there is a single source of truth rather than distributing across multiple files.

peaceiris commented 4 years ago

The following also works well.

    - name: Read .nvmrc
      run: echo ::set-output name=NVMRC::$(cat .nvmrc)
      id: nvm

    - name: Setup node
      uses: actions/setup-node@v1
      with:
        node-version: '${{ steps.nvm.outputs.NVMRC }}'
chingc commented 4 years ago

Has there been any progress towards natively supporting .nvmrc instead of having to use set-output?

thejoebourneidentity commented 4 years ago

We got the following issue in the virtual environments repo: https://github.com/actions/virtual-environments/issues/4

What's the latest on the plans to support .nvmrc? Is it on the roadmap?

chrispat commented 4 years ago

I think the action could be updated to support .nvmrc as the default if no version is specified in the node-version parameter. At the moment we do not have anyone we an assign to do that work in the action but it does feel like something we could code review as a community contribution.

thejoebourneidentity commented 4 years ago

I chatted with @bryanmacfarlane about this. I think for now we're going to close this issue, and instead move forward with the recommendation to just add nvm to the image as is requested in the virtual environments repo. https://github.com/actions/virtual-environments/issues/4

sarink commented 4 years ago

cating an nvmrc file and using that version should be pretty simple, right?

If you're not already using nvm under the hood to install node versions... Why add another dependency to the mix?

In any case

      - name: Read .nvmrc
        id: node_version
        run: echo ::set-output name=NODE_VERSION::$(cat .nvmrc)

      - name: Setup node
        uses: actions/setup-node@v1
        with:
          node-version: ${{ steps.node_version.outputs.NODE_VERSION }}

works, it's just really wordy to have to put it everywhere

bryanmacfarlane commented 4 years ago

Yeah, pretty straight forward and we haven't closed so we'll get to it at some point. Note that in your example above you should add shell: bash to your first run step if you want to ensure it works on windows as well

schoenwaldnils commented 4 years ago

Wait, this is not supported yet? Well, good to know now ๐Ÿคท๐Ÿปโ€โ™‚๏ธ I really hope this will be getting implemented soon.

peaceiris commented 4 years ago

Update/Add nvm ยท Issue #4 ยท actions/virtual-environments was closed and nvm will be available on Actions runners ๐Ÿš€

chingc commented 4 years ago

Update/Add nvm ยท Issue #4 ยท actions/virtual-environments was closed and nvm will be available on Actions runners ๐Ÿš€

I'm so happy about this.

vvo commented 4 years ago

Awesome! How can we know once it's deployed and we can remove custom actions we had to support nvm? Thanks ๐Ÿ™

thejoebourneidentity commented 4 years ago

Sorry for the late reply here, you can always follow along over at https://github.com/actions/virtual-environments

For instance, nvm was added some time back to the Ubuntu images :) https://github.com/actions/virtual-environments/blob/master/images/linux/Ubuntu1804-README.md

I think we can close this issue @bryanmacfarlane

chrispat commented 4 years ago

I don't think having NVM on the images is the same thing given NVM does not support Windows. I think the best option would be to have this actions support the .nvmrc. If someone would like to raise a PR for it we could likely accept it.

runofthemillgeek commented 4 years ago

@chrispat can I pick this up? Would love to know if you have any thoughts in mind or guidance w.r.t to this.

chrispat commented 4 years ago

@sangeeth96 feel free to open a PR. My general expectation would be that if node-version is not supplied the action would look for a .nvmrc in the root of the workspace and attempt to use the value from that.

airtonix commented 3 years ago

I don't think having NVM on the images is the same thing given NVM does not support Windows. I think the best option would be to have this actions support the .nvmrc. If someone would like to raise a PR for it we could likely accept it.

On windows you should be using nodist, which then makes this a non issue as nodist doesn't require all the messing about like nvm does.

damianobarbati commented 3 years ago

Guys sorry to ask, but... what is the current correct way to let setup-node use the .nvmrc specific node version? ๐Ÿ˜… I got lost and I can't find it in the README

dessant commented 3 years ago

@damianobarbati, like this:

jobs:
  build:
    name: Build
    runs-on: ubuntu-18.04
    steps:
      - name: Clone repository
        uses: actions/checkout@v2
        with:
          persist-credentials: 'false'
      - name: Get Node.js version
        id: nvm
        run: echo ::set-output name=NODE_VERSION::$(cat .nvmrc)
      - name: Setup Node.js
        uses: actions/setup-node@v2
        with:
          node-version: ${{ steps.nvm.outputs.NODE_VERSION }}
damianobarbati commented 3 years ago

@dessant thanks x1000!

It's so verbose though, is there any intention to make it slimmer with an option in setup-node action?

chingc commented 3 years ago

@damianobarbati If you're using GitHub runners, the Linux and macOS machines have had nvm installed by default for quite some time. You can see a full list of supported software here. I've stopped using setup-node ever since.

damianobarbati commented 3 years ago

@chingc are you sure? I read the article and I'm using Ubuntu 20.04 in my actions, but

      - name: check
        run: |
          nvm install < .nvmrc

results in command not found

corpulentcoffee commented 3 years ago

@damianobarbati For reasons, nvm only works under a login shell (e.g. shell: bash --login {0})... so something like

- name: npm test
  shell: bash --login {0}
  run: nvm install && npm ci && npm test

... might be what you want. If your run script is complex or multiple lines, you might want to pass bash other flags for safety too (e.g. -e, -o pipefail, -O inherit_errexit), or move it out to an external script.

Unlike actions/setup-node, nvm's changes are only active for the duration of the run script, so successive runs that need the same Node environment would want to start with nvm use.

privatenumber commented 3 years ago

Noticed errors weren't being caught when using shell: bash -l {0}

To make sure the GitHub Action fails correctly, make sure to use the -e flag so that bash exists if a command exits with a non-zero status:

shell: bash -e -l {0}

For those looking to copy-paste, my workflow looks like this:

name: Lint code

on:
  push:
    branches: [master, next, next-major, beta, alpha]
  pull_request:
    branches: [master, next, next-major, beta, alpha]

jobs:
  lint-code:
    name: Lint code
    runs-on: ubuntu-latest

    steps:
      - name: Checkout
        uses: actions/checkout@v2
      - name: Lint code
        shell: bash -e -l {0}
        run: |
          nvm i
          npm ci
          npm run lint
skjnldsv commented 3 years ago

What I recently develpped to use the package.json engines

jobs:
  build:
    runs-on: ubuntu-latest

    name: node
    steps:
      - name: Checkout
        uses: actions/checkout@v2

      - name: Read package.json node and npm engines version
        uses: skjnldsv/read-package-engines-version-actions@v1
        id: package-engines-versions

      - name: Show node version number
        run: echo "Node version is ${{ steps.package-engines-versions.outputs.nodeVersion }}"

      - name: Show npm version number
        run: echo "Npm version is ${{ steps.package-engines-versions.outputs.npmVersion }}"

      - name: Set up node ${{ steps.package-engines-versions.outputs.nodeVersion }}
        uses: actions/setup-node@v2
        with:
          node-version: ${{ steps.package-engines-versions.outputs.nodeVersion }}

      - name: Set up npm ${{ steps.package-engines-versions.outputs.npmVersion }}
        run: npm i -g npm@"${{ steps.package-engines-versions.outputs.npmVersion }}"

      - name: Install dependencies & build
        run: |
          npm ci
          npm run build --if-present

      # ... etc
gordey4doronin commented 3 years ago

@peaceiris commented on 9 Oct 2019

The following also works well.

    - name: Read .nvmrc
      run: echo ::set-output name=NVMRC::$(cat .nvmrc)
      id: nvm

    - name: Setup node
      uses: actions/setup-node@v1
      with:
        node-version: '${{ steps.nvm.outputs.NVMRC }}'

Yet another variation. ๐Ÿ‘‡ I guess no real advantages or disadvantages though.

    - name: Read .nvmrc
      run: echo NVMRC=`cat .nvmrc` >> $GITHUB_ENV

    - name: Setup node
      uses: actions/setup-node@v2
      with:
        node-version: ${{ env.NVMRC }}
jameswald commented 2 years ago

Is this fixed with https://github.com/actions/setup-node/pull/338?

dougmoscrop commented 2 years ago

I declare it fixed! I didn't say it, I declared it

mauriciabad commented 2 years ago

If you got here from googling, let me tell you, that the newest and simplest way of reading the .nvmrc file and using it in actions/setup-node is with the node-version-file option:

  - name: Setup node
    uses: actions/setup-node@v3
    with:
      node-version-file: '.nvmrc'
HarelM commented 10 months ago

I recently added this to my action and publishing started failing. I'm still trying to understand if this is the root cause, but I believe that setting node-version-file causes registry-url to be empty, which I'm not sure is the expected behavior. Let me know if there's a need to open a different issue about it.