ignite icon indicating copy to clipboard operation
ignite copied to clipboard

chore: check node version during preinstall

Open frankcalise opened this issue 1 year ago • 3 comments

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

  • An alternative to #2799 which only addresses nvm users, checks the node version prior to installing dependencies
  • nvm doesn't respect package.json's engines.node key (while it seems asdf does), but scripts/node-version-check.js was added to do this across all tooling

Screenshots (if applicable)

image

frankcalise avatar Oct 15 '24 16:10 frankcalise

@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.

lindboe avatar Oct 17 '24 17:10 lindboe

@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?

frankcalise avatar Oct 17 '24 19:10 frankcalise

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.

lindboe avatar Oct 18 '24 23:10 lindboe