feat(cli): Enforce a node version via toml option #8907
Conversation
|
@Josh-Walker-GM I think you just about nailed it in terms of implementation. Besides improving the copy in the error message a bit, the main question is really just where does this go in |
|
Note: Should this also check for node versions less than we support so that users can't accidentally run in v16 or whatever? |
|
|
||
| import { getConfig } from '@redwoodjs/project-config' | ||
|
|
||
| export async function enforceNodeVersionConfig() { |
There was a problem hiding this comment.
@Josh-Walker-GM sorry to insert myself into this conversation, I'm a little bit scared of doing checks on the CLI on every run - could I suggest that we wrap the whole thing in a try catch?
This way if anything fails in the logic, for example cooercing the semver version (which happens sometimes, for example with the experimental releases) - it won't stop someone from using rw.
Up to you anyway, just a suggestion.
There was a problem hiding this comment.
Yeah that's certainly something we should to do, happy to.
|
I don't have the full back-story here, but I'm going to make us hold off on this. I think it's in the best interest of a RW app developer to have the same version of Node in all environments the app runs. Most notably both locally, with all devs on the team, and also when deployed. So I don't think we should allow specifying a range or options of different allowed node versions (like When we build the Finally, seeing how quick we've been to adopt new node versions lately, and Netlify's new process on supporting new Node versions I think this might not be as important for our users as it once was. |
|
@Tobbe I was adding this to our list to talk about tomorrow. It's about two things:
TBD |
…es (#9728) Continuation of #8907. Following up on our meeting today (@Tobbe, @thedavidprice; this PR doesn't implement the configurable engines functionality yet). This PR - removes the `.nvmrc` file from CRWA and test project fixtures Setup deploy commands should add node version files if they need them, preferably something nvm agnostic like Netlify's `.node-version` - removes yarn from engines in CRWA's root package.json since it doesn't do anything and just creates confusion - adds a node version check to build and dev This piece of middleware is lightweight; it doesn't involve a child process or the file system, it just checks against `process.version`. I also removed the babel check because it's been ages since we've made that change and original concerns against checking for the node version were about adding overhead tot he CLI. So let's remove unnecessary middleware if we're going to add more Right now, the node version check just emits a warning. Should it error out? (I should've taken better notes.) Build:  Dev:  ## Tasks - [ ] research deploy providers and update `yarn rw setup deploy` commands - [x] settle on warning or error for build and dev - [x] settle on the contents of the message - [x] update CRWA ### Deploy providers - [x] baremetal Pretty sure it's completely up to you. - [x] coherence Via Nixpacks, `engines.node` in the root package.json: https://nixpacks.com/docs/providers/node#setup. - [x] flightcontrol Via Nixpacks, `engines.node` in the root package.json: https://www.flightcontrol.dev/docs/getting-started/javascript/setting-node-version#using-the-packagejson-file. - [x] netlify Via the `.node-version` file, or via in `netlify.toml`; see https://docs.netlify.com/configure-builds/manage-dependencies/#node-js-and-javascript, https://docs.netlify.com/configure-builds/file-based-configuration/#sample-netlify-toml-file. - [ ] vercel Takes its cue from engines.node in the root package.json: https://vercel.com/docs/functions/serverless-functions/runtimes/node-js#version-overrides-in-package.json. - [x] render Same as netlify more or less, but also respects `engines.node` in the root package.json: https://docs.render.com/docs/node-version.
…es (#9728) Continuation of #8907. Following up on our meeting today (@Tobbe, @thedavidprice; this PR doesn't implement the configurable engines functionality yet). This PR - removes the `.nvmrc` file from CRWA and test project fixtures Setup deploy commands should add node version files if they need them, preferably something nvm agnostic like Netlify's `.node-version` - removes yarn from engines in CRWA's root package.json since it doesn't do anything and just creates confusion - adds a node version check to build and dev This piece of middleware is lightweight; it doesn't involve a child process or the file system, it just checks against `process.version`. I also removed the babel check because it's been ages since we've made that change and original concerns against checking for the node version were about adding overhead tot he CLI. So let's remove unnecessary middleware if we're going to add more Right now, the node version check just emits a warning. Should it error out? (I should've taken better notes.) Build:  Dev:  - [ ] research deploy providers and update `yarn rw setup deploy` commands - [x] settle on warning or error for build and dev - [x] settle on the contents of the message - [x] update CRWA - [x] baremetal Pretty sure it's completely up to you. - [x] coherence Via Nixpacks, `engines.node` in the root package.json: https://nixpacks.com/docs/providers/node#setup. - [x] flightcontrol Via Nixpacks, `engines.node` in the root package.json: https://www.flightcontrol.dev/docs/getting-started/javascript/setting-node-version#using-the-packagejson-file. - [x] netlify Via the `.node-version` file, or via in `netlify.toml`; see https://docs.netlify.com/configure-builds/manage-dependencies/#node-js-and-javascript, https://docs.netlify.com/configure-builds/file-based-configuration/#sample-netlify-toml-file. - [ ] vercel Takes its cue from engines.node in the root package.json: https://vercel.com/docs/functions/serverless-functions/runtimes/node-js#version-overrides-in-package.json. - [x] render Same as netlify more or less, but also respects `engines.node` in the root package.json: https://docs.render.com/docs/node-version.
ping @thedavidprice, @jtoar - I didn't really have much information about what this feature was intended to be. I had agreed to take it on to free up Dom a little more.
Adds a toml option like so:
It defaults to undefined which then avoids any checks. When set the CLI will perform a check before all commands and fail if the current node version does not match the specification set in the toml.
Examples:
