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: improve node version resolution #267

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

bilals12
Copy link
Member

Node.js Version Resolution Enhancement

Problem

The action currently fails when using major version numbers (e.g., node: "20") in workflows, producing an error that lists all versions containing "20" without properly resolving to the latest stable version.

Solution

Enhanced the Node version resolution logic to:

  1. Detect when a major version number is specified (e.g., "20")
  2. Query the Node.js version registry to find all matching versions
  3. Filter for stable versions matching the major version
  4. Sort and select the latest compatible version
  5. Install the resolved version

Changes

  • Modified getNodeVersion() in node.js to handle major version numbers
  • Added version resolution logic using Node.js version registry data
  • Added comprehensive tests:
    • Unit tests for version resolution
    • Manual test script for local verification
    • GitHub Actions workflow for integration testing

Testing

  • ✅ Unit tests pass (npm test)
  • ✅ Manual testing with test script confirms:
    • Major version resolution (e.g., "20" -> "20.18.1")
    • Specific version handling (e.g., "20.18.1")
    • Error handling for invalid versions
  • ✅ Added workflow tests for GitHub Actions environment

Example Usage

Major Version (Recommended)

- name: Setup Node.js
  uses: open-turo/action-setup-tools@v2
  with:
    node: "20"  # Will automatically use latest stable 20.x.x version

Specific Version

- name: Setup Node.js
  uses: open-turo/action-setup-tools@v2
  with:
    node: "20.18.1"  # Will use exact version

Using Version Files

The action also supports reading versions from standard Node.js version files:

Using .node-version

20.18.1
- name: Setup Node.js
  uses: open-turo/action-setup-tools@v2
  # Version will be read from .node-version file

Using .nvmrc

20
- name: Setup Node.js
  uses: open-turo/action-setup-tools@v2
  # Version will be read from .nvmrc file if .node-version is not present

The action will automatically:

  1. Use any explicitly specified version in the workflow
  2. Fall back to .node-version if present
  3. Fall back to .nvmrc if present
  4. Resolve major versions to latest stable releases

@bilals12 bilals12 self-assigned this Jan 17, 2025
@bilals12 bilals12 requested a review from a team as a code owner January 17, 2025 20:47
@bilals12 bilals12 requested a review from tagoro9 January 17, 2025 20:48
- Change matrix.node-version to matrix.node to match action input parameter
- Update all references in test workflow to use consistent naming
- Maintain alignment with user-facing parameter name
- Add node input parameter to main action.yaml
- Add node input parameter to action-test/action.yaml
- Support both major version (20) and specific version (20.18.1) formats
Copy link

github-actions bot commented Jan 17, 2025

Changes need to be built and committed to ./dist.

In your local repo, run:

npm run prepare
git commit -a --amend --no-edit
git push --force-with-lease

This will add those changes to your last commit, and get them pushed to the remote.

Changes need to be built and committed to ./dist.

In your local repo, run:

npm run prepare
git commit -a --amend --no-edit
git push --force-with-lease

This will add those changes to your last commit, and get them pushed to the remote.

Changes need to be built and committed to ./dist.

In your local repo, run:

npm run prepare
git commit -a --amend --no-edit
git push --force-with-lease

This will add those changes to your last commit, and get them pushed to the remote.

@bilals12 bilals12 force-pushed the f/fix-node-versioning-bug branch from c020e3a to 7767eae Compare January 17, 2025 21:04
- Add handling for major version numbers (e.g., 20) in parseNvmrcVersion
- Return latest stable version for major version inputs (e.g., 20 -> 20.18.1)
- Add tests for major version resolution and specific version handling
- Move version resolution logic into parseNvmrcVersion method

This allows workflows to use simpler version specifications like:
  node: 20
instead of requiring exact versions like:
  node: 20.18.1
Copy link

Release notes preview

Below is a preview of the release notes if your PR gets merged.


2.1.0 (2025-01-21)

Bug Fixes

  • adding updated dist files (110b53a)
  • linting errors (f371eed)
  • linting: addressing lint issues (6b5f1d1)
  • package: resolving vulns via npm audit fix (7767eae)

Features

  • add node version input parameter (837acb4)
  • improve node version resolution (e423b97)
  • support major version numbers in node version resolution (5b0fd51)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants