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

chore: check node version during preinstall #2805

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

frankcalise
Copy link
Contributor

Please verify the following:

  • yarn test jest tests pass with new tests, if relevant
  • yarn lint eslint checks pass with new code, if relevant
  • yarn format:check prettier checks pass with new code, if relevant
  • README.md (or relevant documentation) has been updated with your changes
  • If this affects functionality there aren't tests for, I manually tested it, including by generating a new app locally if needed (see docs).

Describe your PR

Screenshots (if applicable)

image

@lindboe
Copy link
Contributor

lindboe commented Oct 17, 2024

@frankcalise I do think this is a significant downgrade from adding .nvmrc for nvm users; nvmrc can be used to really simply install the correct version immediately (and in my case, it'll switch automatically when I switch directories).

Given how much nvm doesn't seem to play nicely with other node version managers in the ecosystem, though, I've been considering switching.

@frankcalise
Copy link
Contributor Author

@lindboe asdf does that also if you add .tool-versions file, but do we really want to add a file for each possible node version manager? I guess supporting the two main ones (that I know of, at least) would be fine

Also, I could have sworn nvm does work against the engines key (I used to use nvm until recently). Can you test with nvm use and go outside the engine range that is defined?

@lindboe
Copy link
Contributor

lindboe commented Oct 18, 2024

Can you test with nvm use and go outside the engine range that is defined?

I can run nvm use 16 on a project that specifies >=18 just fine, and running nvm use with no args doesn't pick up the engines info or anything, it just says "No .nvmrc file found".

asdf does that also if you add .tool-versions file, but do we really want to add a file for each possible node version manager? I guess supporting the two main ones (that I know of, at least) would be fine

I was actually thinking the two of .nvmrc and .node-version, if any, since node is the main thing we want to control, and AFAIK nvm is the main version manager that doesn't support .node-version. Which I think is fairly unreasonable, though.

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