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

build(node): use .nvmrc to declare node version #801

Conversation

kinggoesgaming
Copy link
Contributor

@kinggoesgaming kinggoesgaming commented Jul 21, 2024

Test Plan

Current tests should pass as-is

Checklist

  • Docs updated

Screenshots

This does not have any changes that need screenshots

closes: #799

@kinggoesgaming kinggoesgaming marked this pull request as ready for review July 21, 2024 20:36
Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Thanks. I don't use nvm though. Is there no way to have these tools reference the engines config already in the package.json?

@kinggoesgaming
Copy link
Contributor Author

kinggoesgaming commented Jul 24, 2024

There might be a GitHub Action for fetching the value (similar to how we fetch fly app name). However I am not familiar with that.

As per .nvmrc, node GitHub Action supports reading the version from any file, as long the only text it has is the version of node to use.

I just used .nvmrc, as nvm is the most popular version manager for node. Technically we can call it whatever we want.

This does not mean that you need to use nvm. Having .nvmrc simply makes it easy for anyone using nvm to use the same version. If you don't it doesn't affect you per-se.

@kinggoesgaming kinggoesgaming force-pushed the 799-use-nvmrc-file-to-declare-the-node-version-to-use branch from 0212fc5 to 30520c1 Compare August 3, 2024 20:17
@kentcdodds
Copy link
Member

Is there any way for these tools to reference the engines config in the package.json so we don't need an entirely separate file for managing the node version?

@alcpereira
Copy link
Contributor

Is there any way for these tools to reference the engines config in the package.json so we don't need an entirely separate file for managing the node version?

I've investigated it, and it seems that:

I personally recommend fnm to everyone that uses nvm as it's significantly faster (see this article regarding nvm).
Should we proceed with recommending fnm in the docs to use the correct version?

Regarding the action setup-node, it also accepts package.json as node-version-file 🎉

@kentcdodds
Copy link
Member

That all sounds fine to me. I'm fine with documentation that recommends fnm as long as we don't need to add yet another root-level file to the codebase.

@kinggoesgaming
Copy link
Contributor Author

Regarding the action setup-node, it also accepts package.json as node-version-file 🎉

Interesting I didn't know this.. I will update to use it

@kentcdodds
Copy link
Member

Closing this for now. Feel free to open a new PR for supporting the package.json

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.

Use .nvmrc file to declare the node version to use
3 participants