Add node preFlight checks to CLI#2892
Conversation
MarkBind requires some checks to improve UX. Let's * Add a new file preFlightChecks to contain all of these checks * Add a preAction hook to run the checks before invoking any command
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a CLI pre-flight check that blocks command execution when the running Node.js version is below the minimum supported version (>= 22.12.0), resolving #2890.
Changes:
- Introduce
preFlightChecks()with a Node version gate usingsemver. - Register a Commander
preActionhook to run pre-flight checks before executing commands. - Add
semverand its TypeScript types to the CLI package dependencies.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/cli/src/util/preFlightChecks.ts | Implements Node version validation and error/exit behavior. |
| packages/cli/index.ts | Hooks pre-flight checks into Commander’s preAction lifecycle. |
| packages/cli/package.json | Adds semver runtime dependency and @types/semver for typing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| logger.error( | ||
| `Markbind ${CLI_VERSION} requires Node >=22.12.0. ` | ||
| + 'Please update Node or use an older version of MarkBind!' | ||
| ); | ||
| process.exit(1); |
There was a problem hiding this comment.
Calling process.exit(1) inside a utility invoked from Commander hooks makes this harder to test and can bypass any centralized error handling/cleanup. Prefer throwing an error (or a Commander-specific error type) and letting the CLI entrypoint decide the exit code.
| logger.error( | |
| `Markbind ${CLI_VERSION} requires Node >=22.12.0. ` | |
| + 'Please update Node or use an older version of MarkBind!' | |
| ); | |
| process.exit(1); | |
| const message = `Markbind ${CLI_VERSION} requires Node >=22.12.0. ` | |
| + 'Please update Node or use an older version of MarkBind!'; | |
| logger.error(message); | |
| throw new Error(message); |
There was a problem hiding this comment.
I'm thinking to leave this in. Throwing an error will print the whole stack trace to the user, it's ugly and less helpful from the user's standpoint IMO.
Instead I've added some explicit documentation through jsDoc that says calling the function may exit the process if checks fail.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2892 +/- ##
========================================
Coverage 71.35% 71.35%
========================================
Files 133 133
Lines 7229 7229
Branches 1611 1712 +101
========================================
Hits 5158 5158
+ Misses 2065 1970 -95
- Partials 6 101 +95 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
What is the purpose of this pull request?
Resolves #2886
Overview of changes:
Adds a pre-flight check that ensures that Node version is >=22.12.0.
Anything you'd like to highlight/discuss:
Here's what it looks like:
Testing instructions:
Use nvm/whatever your environment uses to install and use a version of node that is incompatible, and see that building a site or any command fails. Switch back to a compatible version and see that it works.
Proposed commit message: (wrap lines at 72 characters)
Add node preflight checks to CLI
Checklist: ☑️
Reviewer checklist:
Indicate the SEMVER impact of the PR:
At the end of the review, please label the PR with the appropriate label:
r.Major,r.Minor,r.Patch.Breaking change release note preparation (if applicable):