Skip to content

chore: Add node-version to package.json and workflows #473

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

Merged

Conversation

baruchiro
Copy link
Contributor

Added .node-version file and updated GitHub Actions workflows to use it for Node.js version management.

@cliffhall
Copy link
Member

Hi @baruchiro!

This seems nice, but I'd much rather use package.json for the file as opposed to adding yet another file to the root of the project for a single variable.

Docs for setup-node show how you can do this. (note, using setup-node@v5):

In main.yml

      - uses: actions/setup-node@v5
        with:
          node-version-file: package.json
          cache: npm

and in package.json

{
  "engines": {
    "node": ">=22.0.0"
  }
}

@cliffhall
Copy link
Member

@baruchiro Any interest in updating the PR to use package.json as the source of the version? I still think this is a good idea.

@cliffhall cliffhall added the waiting on submitter Waiting for the submitter to provide more info label Jun 27, 2025
@baruchiro
Copy link
Contributor Author

@cliffhall updated.

However, I have noticed the Dockerfile versions are 24 and not 22 as declared in README: ^22.7.5.

@cliffhall
Copy link
Member

However, I have noticed the Dockerfile versions are 24 and not 22 as declared in README: ^22.7.5.

@baruchiro Could you update the version to (>=22.7.5) in package.json and README?

For reference, that value was set by this PR where the contributor said that an error was reported for versions less than 22.7.5.

The problem was related to the Fetch API, and what I found was:

The lowest Node.js version where Fetch is natively supported and stable on both Windows and macOS is Node.js 21. While Fetch was introduced as an experimental feature in Node.js v18, it became fully stable and supported in Node.js v21. Prior versions like v12.20.0, while supporting node-fetch (a library that implements Fetch), don't offer the built-in Fetch API.

But to be safe, I'd keep it at 22.7.5 or higher.

@baruchiro
Copy link
Contributor Author

@baruchiro Could you update the version to (>=22.7.5) in package.json and README?

Only needed in the package.json, done

@baruchiro baruchiro changed the title chore: Add .node-version file and update workflows to use it chore: Add node-version to package.json and workflows Jul 1, 2025
Copy link
Member

@cliffhall cliffhall left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@cliffhall
Copy link
Member

Resolved properly

Screenshot 2025-07-01 at 3 32 19 PM

@cliffhall cliffhall merged commit 7460ea2 into modelcontextprotocol:main Jul 1, 2025
4 checks passed
@baruchiro baruchiro deleted the baruchiro/node-version branch July 2, 2025 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting on submitter Waiting for the submitter to provide more info
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants