-
Notifications
You must be signed in to change notification settings - Fork 45
chore: add package.json node engine to pre commit version check #1734
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
base: dev
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR extends the pre-commit hook to validate that the Node version specified in package.json (under engines.node) matches the versions in .tool-versions and .nvmrc, ensuring consistency across all Node version declarations in the project.
- Adds extraction of Node version from
package.jsonengines field - Updates the version mismatch check to compare all three sources
- Updates error messages to include package.json version and reflect the three-way comparison
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| NVMRC_VERSION=$(tr -d '\n\r' < .nvmrc) | ||
| NVMRC_VERSION=$(tr -d '\n\r' <.nvmrc) | ||
| # Extract version from package.json > engines.node | ||
| PACKAGE_VERSION=$(grep -A 1 '"engines"' package.json | grep '"node"' | cut -d'"' -f4) |
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The grep-based extraction is fragile and may fail in certain scenarios. The current approach assumes "node" appears exactly one line after "engines" in the JSON, which may not always be true (e.g., if there are comments, different formatting, or additional whitespace). Consider using a more robust JSON parsing tool like jq to extract the value reliably, for example: jq -r '.engines.node // ""' package.json. This would handle various JSON formatting styles and provide an empty string if the field doesn't exist.
| PACKAGE_VERSION=$(grep -A 1 '"engines"' package.json | grep '"node"' | cut -d'"' -f4) | |
| PACKAGE_VERSION=$(jq -r '.engines.node // ""' package.json) |
| NVMRC_VERSION=$(tr -d '\n\r' <.nvmrc) | ||
| # Extract version from package.json > engines.node | ||
| PACKAGE_VERSION=$(grep -A 1 '"engines"' package.json | grep '"node"' | cut -d'"' -f4) | ||
|
|
Copilot
AI
Dec 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comparison logic doesn't validate whether PACKAGE_VERSION was successfully extracted. If package.json doesn't have an engines.node field or the extraction fails, PACKAGE_VERSION will be empty, and the comparison will likely pass incorrectly or produce confusing error messages. Add a check to ensure PACKAGE_VERSION is not empty before performing the comparison, or handle the case where it might not be present.
| if [ -z "$PACKAGE_VERSION" ]; then | |
| echo "❌ Could not extract Node version from package.json (missing or malformed engines.node field)." | |
| exit 1 | |
| fi |
No description provided.