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

feat(steps): add install-node step #49

Merged
merged 4 commits into from May 24, 2024

Conversation

barbados-clemens
Copy link
Contributor

@barbados-clemens barbados-clemens commented May 6, 2024

This step allows installing a defined version of node via nvm.
This version can be passed to agents via the node_version input.
environment variable or via populating a .nvmrc file in the workspace.

- name: Install Node v18
  uses:
  'nrwl/nx-cloud-workflows/<version>/workflow-steps/install-nodes/main.yaml'
  inputs:
    node_version: 18

if a .nvmrc file exists in the workspace (via a checkout step), then
the node_version input can be omitted.

- name: Install Node v18
  uses:
  'nrwl/nx-cloud-workflows/<version>/workflow-steps/install-nodes/main.yaml'

the volta.node version will be picked up as well if node_version input or .nvmrc file isn't passed in.

NOTE: we still allow passing NODE_VERSION env as a fallback for now. so we could technically publish a v3 and v4 of the step if so desired without modifications. This would allow us to add this step in the default workflow and folks can pass via the nx-cloud cli a --with-env-vars="NODE_VERSION" and that node version will be used. (or a .nvmrc/volta.node version).

Will hold off on updating the default template until I can confirm how we want to proceed.

@barbados-clemens barbados-clemens self-assigned this May 6, 2024
@barbados-clemens barbados-clemens force-pushed the feat/APP-1520-node-js-step branch 2 times, most recently from 16886ee to 5a011be Compare May 6, 2024 16:29
// TODO: switch step to remove nvm instaall when nvm is in the base image
if (process.env.NX_CLOUD_NODE_VERSION || existsSync('.nvmrc')) {
try {
// have to run as single command to keep the nvm env vars in the same shell
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: there was some issues when I tried running these commands as discrete commands where I'd get permission denied and other errors when trying to load in the nvm.sh file, so I just switch to mimic the equivalent in bash scripts to get things working.

I also tried adding nvm into the base image, but ran into the same issues with permission errors and unable to find things correctly in PATH. unsure what the exact issue is there.

@barbados-clemens
Copy link
Contributor Author

Also I'd like some help in testing this, as trying to reference this step results in steps erroring out sliently in some way that I'm unsure how to debug.

image
https://staging.nx.app/cipes/6638e2898f54e76fcee20fb4

@barbados-clemens barbados-clemens added the enhancement New feature or request label May 13, 2024
@barbados-clemens
Copy link
Contributor Author

Might need to go back to the drawing board on this one, as using corepack results in potential issues of peoples package.json getting modified if they aren't using corepack which can result in affected/cache hits.
WkMacM1-Vq6trj0J_05200908

! The local project doesn't define a 'packageManager' field. Corepack will now add one referencing yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e.
! For more details about this field, consult the documentation at https://nodejs.org/api/packages.html#packagemanager

@JamesHenry
Copy link
Collaborator

@barbados-clemens You can try setting COREPACK_ENABLE_AUTO_PIN=0

Source: https://github.com/nodejs/corepack?tab=readme-ov-file#environment-variables

@barbados-clemens barbados-clemens marked this pull request as ready for review May 24, 2024 15:10
This step allows installing a defined version of node via nvm.
This version can be passed to agents via the `NX_CLOUD_NODE_VERSION`
environment variable or via populating a `.nvmrc` file in the workspace.
```yaml
- name: Install Node v18
  uses:
  'nrwl/nx-cloud-workflows/<version>/workflow-steps/install-nodes/main.yaml'
  env:
    NX_CLOUD_NODE_VERSION: 18

```

if a `.nvmrc` file exists in the workspace (via a checkout step), then
  the `NX_CLOUD_NODE_VERSION` can be omitted.

```yaml
- name: Install Node v18
  uses:
  'nrwl/nx-cloud-workflows/<version>/workflow-steps/install-nodes/main.yaml'
```
@barbados-clemens barbados-clemens force-pushed the feat/APP-1520-node-js-step branch 2 times, most recently from 877ba77 to b5511e5 Compare May 24, 2024 15:13
@barbados-clemens
Copy link
Contributor Author

fixed issued with info from james in a new test image.

we'll follow up with support windows when we figure out what toolchain
makes the most sense to use for custom node versions in windows
@barbados-clemens barbados-clemens merged commit 205bcd2 into main May 24, 2024
1 check passed
@barbados-clemens barbados-clemens deleted the feat/APP-1520-node-js-step branch May 24, 2024 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants