Skip to content

Check for a .nvmrc or .node-version file to determine the Node.js version #1374

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

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

Conversation

Mathiyarasy
Copy link
Contributor

@Mathiyarasy Mathiyarasy commented Jun 2, 2025

Feature Name

  • node

Description of Changes

Changelog

  • Added a new version option, 'project-file', this checks for a .nvmrc or .node-version file to determine the Node.js version.
  • Updated the install.sh script to retrieve the Node.js version from these files if they exist and are not empty.
  • If 'project-file' is specified but neither file exists or both are empty, the script installs the Node.js 'lts' version by default.
  • Added new test cases
  • Replaced the hardcoded 'lts' Node.js version in the existing test case with logic to fetch the version from the Node.js URL

@Mathiyarasy Mathiyarasy requested a review from Kaniska244 June 3, 2025 11:00
@Mathiyarasy Mathiyarasy marked this pull request as ready for review June 3, 2025 11:59
@Mathiyarasy Mathiyarasy requested a review from a team as a code owner June 3, 2025 11:59
Copy link
Contributor

@Kaniska244 Kaniska244 left a comment

Choose a reason for hiding this comment

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

Hi @Mathiyarasy ,

Change looks fine. Left one small comment alone.

"node": {
"version": "project-file"
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Any specific reason why we are adding these test cases such that the container is built from Dockerfile only & not using the image tag directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes @Kaniska244
Using Docker File I can include the .nvmrc (or .node-version) file in the image during the build process which is then gets used in feature.

# Determine the Node.js version using the .nvmrc or .node-version file if present.
if [[ "${NODE_VERSION}" == "project-file" ]]; then
echo "Finding Node version from .nvmrc or .node-version file..."
NODE_VERSION_PATH=$(find . -type f -name ".node-version" | head -n 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure find is the best option. Can we not assume the files need to be in the root directory? What if the project is huge, find would take a long time. if there are multiple of these files, find might order them differently based on where it is running, so something like:

  for version_file in ".node-version" ".nvmrc"; do
        if [[ -f "$version_file" && -s "$version_file" ]]; then
            file_version=$(tr -d '[:space:]' < "$version_file")
            if [[ -n "$file_version" ]]; then
                echo "Using Node version from $version_file: $file_version"
                NODE_VERSION="$file_version"
                break
            else
                echo "$version_file exists but contains only whitespace. Continuing search..."
            fi
        fi
    done

might work more predictably

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.

3 participants