Engines and Node 18+Windows tests

.github/workflows/ci.yml

@@ -66,12 +66,12 @@ jobs:
       fail-fast: false
       matrix:
         node-version:
-          - 12.13.0
-          - 12.x
-          - 14.15.0
+          - 14.17.0
           - 14.x
-          - 16.0.0
+          - 16.13.0
           - 16.x
+          - 18.0.0
+          - 18.x
         platform:
           - os: ubuntu-latest
             shell: bash

.github/workflows/release.yml

@@ -27,12 +27,12 @@ jobs:
       fail-fast: false
       matrix:
         node-version:
-          - 12.13.0
-          - 12.x
-          - 14.15.0
+          - 14.17.0
           - 14.x
-          - 16.0.0
+          - 16.13.0
           - 16.x
+          - 18.0.0
+          - 18.x
         platform:
           - os: ubuntu-latest
             shell: bash
@@ -77,12 +77,12 @@ jobs:
       fail-fast: false
       matrix:
         node-version:
-          - 12.13.0
-          - 12.x
-          - 14.15.0
+          - 14.17.0
           - 14.x
-          - 16.0.0
+          - 16.13.0
           - 16.x
+          - 18.0.0
+          - 18.x
         platform:
           - os: ubuntu-latest
             shell: bash

lib/cli.js

@@ -1,57 +1,112 @@
+// This is separate to indicate that it should contain code we expect to work in
+// all conceivably runnable versions of node.  This is a best effort to catch
+// syntax errors to give users a good error message if they are using a node
+// version that doesn't allow syntax we are using such as private properties, etc
+const createEnginesValidation = () => {
+  const node = process.version.replace(/-.*$/, '')
+  const pkg = require('../package.json')
+  const engines = pkg.engines.node
+  const npm = `v${pkg.version}`
+
+  const cols = Math.min(Math.max(20, process.stdout.columns) || 80, 80)
+  const wrap = (lines) => lines
+    .join(' ')
+    .split(/[ \n]+/)
+    .reduce((left, right) => {
+      const last = left.split('\n').pop()
+      const join = last.length && last.length + right.length > cols ? '\n' : ' '
+      return left + join + right
+    })
+    .trim()
+
+  const unsupportedMessage = wrap([
+    `npm ${npm} does not support Node.js ${node}.`,
+    `You should probably upgrade to a newer version of node as we can't make any`,
+    `promises that npm will work with this version.`,
+    `This version of npm supports the following node versions: \`${engines}\`.`,
+    'You can find the latest version at https://nodejs.org/.',
+  ])
+
+  const brokenMessage = wrap([
+    `ERROR: npm ${npm} is known not to run on Node.js ${node}.`,
+    `You'll need to upgrade to a newer Node.js version in order to use this version of npm.`,
+    `This version of npm supports the following node versions: \`${engines}\`.`,
+    'You can find the latest version at https://nodejs.org/.',
+  ])
+
+  // coverage ignored because this is only hit in very unsupported node versions
+  // and it's a best effort attempt to show something nice in those cases
+  /* istanbul ignore next */
+  const syntaxErrorHandler = (err) => {
+    if (err instanceof SyntaxError) {
+      // eslint-disable-next-line no-console
+      console.error(`${brokenMessage}\n\nERROR:`)
+      // eslint-disable-next-line no-console
+      console.error(err)
+      return process.exit(1)
+    }
+    throw err
+  }
+
+  process.on('uncaughtException', syntaxErrorHandler)
+  process.on('unhandledRejection', syntaxErrorHandler)
+
+  return {
+    node,
+    engines,
+    unsupportedMessage,
+    off: () => {
+      process.off('uncaughtException', syntaxErrorHandler)
+      process.off('unhandledRejection', syntaxErrorHandler)
+    },
+  }
+}
+
 // Separated out for easier unit testing
 module.exports = async process => {
   // set it here so that regardless of what happens later, we don't
   // leak any private CLI configs to other programs
   process.title = 'npm'
 
-  // We used to differentiate between known broken and unsupported
-  // versions of node and attempt to only log unsupported but still run.
-  // After we dropped node 10 support, we can use new features
-  // (like static, private, etc) which will only give vague syntax errors,
-  // so now both broken and unsupported use console, but only broken
-  // will process.exit. It is important to now perform *both* of these
-  // checks as early as possible so the user gets the error message.
-  const semver = require('semver')
-  const supported = require('../package.json').engines.node
-  const knownBroken = '<12.5.0'
-
-  const nodejsVersion = process.version.replace(/-.*$/, '')
-  /* eslint-disable no-console */
-  if (semver.satisfies(nodejsVersion, knownBroken)) {
-    console.error('ERROR: npm is known not to run on Node.js ' + process.version)
-    console.error("You'll need to upgrade to a newer Node.js version in order to use this")
-    console.error('version of npm. You can find the latest version at https://nodejs.org/')
-    process.exit(1)
-  }
-  if (!semver.satisfies(nodejsVersion, supported)) {
-    console.error('npm does not support Node.js ' + process.version)
-    console.error('You should probably upgrade to a newer version of node as we')
-    console.error("can't make any promises that npm will work with this version.")
-    console.error('You can find the latest version at https://nodejs.org/')
-  }
-  /* eslint-enable no-console */
+  // Nothing should happen before this line if we can't guarantee it will
+  // not have syntax errors in some version of node
+  const validateEngines = createEnginesValidation()
 
+  const satisfies = require('semver/functions/satisfies')
   const exitHandler = require('./utils/exit-handler.js')
-  process.on('uncaughtException', exitHandler)
-  process.on('unhandledRejection', exitHandler)
-
   const Npm = require('./npm.js')
   const npm = new Npm()
   exitHandler.setNpm(npm)
 
-  // if npm is called as "npmg" or "npm_g", then
-  // run in global mode.
+  // if npm is called as "npmg" or "npm_g", then run in global mode.
   if (process.argv[1][process.argv[1].length - 1] === 'g') {
     process.argv.splice(1, 1, 'npm', '-g')
   }
 
-  const log = require('./utils/log-shim.js')
   // only log node and npm paths in argv initially since argv can contain
   // sensitive info. a cleaned version will be logged later
+  const log = require('./utils/log-shim.js')
   log.verbose('cli', process.argv.slice(0, 2).join(' '))
   log.info('using', 'npm@%s', npm.version)
   log.info('using', 'node@%s', process.version)
 
+  // At this point we've required a few files and can be pretty sure
+  // we dont contain invalid syntax for this version of node. It's
+  // possible a lazy require would, but that's unlikely enough that
+  // it's not worth catching anymore and we attach the more important
+  // exit handlers.
+  validateEngines.off()
+  process.on('uncaughtException', exitHandler)
+  process.on('unhandledRejection', exitHandler)
+
+  // It is now safe to log a warning if they are using a version of node
+  // that is not going to fail on syntax errors but is still unsupported
+  // and untested and might not work reliably. This is safe to use the logger
+  // now which we want since this will show up in the error log too.
+  if (!satisfies(validateEngines.node, validateEngines.engines)) {
+    log.warn('cli', validateEngines.unsupportedMessage)
+  }
+
   let cmd
   // now actually fire up npm and run the command.
   // this is how to use npm programmatically:

package-lock.json

@@ -178,7 +178,7 @@
         "tap": "^16.0.1"
       },
       "engines": {
-        "node": "^12.13.0 || ^14.15.0 || >=16"
+        "node": "^14.17.0 || ^16.13.0 || >=18.0.0"
       }
     },
     "docs": {

package.json

@@ -256,6 +256,6 @@
   },
   "license": "Artistic-2.0",
   "engines": {
-    "node": "^12.13.0 || ^14.15.0 || >=16"
+    "node": "^14.17.0 || ^16.13.0 || >=18.0.0"
   }
 }

test/fixtures/mock-npm.js

@@ -66,6 +66,7 @@ const LoadMockNpm = async (t, {
   globalPrefixDir = { lib: {} },
   config = {},
   mocks = {},
+  otherDirs = {},
   globals = null,
 } = {}) => {
   // Mock some globals with their original values so they get torn down
@@ -107,13 +108,15 @@ const LoadMockNpm = async (t, {
     prefix: prefixDir,
     cache: cacheDir,
     global: globalPrefixDir,
+    other: otherDirs,
   })
   const dirs = {
     testdir: dir,
     prefix: path.join(dir, 'prefix'),
     cache: path.join(dir, 'cache'),
     globalPrefix: path.join(dir, 'global'),
     home: path.join(dir, 'home'),
+    other: path.join(dir, 'other'),
   }
 
   // Set cache to testdir via env var so it is available when load is run

test/lib/cli.js

@@ -172,37 +172,15 @@ t.test('load error calls error handler', async t => {
   t.strictSame(exitHandlerCalled(), [err])
 })
 
-t.test('known broken node version', async t => {
-  const errors = []
-  let exitCode
-  const { cli } = await cliMock(t, {
-    globals: {
-      'console.error': (msg) => errors.push(msg),
-      'process.version': '6.0.0',
-      'process.exit': e => exitCode = e,
-    },
-  })
-  await cli(process)
-  t.match(errors, [
-    'ERROR: npm is known not to run on Node.js 6.0.0',
-    'You\'ll need to upgrade to a newer Node.js version in order to use this',
-    'version of npm. You can find the latest version at https://nodejs.org/',
-  ])
-  t.match(exitCode, 1)
-})
-
 t.test('unsupported node version', async t => {
-  const errors = []
-  const { cli } = await cliMock(t, {
+  const { cli, logs } = await cliMock(t, {
     globals: {
-      'console.error': (msg) => errors.push(msg),
       'process.version': '12.6.0',
     },
   })
   await cli(process)
-  t.match(errors, [
-    'npm does not support Node.js 12.6.0',
-    'You should probably upgrade to a newer version of node as we',
-    'can\'t make any promises that npm will work with this version.',
-  ])
+  t.match(
+    logs.warn[0][1],
+    /npm v.* does not support Node\.js 12\.6\.0\./
+  )
 })