Fix downgrade to npm 5.x when using version 10.x or greater (#1141)

* Fix downgrade to npm 5.x when using version 10.x or greater

Several checks of the `major` version of npm being used were only accounting for the first character of the version string. E.g.; version `10.1.0` would be treated as `major=1` instead of `major=10`.

This PR changes those checks to split on `.` characters using `cut` and read the `major` version as the first field and adds a test to verify.

Fixes #1140

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## main
 
+- Fixed issue where npm versions `>=10` were being downgraded to version `5.x` ([#1141](https://github.com/heroku/heroku-buildpack-nodejs/pull/1141))
+
 ## v221 (2023-09-19)
 
 - Improved error messaging when installing an incompatible npm version.

bin/compile

@@ -32,6 +32,8 @@ source "$BP_DIR/lib/output.sh"
 source "$BP_DIR/lib/monitor.sh"
 # shellcheck source=lib/environment.sh
 source "$BP_DIR/lib/environment.sh"
+# shellcheck source=lib/npm.sh
+source "$BP_DIR/lib/npm.sh"
 # shellcheck source=lib/failure.sh
 source "$BP_DIR/lib/failure.sh"
 # shellcheck source=lib/binaries.sh

lib/binaries.sh

@@ -112,7 +112,7 @@ install_npm() {
 
   # If the user has not specified a version of npm, but has an npm lockfile
   # upgrade them to npm 5.x if a suitable version was not installed with Node
-  if $npm_lock && [ "$version" == "" ] && [ "${npm_version:0:1}" -lt "5" ]; then
+  if $npm_lock && [ "$version" == "" ] && [ "$(npm_version_major)" -lt "5" ]; then
     echo "Detected package-lock.json: defaulting npm to version 5.x.x"
     version="5.x.x"
   fi

lib/dependencies.sh

@@ -197,12 +197,10 @@ has_npm_lock() {
 should_use_npm_ci() {
   local build_dir=${1:-}
   local npm_version
+  local major
 
   npm_version=$(npm --version)
-  # major_string will be ex: "4." "5." "10"
-  local major_string=${npm_version:0:2}
-  # strip any "."s from major_string
-  local major=${major_string//.}
+  major=$(npm_version_major)
 
   # We should only run `npm ci` if all of the manifest files are there, and we are running at least npm 6.x
   # `npm ci` was introduced in the 5.x line in 5.7.0, but this sees very little usage, < 5% of builds

lib/failure.sh

@@ -745,7 +745,7 @@ warn_old_npm() {
 
   npm_version="$(npm --version)"
 
-  if [ "${npm_version:0:1}" -lt "2" ]; then
+  if [ "$(npm_version_major)" -lt "2" ]; then
     warning "This version of npm ($npm_version) has several known issues. Please update your npm version in package.json." "https://devcenter.heroku.com/articles/nodejs-support#specifying-an-npm-version"
     mcount 'warnings.npm.old'
   fi
@@ -757,7 +757,7 @@ warn_old_npm_lockfile() {
 
   npm_version="$(npm --version)"
 
-  if $npm_lock && [ "${npm_version:0:1}" -lt "5" ]; then
+  if $npm_lock && [ "$(npm_version_major)" -lt "5" ]; then
     warn "This version of npm ($npm_version) does not support package-lock.json. Please
        update your npm version in package.json." "https://devcenter.heroku.com/articles/nodejs-support#specifying-an-npm-version"
     mcount 'warnings.npm.old-and-lockfile'

lib/npm.sh

@@ -0,0 +1,5 @@
+#!/usr/bin/env bash
+
+npm_version_major() {
+  npm --version | cut -d "." -f 1
+}

test/fixtures/use-npm-v10/package.json

@@ -0,0 +1,9 @@
+{
+  "name": "use-npm-v10",
+  "version": "1.0.0",
+  "main": "index.js",
+  "license": "MIT",
+  "engines": {
+    "node": "^20.7"
+  }
+}

test/run

@@ -1529,6 +1529,13 @@ testNpmCiCachingWhenUseNpmInstallIsFalse() {
   assertCapturedSuccess
 }
 
+testUseNpm10() {
+  compile "use-npm-v10"
+  assertNotCaptured "Detected package-lock.json: defaulting npm to version 5.x.x"
+  assertCaptured "Using default npm version: 10"
+  assertCapturedSuccess
+}
+
 # Utils
 
 pushd "$(dirname 0)" >/dev/null